-
Notifications
You must be signed in to change notification settings - Fork 30.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
v8: use AliasedBuffers for passing heap statistics around #32929
Conversation
Instead of holding shared pointers to ArrayBuffers, simplify the code by using AliasedBuffers directly which allows the binding to own the buffers.
This seems to cause a performance regression for
|
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 but I've been thinking strongly about moving things away from AliasedBuffer to the new AliasedStruct to give us a bit more natural way of interacting with the data on the native side.
For instance, instead of having to use buffer[idx] = val
on the native side, we'd have something more like buffer.foo = val
@mscdex yes, the performance regression is caused by more calls into the C++ in @jasnell Thanks for the heads up, it seems #32778 landed fairly recently. It seems to make more sense than the current homogenous array-based approach to me, though I guess we can make the transition incrementally :) |
Perhaps we should remove benchmark/v8/get-stats.js then? |
Instead of holding shared pointers to ArrayBuffers, simplify the code by using AliasedBuffers directly which allows the binding to own the buffers. PR-URL: #32929 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Landed in aa9708e |
Instead of holding shared pointers to ArrayBuffers, simplify the code by using AliasedBuffers directly which allows the binding to own the buffers. PR-URL: #32929 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Instead of holding shared pointers to ArrayBuffers, simplify
the code by using AliasedBuffers directly which allows the
binding to own the buffers.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes