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
perf(ivy): improve styling performance #33326
Conversation
change the existing implementation from using ``` string.split(/\s+/); ``` to a char scan which performers the same thing. The reason why `split(/\s+/)` is slow is that: - `/\s+/` allocates new `RegExp` every time this code executes. - `RegExp` scans are a lot more expensive because they are more powerful.
Wow, I am quite surprised the VM does not optimize the RegExp literal and creates a new RegExp each time. Are you saying that Also, ooc, have you tried |
|
I've run benchmarks on this PR and here are the results. Before:
After:
so there is a clear improvement here (although I was expecting it to be higher, TBH). Those numbers are confirmed by checking the results under profiler. Before: After: One can see that |
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.
LGTM. I think we should merge it.
@mhevery I will let you decide on if the perf improvement to the additional code makes it worthwhile.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
change the existing implementation from using
to a char scan which performers the same thing.
The reason why
split(/\s+/)
is slow is that:/\s+/
allocates newRegExp
every time this code executes.RegExp
scans are a lot more expensive because they are more powerful.