Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Performance] Cache phpunit results and run defects first to speed up the Mutation Testing execution #1549

Merged
merged 8 commits into from Sep 4, 2021

Conversation

maks-rafalko
Copy link
Member

@maks-rafalko maks-rafalko commented Aug 8, 2021

Implements #1519 feature request

When Infection runs Mutation Analysis for the first time, we can save the information about which Mutant was killed by which test, and then on the subsequent Infection executions (second and farther), we can use this cached knowledge and run those tests that killed particular Mutants first.

This should speed up MT as killed Mutants will be killed from the first "shot".


Running this new feature for one branch on the real project with slow functional test, I got the following result:

- Time: 1m 49s. Memory: 0.07GB (first run)
+ Time: 38s. Memory: 0.07GB (second run)

This is a very great result, and here is an explanation why it happens:

(on the left - the first run's log, on the right - the second run's log with reordered tests where defects run first and kill Mutants from the first shot)

orig-vs-cached-4

You can notice that on the first Infection run, we had to run 489 tests in order to kill PublicVisibility Mutant and it took 38 seconds (!), because functional tests are quite slow.

On the same time, on the second Infection run, we got the information from .phpunit.result.cache file and executed failed test first (that test that was 489th on the first run), and we killed Mutant by 0.5s.

For the test suites with fast unit tests, the difference is not so interesting, because unit tests are very fast, and running 10 unit tests or 1 unit test from those 10 - can be almost the same by time.

This is what I get for Infection itself, the total time is almost identical (3m 00s), the second and subsequent runs can be just a little bit faster, and the reason is:

orig-vs-cached-5

You can notice that instead of 34 tests we now run only 1 for this Mutant, but the difference in time is not very big:

- Time: 00:00.071, Memory: 14.00 MB
+ Time: 00:00.016, Memory: 14.00 MB

it is 0.071 - 0.016 = 0.055s

However, it's still makes sense to do it even for unit tests, that's why this feature is enabled out of the box.


Technical details

A couple of words about implementation:

  • PHPUnit has no public API to order tests inside the test suite, only setting <file> tags where PHPUnit will execute the tests from the first to the last (this is what we already do in master)
  • PHPUnit has no ability to pass defects (failed tests in the previous run) from stdin, it only reads them from cacheResultFile which is by default is .phpunit.result.cache
  • PHPUnit has built in feature called "Result Cache" - it saves execution time, defects (tests that failed) and other information that can be reused on subsequent calls. This is what I used to implement this performance improvement in this PR

Algotihm:

For each Mutant's phpunit.xml, we now add cacheResult=true and cacheResultFile=.phpunit.result.cache.%MUTANT_HASH% and executionOrder=defects attributes so that PHPUnit start collecting failed tests into separate .phpunit.result.cache.%MUTANT_HASH% files.

We save these cache files and do not remove them after Infection finishes its work (note: other temp files are removed).

Then, when Infection is executed again (second and more times), all these .phpunit.result.cache.%MUTANT_HASH% files are automatically reused by PHPUnit when we run processes for Mutants. And if PHPUnit finds a previously failed test, it automatically runs it first thanks to executionOrder=defects. If it doesn't find failed test (Mutant was escaped), it just runs the fast tests first as we have on master now).

In other words, this feature is implemented by reusing the PHPUnit's feature, and all the logic of storing failed tests and running them first is done by PHPUnit itself.

@MidnightDesign
Copy link
Sponsor Contributor

I ran it on a couple of repositories, and it's not the huge win that I would have expected:

--- Time: 5m 1s. Memory: 0.13GB
+++ Time: 4m 53s. Memory: 0.13GB
--- Time: 1m 46s. Memory: 0.17GB
+++ Time: 1m 29s. Memory: 0.17GB

The first one is our GraphQL API and it's basically nothing but functional tests (without I/O). But that's actually to be expected since each test class is pretty much tied to a resolver class.

Anyway, it does improve performance. For some projects more, for others less.

What do you think about the CI angle I mentioned in my original post? Carrying that information to GH Actions and speeding up those checks.

@maks-rafalko
Copy link
Member Author

@MidnightDesign are your results for the second Infection run? (when the cache files are already generated)

Anyway, it does improve performance. For some projects more, for others less.

Exactly. The more tests cover one mutated line, the more benefit this change will give.

What do you think about the CI angle I mentioned in my original post? Carrying that information to GH Actions and speeding up those checks.

  1. If you are speaking about committing these cache files to repository - I don't think it's an option for Infection itself, because the size is 16M
  2. If you are speaking about somehow reusing these cache files on GH actions - we should definitely try. However, it's not a part of this PR and I'm not sure I have enough knowledge to do it, will need to investigate. Anyway, be it GHA or Jenkins, this will be a responsibility of developers to reuse cache files. What we can do though - write a detailed docs on how to do it.

@MidnightDesign
Copy link
Sponsor Contributor

@MidnightDesign are your results for the second Infection run? (when the cache files are already generated)

--- = first run, +++ = second run

  1. If you are speaking about committing these cache files to repository - I don't think it's an option for Infection itself, because the size is 16M

Oh, really? Well, I guess that's a result of the implementation. When I was thinking about this improvement initially, I imagined the cache file to look something like a map of mutant identifier -> test method, which would probably be a lot smaller (but also way more involved to implement).

  1. If you are speaking about somehow reusing these cache files on GH actions - we should definitely try. However, it's not a part of this PR and I'm not sure I have enough knowledge to do it, will need to investigate. Anyway, be it GHA or Jenkins, this will be a responsibility of developers to reuse cache files. What we can do though - write a detailed docs on how to do it.

Just using a cache in CI is probably the better option, you're right. Maybe there should be an Infection Action that would take care of caching transparently - in addition to the documentation that you mentioned.

@hcoles
Copy link

hcoles commented Aug 9, 2021

@maks-rafalko You might be interesting in pitest's incremental analysis feature. This PR looks to implement one of the strategies it employs to speed things up, but others are also possible

https://pitest.org/quickstart/incremental_analysis/

Reading it again (it's a long time since I wrote it), I'm not sure number 5 is a great idea, but the others give pitest a huge speedup once the data has been collected.

@maks-rafalko
Copy link
Member Author

@hcoles thanks for sharing, I've added your comment to the corresponding ticket

Several ideas listed on https://pitest.org/quickstart/incremental_analysis/ look interesting and should work, but this will require experimenting and develop it as completely standalone feature with "infection cache files" rather than PHPUnit's cache file as it's done in this PR. One day we will try it

…Testing execution

When Infection runs Mutation Analysis for the first time, we can save the information about which Mutant was killed by which test, and then on the subsequent Infection execution (second and more), we can use this cached knowledge and run those test first that killed particular Mutant in a previous call.

This should speed up MT as killed Mutants will be killed from the first "shot".
@maks-rafalko
Copy link
Member Author

No review for ~30 days, merging for now since I'm going to prepare a release

@maks-rafalko maks-rafalko merged commit bba07d8 into master Sep 4, 2021
@maks-rafalko maks-rafalko deleted the feature/run-killing-first branch September 4, 2021 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Developer Experience Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants