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

Improve performance of isEqual() by skipping Comparator Factory #1478

Merged
merged 1 commit into from
Oct 21, 2014
Merged

Improve performance of isEqual() by skipping Comparator Factory #1478

merged 1 commit into from
Oct 21, 2014

Conversation

willemstuursma
Copy link
Contributor

For our case, a lot of time during test evaluation is spent on the comparison of different values through the assertEquals() assertion. See for example the performance trace of a test run:

Webgrind run of our tests

This PR will change PHPUnit to do a quick strict comparison check before doing an equality check. This check is very cheap, and is the most common case during test runs. If the expected and the actual value are not identical, the existing code path is followed and the Comparator library is used.

Performance of tests with this patch:

Time: 56.02 seconds, Memory: 261.00Mb

OK, but incomplete, skipped, or risky tests!
Tests: 4488, Assertions: 10734, Incomplete: 2, Skipped: 22.

Performance with base PHPUnit (4.3.1)

Time: 1.21 minutes, Memory: 261.00Mb

OK, but incomplete, skipped, or risky tests!
Tests: 4488, Assertions: 10734, Incomplete: 2, Skipped: 22.

Performance improvement is about 25-30%.

sebastianbergmann added a commit that referenced this pull request Oct 21, 2014
Improve performance of isEqual() by skipping Comparator Factory
@sebastianbergmann sebastianbergmann merged commit fe7f30c into sebastianbergmann:master Oct 21, 2014
@willemstuursma
Copy link
Contributor Author

Thanks, that was quick!

@sebastianbergmann
Copy link
Owner

Thank you for the idea. I changed the patch after the merge, though. Next step should be figuring out a solution for #1474.

@willemstuursma willemstuursma deleted the isequal-performance branch October 21, 2014 12:13
@willemstuursma
Copy link
Contributor Author

Any chances of this landing in the 4.3 branch too?

@sebastianbergmann
Copy link
Owner

No. This is too big of a change for a branch that is in "bugfix only" mode. This will go into PHPUnit 4.5.

@sebastianbergmann sebastianbergmann added this to the PHPUnit 4.5 milestone Oct 22, 2014
@willemstuursma
Copy link
Contributor Author

I've thought a bit about #1474. I see that is uses a singleton for the Comparator\Factory class. Most of the overhead is in constructing this object. So if it uses a singleton, we do not need the changes from this PR anymore, lost of time will already be saved by not instanciating the Factory for every comparison.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants