Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[PropertyAccess] Remove most ref mismatches to improve perf#18224
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
df9df89 toea33a2bComparestof commentedMar 18, 2016
do you have a profiling comparison of the gain to know how much difference it makes ? |
| }elseif (is_array($zval[self::VALUE])) { | ||
| $result[self::REF] = &$zval[self::REF][$index]; | ||
| }elseif (is_object($result[self::VALUE])) { | ||
| // Objects are always passed around by reference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this comment is not accurate (but we don't need a reference when dealing with objects). I suggest updating it to avoid confusing people reading the code
2fceaef to0c8e7edComparenicolas-grekas commentedMar 18, 2016
Here is a first comparison: $var =range(1,1000000);$pa =newPropertyAccessor();$res =$pa->getValue($var,'[12]'); |
javiereguiluz commentedMar 18, 2016
A more realistic benchmark would be to use |
0c8e7ed to72940d7Comparenicolas-grekas commentedMar 18, 2016
And one with setValue: $var =range(1,100000);$pa =newPropertyAccessor();$res =$pa->setValue($var,'[12]',$var); |
stof commentedMar 21, 2016
These numbers are very impressive. @nicolas-grekas can you also post a benchmark for a deeper property path ? 👍 |
fabpot commentedMar 21, 2016
Thank you@nicolas-grekas. |
…f (nicolas-grekas)This PR was merged into the 2.3 branch.Discussion----------[PropertyAccess] Remove most ref mismatches to improve perf| Q | A| ------------- | ---| Branch? | 2.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | no| Fixed tickets | -| License | MIT| Doc PR | -This PR is for PHP5 where ref mismatches is a perf pain: it removes all ref mismatches along the "getValue" path, and keeps only the required ones on the "setValue" path.Commits-------72940d7 [PropertyAccess] Remove most ref mismatches to improve perf
webmozart commentedMar 30, 2016
Awesome, thanks@nicolas-grekas :) |
This PR is for PHP5 where ref mismatches is a perf pain: it removes all ref mismatches along the "getValue" path, and keeps only the required ones on the "setValue" path.