Closed jderusse closed 4 years ago
Reviewing the compile
function is very hard.
I've add a "matrix" test that take all case (combining operators and type of version) and compare the result of matches
with compile
to ensure both implementations are identical
Which triggers 4 errors that sounds legit to me
==3.0-b2 matches >3.0-beta2 => current = true, mine = false
==3.0-b2 matches <3.0-beta2 => current = true, mine = false
==3.0-beta2 matches >3.0-b2 => current = true, mine = false
==3.0-beta2 matches <3.0-b2 => current = true, mine = false
This looks great to me!
Merging #86 into master will decrease coverage by
0.94%
. The diff coverage is88.88%
.
@@ Coverage Diff @@
## master #86 +/- ##
============================================
- Coverage 96.04% 95.10% -0.95%
- Complexity 251 287 +36
============================================
Files 7 8 +1
Lines 481 552 +71
============================================
+ Hits 462 525 +63
- Misses 19 27 +8
Impacted Files | Coverage Δ | Complexity Δ | |
---|---|---|---|
src/Constraint/EmptyConstraint.php | 62.50% <0.00%> (-8.93%) |
8.00 <1.00> (+1.00) |
:arrow_down: |
src/Constraint/MultiConstraint.php | 91.66% <66.66%> (-4.63%) |
55.00 <8.00> (+8.00) |
:arrow_down: |
src/CompiledMatcher.php | 94.11% <94.11%> (ø) |
8.00 <8.00> (?) |
|
src/Constraint/Constraint.php | 100.00% <100.00%> (ø) |
57.00 <19.00> (+19.00) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update bd21c31...8f687aa. Read the comment docs.
Tried this on a few projects and seeing about 20% CPU time reduction on pure dependency solving for ~0.3% memory increase, so totally worth it. Thanks @jderusse and all reviewers for the great work here!
This PR adds a
Compile
method to the constraints to optimize constraints computation.By caching the compiled code, we will be able to perform much more complex optimization on MultiConstraint https://github.com/composer/semver/pull/84#issuecomment-620502945
By using this new feature in the most used called locations in Composer (
Pool
,PoolBuilder
andComposerRepository
) I get 15% CPU https://blackfire.io/profiles/compare/cfa5e43e-0510-4b24-bd44-722dc0c3c7ce/graphNot covered by the current PR:
$compareBranches=true
=> adds complexity in Constraint::compile method and called 15x less often. But that could be doables tell me if you think it worth itMutliConstraint->match(MultiConstraint)
Here is a sample of code generated by the compile method