replace string slices with startsWith comparison#9
Merged
guybedford merged 2 commits intonodejs:masterfrom Oct 3, 2020
Merged
Conversation
Collaborator
|
I must admit I am somewhat shocked V8 doesn't just do this optimization internally, but the PR looks great thank you. I will update the benchmarks now. |
guybedford
reviewed
Oct 3, 2020
Collaborator
|
Yeah I'm also seeing a ~20% improvement here, benchmarks updated in 9a551db. Note that the huge benefit of Wasm is not the warm speed but the cold speed because the compilation doesn't need to be done JIT which saves ~ 250ms of compile time V8 otherwise has to do. |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit replaces string slice operations in the lexer with
startsWithoperations, using an offset position to start the search from. This avoids intermediate string slices so should be easier on the allocator and garbage collector.I am seeing a performance improvement of -14% on average with these changes, as measured on the following system:
CPU: AMD Ryzen 3700X @ 3.6GHz (turbo'ing disabled for more stable results)
Mem: 64GB
OS: macOS 10.15.2 (Hackintosh)
Node.js: 14.13.0
Samples: 1000 (increased from default 25)
The WASM results are not affected, obviously:
Raw output:
master (4d56d33)
PR
I am curious to see results on other hardware, as the measurements in the readme show that JS is roughly 47% slower than WASM, but for me it's only 29% on master; this is reduced to just 11% with these changes.