-
Notifications
You must be signed in to change notification settings - Fork 144
Performance take2 #69
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
Conversation
Especially when these functions are being called hundreds of thousands of times during a parse, preincrementing absolutely is a noticeable performance boost.
We can get this information from the peek() return.
We already do a boundary check later in the method (after determining the peek size).
Only very rarely do we care about case sensitivity. Most of the time, we are dealing with symbols. This saves on a ton of strtolower() calls. Also can save on a more expensive mb_strlen() call if we know we are dealing with symbols (for any practical charset).
Cache the common case of a single byte of peek data.
No reason for this method to be static Optimize for the most-expected case - a non-prefixed identifier.
I don’t see much of an improvement in running time for the unit tests but it’s not getting worse either so I’ll merge this. |
Performance improvements by caching some invariants
Your unit tests are not a good real-world example, at least for run-times. Think of CSS parsing in the browser setting - you are parsing 100KB+ CSS stylesheets every few seconds. Which is what we are seeing in our code (we might be seeing 3 different PHP processes concurrently parsing 200+ KB of CSS EVERY SECOND. This is not unexpected, since there are thousands of concurrent users). Thus, at least for us, parsing speed is an issue. And these changes half the parsing speed - as verified by xdebug - on larger CSS payloads. |
I have some CSS test files locally (taken from Facebook and other live sites) that I haven’t committed into the repository as they don’t really test any features. I was talking about them. With your patch parsing all of these takes a second less than it did before (totalling 16 seconds). |
Will open another ticket with fix. |
So a smaller, less ambitious performance try. The combination of these provides about a 25% performance boost, at least according to xdebug data.
peek() is the deadly method that needs to be worked on further. It consumes about 36% of runtime. comes() is the next biggest performance hog, but it only consumes about 11%.