-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
Include Sparkplug compiler #38855
Comments
I think we should not early unflag this, instability in this type of component could lead to misbehavior or even security risk. |
While I'm not interested in unflagging it by default until V8 unflags it by default, I would love to see some of our benchmark results with this enabled compared to without it. (I wonder if the V8 team at Google is already doing that?) |
@Trott you might be able to run some benchmarks with our v8 canary and that flag |
I guess you could say it is unflagged since Chrome 91 uses it by default. Bit of a stretch maybe. https://blog.chromium.org/2021/05/chrome-is-faster-in-m91.html?m=1 |
I wonder if/when the V8 team intends to make it the default. I would have thought whatever Chrome 91 got would have been the default in V8 9.1, but I guess that was a bad assumption! |
If chromium was unflagging it in v8 9.0 i would feel more comfortable with this, but i think we should wait. |
You'd be comfortable unflagging it once we are using V8 9.1? Or that remains to be seen? |
I mean I'd rather we just don't unflag things at all. Once we have v8 9.1 that would go from blocking concern to very strong concern. |
I noticed v8 9.1 was added in e82ef4148e (released in 16.4), would it be alright to revisit the unflagging? :) |
Sparkplug will be enabled by default in V8 9.4: v8/v8@86c81a8 |
@targos sounds great, thanks! I will close this as resolved since no action is required from the node side on this |
FYI #40285 V8 9.4 landed on this pr. |
Yep, 9.4 is officially in 16.11: https://github.com/nodejs/node/releases/tag/v16.11.0. Here we go :) |
Is your feature request related to a problem? Please describe.
Google recently released the Sparkplug compiler as part of V8 9.1, sporting performance improvements of up to 23% on real-world JS applications: https://v8.dev/blog/sparkplug
Describe the solution you'd like
Currently, Sparkplug is behind the
--sparkplug
flag in v8 9.1. It would be great if it could be enabled by default in Node.Describe alternatives you've considered
N/A
The text was updated successfully, but these errors were encountered: