deps,lib,tools: remove jitless and lite modes - #66459
Conversation
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
|
Review requested:
|
nodejs-github-bot
added
lib / src
marco-ippolito
added
the
semver-major
There was a problem hiding this comment.
These are going to require perpetual V8 floating patches, are they strictly necessary? eg. can we override --no-jitless in InitializeNodeWithArgsInternal?
There was a problem hiding this comment.
Done. How about now?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #66459 +/- ##
==========================================
- Coverage 90.43% 90.42% -0.02%
==========================================
Files 791 790 -1
Lines 275604 275427 -177
Branches 52849 52812 -37
==========================================
- Hits 249239 249045 -194
- Misses 16770 16784 +14
- Partials 9595 9598 +3
🚀 New features to boost your workflow:
|
ShogunPanda
added
the
request-ci
mcollina
added
request-ci
github-actions
Bot
removed
the
request-ci
ShogunPanda
added
commit-queue
|
Landed in 7e39b87 |
nodejs-github-bot
removed
the
commit-queue
| // Node.js requires WebAssembly for built-in functionality. Override these | ||
| // modes after all option sources, before V8 applies their implications. | ||
| // Explicitly allow overriding even when contradiction checks are enabled. | ||
| V8::SetFlagsFromString("--allow-overwriting-for-next-flag --no-lite-mode " |
There was a problem hiding this comment.
--allow-overwriting-for-next-flag has been removed by V8 which now only allows ignoring the contradiction https://chromium-review.googlesource.com/c/v8/v8/+/7775553
There was a problem hiding this comment.
I opened a revert #66516 to unblock the V8 upgrade. IMO to reland this PR, it's better to add some support in the upstream for the override first, otherwise this just leaves no way of opting-into jitless etc. even if the users do it at their own risk.
There was a problem hiding this comment.
@JoyeeChung Thanks, I didn't know about it.
We have two choices here:
1 - As in the original PR we remove support for jitless. Unlike the original PR I would throw if the user tries to enable it rather than overwrite it.
2 - we let them to, but whenever the user tries to access WebAssemly we also throw an error and exit. But this also means that some part of Node might be totally unaccessible in the future.
To clarify, when I introduce Milo as replacement of llhttp, the entire http1 will be affected. Fetch is already like that today. That's why I went for the hard route.
WDYT?
There was a problem hiding this comment.
@joyeecheung I just sent #66524, which should eliminate the need for the revert. PTAL.
I don't understand why we don't leave that choice to the user, they might have good reasons to disable those, maybe "partially functional" is good enough for them |
|
IMHO is a very niche feature that poses more maintainance burden on top of collaborators because all the times we have to protect access to WASM or similar and often provide fallbacks. Instead, cutting it off will allow us to draw a clear line on what we do or don't support and can lead us to a lean and modern Node. |
|
I think you should update the description with that explanation, which is more honest and less paternalistic. I would also mention the plans regarding milo. My two cents: it’s better to leave a blank description that use an LLM generated one, but that’s another topic |
9 participants
Summary
Remove support for jitless execution and V8 lite mode. Built-in functionality, including
fetch()through Undici and TypeScript through Amaro, requires WebAssembly; supporting configurations without it leaves Node.js partially functional.Changes
NODE_OPTIONS, and V8’s runtime flag API.--v8-lite-modebuild support, always enable WebAssembly, and reject interpreter-only V8 build defines.ERR_WEBASSEMBLY_NOT_SUPPORTED.Notes
Assisted-By: OpenAI:GPT-6 Astra Ultrafast <openai/gpt-6-astra-ultrafast>