[wasm-custom-desc] Fast path for installing methods [v8/v8 : main]
Jakob Kummerow (Gerrit)
Jakob Kummerow voted and added 1 comment![]( "Open in Gerrit")
Votes added by Jakob Kummerow
| Commit-Queue | +1 |
1 comment
WDYT, is this a good approach? This basically duplicates small bits of `lookup.cc`, which I'm not too happy about, but it does provide significant speedups, so perhaps the complexity is justifiable? I'm open to massaging / widening / restricting the fast path if you think that improves the situation.
Related details
- Igor Sheludko
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Kummerow (Gerrit)
Jakob Kummerow added 1 comment![]( "Open in Gerrit")
// perspective it'd be fine to just bail out of the fast path; but theAn earlier local version of this patch did that, but making sure that we don't screw things up when bailing out was messy and brittle, so I've changed it to this now and I think that's simpler and more robust.
Related details
- Igor Sheludko
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Igor Sheludko (Gerrit)
Igor Sheludko voted and added 3 comments![]( "Open in Gerrit")
Votes added by Igor Sheludko
| Code-Review | +1 |
3 comments
lgtm with comments
// here, but that would have a significant performance cost, so for nowProbably not that big if we had `strict_function_with_NAME_AND_readonly_prototype_map` (it's just a two liner to add it).
if (fast_path) {I guess we should invalidate the prototype's validity cell at the end of all stores just in case someone already relies on it (on both throwing and non-throwing paths).
Related details
- Jakob Kummerow
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jakob Kummerow (Gerrit)
Jakob Kummerow voted and added 3 comments![]( "Open in Gerrit")
Votes added by Jakob Kummerow
| Auto-Submit | +1 |
| Commit-Queue | +1 |
3 comments
Thanks for the review and the great comments! I've addressed them; and it turned out that including the bug fix we discussed offline in this CL was really easy; but updating the test lost the +1. PTAL.
// here, but that would have a significant performance cost, so for nowProbably not that big if we had `strict_function_with_NAME_AND_readonly_prototype_map` (it's just a two liner to add it).
Thanks for that idea! Added it to the comment in case we need it in the future.
I guess we should invalidate the prototype's validity cell at the end of all stores just in case someone already relies on it (on both throwing and non-throwing paths).
- Igor Sheludko
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Igor Sheludko (Gerrit)
Igor Sheludko voted and added 1 comment![]( "Open in Gerrit")
Votes added by Igor Sheludko
| Code-Review | +1 |
| Commit-Queue | +2 |
1 comment
lgtm
- Jakob Kummerow
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[wasm-custom-desc] Fast path for installing methods
This patch adds a fast path for the common case of installing
non-conflicting methods on previously-empty prototype objects.
This saves about 10ms for 100K methods.
- M src/runtime/runtime-wasm.cc
- M test/mjsunit/wasm/custom-descriptors-interop.js
Code-Review: +1 by Igor Sheludko
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |