-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Inserter: Cache search normalization results #60080
Inserter: Cache search normalization results #60080
Conversation
Size Change: +1.3 kB (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
Thank you, @tyxla!
The changes work well when testing locally, and I see that all CI checks are green. The performance improvement will be more visible when the commit is accounted for in the CodeVitals.
* Inserter: Cache search normalization results * Use separate objects for caching normalized data * Use maps for caching * Declare normalization regexes just once * Rename function as suggested Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
* Inserter: Cache search normalization results * Use separate objects for caching normalized data * Use maps for caching * Declare normalization regexes just once * Rename function as suggested Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
What?
We're adding some internal cache to the internal inserter search input normalization functions.
Why?
During a single search, we'll call each of these functions hundreds of times, with the same input and the same output.
Internally, both of them will do some string manipulation, including regex search and replace, and there's no need to run it every time when we can only run it once per session.
Although I don't aim for any visible performance improvements, in my local testing, this saves a few ms (5ms on average) for every inserter search. Even if it saves 1 ms, that would still be an improvement of over 10% for each inserter search, so it's worth trying out.
Performance tests seem to indicate a very slight improvement to the inserter search metrics:
however, I usually don't trust them that much, as the demonstrated difference is within the expected noise levels.
How?
We're targeting
extractWords
andnormalizeSearchInput
since they're both internal, private functions.~For both of them, we're caching the normalization / extraction results in the function object itself, since that way, we don't need to create an extra object for the cache. ~
IMHO that's fine to do since those functions are private for the inserter search items module.
Testing Instructions
Testing Instructions for Keyboard
None
Screenshots or screencast
None