-
Notifications
You must be signed in to change notification settings - Fork 680
Add property typehints in Internal directory #8897
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
Add property typehints in Internal directory #8897
Conversation
@jack-worman would you mind running benchmarks against this PR and the master branch? It looks like it could have more performance impact than your previous PR. |
OLD (5.2.0):
256.11s user 20.99s system 342% cpu 1:20.90 total
251.93s user 19.63s system 351% cpu 1:17.33 total
252.73s user 19.57s system 347% cpu 1:18.33 total
254.36s user 19.10s system 350% cpu 1:17.92 total
252.01s user 19.30s system 352% cpu 1:16.94 total
247.46s user 18.03s system 340% cpu 1:17.97 total
NEW (this branch):
246.72s user 19.21s system 346% cpu 1:16.84 total
243.94s user 18.63s system 354% cpu 1:14.13 total
244.22s user 18.06s system 357% cpu 1:13.39 total
249.78s user 19.64s system 346% cpu 1:17.82 total
260.73s user 21.50s system 336% cpu 1:23.89 total
255.39s user 19.22s system 354% cpu 1:17.42 total Using eyeball statistics, I would say there is no performance difference. |
06dae8d
to
33ecb1a
Compare
33ecb1a
to
76a7c7d
Compare
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.
Should we ignore backward compatibiliy checks here? |
Those BC breaks are because those files were missing their |
If there are no performance impacts (and according to your tests performance improved even, were you using opcache? If yes, the typehints might've triggered some optimizations), LGTM! |
Thanks! |
Added more property typehints but just for in the Internal directory to ensure BC.
I then used php-cs-fixer to clean up redundant phpdocs.