Deferify Mutant

[sanmai]

This PR is a follow-up to #1422:

• Deferify MutantExecutionResult
• Deferify MutantFactory
• Blackfire diff shows 34% time reduction
• As seen on CI: 7m 10s on master vs 6m 21s for this PR
• Covered by tests

Running this on Psalm with a config below, without any file logging:

- Time: 2m 23s. Memory: 2.15GB
+ Time: 1m 42s. Memory: 2.15GB

infection.json and command line arguments

{
    "timeout": 2,
    "source": {
        "directories": [
            "src"
        ]
    },
    "mutators": {
        "@unwrap": true
    }
}

Command line:

infection.phar --skip-initial-tests --coverage=build/logs -j2

There's almost no difference if logging is enabled, which makes sense: we'll have to unlazify the diffs to log them.

- Time: 2m 24s. Memory: 2.15GB
+ Time: 2m 23s. Memory: 2.15GB

With --no-progress --show-mutations and no additional logging I get:

- Time: 2m 17s. Memory: 0.29GB
+ Time: 1m 38s. Memory: 0.53GB

This is 30% faster!

It is true this lazy evaluation isn't coming without a price, but I tend to think it's worth it. More about it below.

Discussion

It is clear to see the speed boost we're having, but there are drawbacks:

1. This boost comes at the expense of raised top memory consumption. The program has to hold chunks of AST instead of flat diffs and source code listings. This is possible to remedy by further work (continued in Refactor ResultsCollector out of MetricsCalculator #1425):
   • Now we're holding on to all mutation results, where it should be possible to discard unneeded results as we go. E.g. when running with --only-covered it makes no sense to hold on to uncovered mutations. We might count them, but that should be it.
   • Likewise, when there's no logging other that to show escaped mutations, we might discard other mutations as soon as we count them.
   • And more, we can stream-write to logs as we receive mutation results.
2. Most of the work to render mutants is being postponed, therefore there could be a lag to write to log files upon finishing with all mutations. This could be remedied by adding an extra progress bar indicator for log-writing process.
3. With all this deferred execution the program is slightly less easy to follow. There's no remedy.

[theofidry]

With all this deferred execution the program is slightly less easy to follow. There's no remedy.

Could be easier to follow if we were to get rid of Mutant? (#1209)

I can take some time to rebase the PR this weekend if necessary, but I would be curious to see if that can help out with those performance gains as well

[sanmai]

Could be easier to follow if we were to get rid of Mutant? (#1209)

There's no contradiction here. If we'd get rid of Mutant, we'll still have to manage diffing and pretty-printing (#1209 does that in the same order, e.g. long before use). And therefore if we'd do that in a deferred way, there will be the same readability issue.

If we'd consider that a Mutant, or what is in place of it, could do none of the diffing and printing, but instead pass around ASTs, we'll no longer have this problem, but that's a huge responsibility skew with quite a blast radius. I'm not so certain it's going to be worth it, yet neither I can say it's a bad idea. It is just somewhat more difficult that the current approach.

In other words, the reason we're paying with sequentiality here is because making program less sequential while achieving the same goal will require less amount of change (smallest blast radius).

[theofidry]

(#1209 does that in the same order, e.g. long before use)

Yes I recall that, but actually could easily be done lazily as you mentioned although this comes at the cost of holding the whole AST in-memory for longer

I did not mean that there is contradiction, just that there a big overlap and that maybe your PR would look a lot simpler after #1209. But if you don't think so I guess I'll have to take on myself to do a more complex rebase :P

That said I would like us to move forward a bit on #1209 because I don't want to have to rebase it if we don't want that change at all...

[sanmai]

I'll have to take on myself to do a more complex rebase

Well, I feel your pain. Sorry about that!

Do you think you can reduce #1209 to more manageable partial PRs? You have an itch to do the rename/removal, and can't ignore but respect that, yet it's kind of difficult to get #1209 in your head. If it wasn't we'll have had it done already.

[maks-rafalko]

I don't think #1209 should block anything here. It's

• already stale for a half of the year
• arguable (I still don't agree with the renamings), we don't have an agreement there

[sanmai]

Thank you @maks-rafalko

[theofidry]

Hm apparently I forgot to push my comment.

I don't think #1209 should block anything here. It's

already stale for a half of the year
arguable (I still don't agree with the renamings), we don't have an agreement there

Outch that hurts :(

Anyway I'll try to see if there is anything that can be extracted. I don't want to spend hours refactoring/rebasing a PR that is not going to be merged

[maks-rafalko]

Outch that hurts :(

didn't mean to hurt you, sorry! But those 2 points are what I see.