Introduce FilteringResultsCollector

[sanmai]

This PR:

• Adds FilteringResultsCollector that excludes unneeded mutation results depending on logging/debugging configuration.
• Covered by tests
• Benchmarks
  • From current master build there's a 66% memory reduction.
  • From 0.20.1 a diff shows -46% time reduction, -23% memory reduction.

I'm not too happy about TargetDetectionStatusesProvider because it does what individual loggers should provide, but that should do it for now. Changing the loggers to supply this information to TargetDetectionStatusesProvider will require fairly substantial refactoring with otherwise uncertain benefits.

Follow-up to #1425

Blackfire is a bit flacky, here are the screenshots.

• Versus the current master
  
• Versus 0.20.1
  

[theofidry]

I'm 👍. Again I cannot focus on the code details atm but the general idea looks good to me.

I however have one reserve but I think it's your plans anyway: I don't think the current TargetDetectionStatusesProvider make any sense.

Its role – if I understand correctly, is to provide which status to filter by to the filter for the collector, collector that is tailored for the loggers.

I guess we can add:

interface MutationTestingResultsLogger {
  // ...

  /**
   * @return string[]
   */
  getUsedDetectionStatuses(): array;
}

So the logger registry can combine all of them without having to know any details. However the tricky part is to achieve that since we are creating the loggers lazily by passing the factory to the subscriber...

So maybe we will need a LoggerProxy which takes the factory and instantiate the underlying loggers underneath. This way both the subscriber and the provider can consume this proxy (which can implement the interface of the logger even).

I'm also wondering about the name, what do you think of renaming TargetDetectionStatusesProvider to LoggerDetectionStatusDetector?

I also wonder if it would make sense to start documenting this part of our application, this is starting to be a complex piece of the app with a lot of indirections...

[maks-rafalko]

👌

I also wonder if it would make sense to start documenting this part of our application, this is starting to be a complex piece of the app with a lot of indirections...

agree. Any good examples of how to do it?

[sanmai]

@theofidry I'm 100% on board with you on the state of TargetDetectionStatusesProvider. I tried to go this getUsedDetectionStatuses() route, but found, like you said, even if some of the loggers would implement such method, it is difficult to get to them both because they're created after the fact, and because they're behind several layer of abstraction.

I'm also wondering about the name, what do you think of renaming TargetDetectionStatusesProvider to LoggerDetectionStatusDetector?

This is a certainly more appropriate name considering the current state of affairs, but I named it such because it is supposed to feed from loggers. It is not supposed to detect anything, in a perfect world. What do you think about LoggerDetectionStatusProvider?

I'll update the code to link to your review.

[sanmai]

I'm going to merge this for now, to see if I can do more in this direction in a next PR.

[theofidry]

It is not supposed to detect anything, in a perfect world. What do you think about LoggerDetectionStatusProvider?

I was seeing more as "detect/find out what detection status the loggers are gonna use"

like you said, even if some of the loggers would implement such method, it is difficult to get to them both because they're created after the fact, and because they're behind several layer of abstraction.

But with this LoggerProxy we can fix that right? (understood that this would be done in a separate PR)