-
-
Notifications
You must be signed in to change notification settings - Fork 411
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
[Merged by Bors] - Add integer type to fast path of to_property_key #2261
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2261 +/- ##
==========================================
+ Coverage 41.31% 41.34% +0.02%
==========================================
Files 234 234
Lines 22019 22023 +4
==========================================
+ Hits 9097 9105 +8
+ Misses 12922 12918 -4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Very nice optimization. I think we could event optimize this further if I read the spec right. The slow path using |
Updated. There was no notable performance improvement though since |
Benchmarks
|
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.
I was unable to reproduce performance gains in my laptop or on GitHub actions, but I guess it's reasonable. LGTM :) thanks!
Thanks for review :) |
Interesting indeed, as I have the same hardware. But might have been that I had other stuff running at the same time. No worries, the optimization seems very reasonable :) |
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.
Very good work! And the new test is really appreciated
bors r+ |
Skip `to_string` for integer type primitives in `to_property_key`. It's unnecessary to convert the integer value to string and convert back to `Index(u32)` type. In this example code, it improves around 10% of runtime. ```js let arr = [1,2,3,4,5]; for (let i = 0; i < 10000000; i++) { arr[0] = 123; } ``` Before: 6.24s After: 5.38s
Pull request successfully merged into main. Build succeeded: |
Skip
to_string
for integer type primitives into_property_key
. It's unnecessary to convert the integer value to string and convert back toIndex(u32)
type.In this example code, it improves around 10% of runtime.
Before: 6.24s
After: 5.38s