GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — deps: update V8 to 13.0 by targos · Pull Request #55014 · nodejs/node

Conversation

@targos

@targos targos commented Sep 19, 2024

Copy link
Copy Markdown
Member

Notable changes:

  • WebAssembly JS String Builtins

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss and kirillgroshkov reacted with thumbs up emoji bricss, avivkeller, bnoordhuis, H4ad, and kirillgroshkov reacted with hooray emoji marco-ippolito, bricss, H4ad, and kirillgroshkov reacted with heart emoji bricss and kirillgroshkov reacted with rocket emoji
All reactions
  • 👍 2 reactions
  • 🎉 5 reactions
  • ❤️ 4 reactions
  • 🚀 2 reactions
@targos targos added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 19, 2024
@nodejs-github-bot

nodejs-github-bot commented Sep 19, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Sep 19, 2024
@avivkeller

avivkeller commented Sep 19, 2024

Copy link
Copy Markdown
Member

@nodejs/tsc per nodejs/Release#1034

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@avivkeller avivkeller added commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. dependencies PRs that add, update, or configure Node.js dependencies. labels Sep 19, 2024
ronag
ronag reviewed Sep 19, 2024
View reviewed changes

@ronag ronag left a comment

Copy link
Copy Markdown
Member

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.

Now I think we have disposable and Co? Maybe notable?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@targos

targos commented Sep 19, 2024

Copy link
Copy Markdown
Member Author

I get the notable changes from https://chromestatus.com/roadmap.
There's nothing about disposable there (and using still triggers a syntax error), why do you expect it to be available?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos targos mentioned this pull request Sep 19, 2024
Linker errors on test-linux action nodejs/node-v8#290
Closed
@ronag

ronag commented Sep 19, 2024

Copy link
Copy Markdown
Member

It's in the v8 code. Maybe not enabled still?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos

targos commented Sep 19, 2024

Copy link
Copy Markdown
Member Author

Probably. The tracking issue is still open: https://issues.chromium.org/issues/42203506

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 19, 2024
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 19, 2024
@nodejs-github-bot

nodejs-github-bot commented Sep 19, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/62572/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Sep 19, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6203/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Sep 19, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6203/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Sep 19, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6203/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@codecov

codecov Bot commented Sep 19, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.69%. Comparing base (304bb9c) to head (63ee9f0).
Report is 22 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #55014      +/-   ##
==========================================
- Coverage   89.20%   88.69%   -0.52%     
==========================================
  Files         663      663              
  Lines      192012   192012              
  Branches    36929    36615     -314     
==========================================
- Hits       171286   170306     -980     
- Misses      13582    14530     +948     
- Partials     7144     7176      +32     

see 94 files with indirect coverage changes

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos

targos commented Sep 20, 2024

Copy link
Copy Markdown
Member Author

@nodejs/platform-windows MSVC complains:

23:17:25 C:\workspace\node-compile-windows-debug\node\deps\v8\src\interpreter\interpreter-generator-tsa.cc(333,1): error C2872: 'Block': ambiguous symbol [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_initializers.vcxproj]
23:17:25 C:\workspace\node-compile-windows-debug\node\deps\v8\src\ast\ast.h(318,7): message : could be 'v8::internal::Block' [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_initializers.vcxproj]
23:17:25 C:\workspace\node-compile-windows-debug\node\deps\v8\src\compiler\turboshaft\graph.h(306,7): message : or       'v8::internal::compiler::turboshaft::Block' [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_initializers.vcxproj]
23:17:25 C:\workspace\node-compile-windows-debug\node\deps\v8\src\interpreter\interpreter-generator-tsa.cc(333,1): error C2440: 'initializing': cannot convert from 'v8::internal::compiler::turboshaft::Block *' to 'v8::internal::Block *' [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_initializers.vcxproj]
23:17:25 C:\workspace\node-compile-windows-debug\node\deps\v8\src\interpreter\interpreter-generator-tsa.cc(333,1): message : Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or parenthesized function-style cast [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_initializers.vcxproj]
23:17:25 C:\workspace\node-compile-windows-debug\node\deps\v8\src\interpreter\interpreter-generator-tsa.cc(333,1): error C2665: 'v8::internal::compiler::turboshaft::CatchScopeImpl<v8::internal::compiler::turboshaft::Assembler<v8::internal::compiler::turboshaft::reducer_list<v8::internal::compiler::turboshaft::TurboshaftAssemblerOpInterface,Reducer,v8::internal::interpreter::BytecodeHandlerReducer,v8::internal::BuiltinsReducer,v8::internal::FeedbackCollectorReducer,v8::internal::compiler::turboshaft::MachineLoweringReducer,v8::internal::compiler::turboshaft::VariableReducer,v8::internal::compiler::turboshaft::TSReducerBase>>>::CatchScopeImpl': no overloaded function could convert all the argument types [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_initializers.vcxproj]
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos

targos commented Sep 20, 2024

Copy link
Copy Markdown
Member Author

@ronag @anonrig
It seems that something changed with the fast API which results in fast C++ functions not being called when they expect FastOneByteString arguments (I verified the tests with --trace-opt --trace-deopt. The JS functions are still optimized).
I don't know what to do here other than 112b303

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ronag

ronag commented Sep 20, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

So basically Fast API is "broken"? This will cause some massive perf regressions (maybe good idea to kick of a benchmark CI to confirm). Can we involve someone with V8 knowledge? If benchmarks do not show regressions then I agree that we just apply 112b303 as a workaround.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mcollina
mcollina approved these changes Sep 20, 2024
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@targos

targos commented Sep 20, 2024

Copy link
Copy Markdown
Member Author

Either fast API is "broken", or the strings used in the test are no longer of the kind "fast one-byte".

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Sep 20, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/62584/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bnoordhuis

bnoordhuis commented Sep 20, 2024

Copy link
Copy Markdown
Member

So basically Fast API is "broken"?

No, the test is. At least two of the four calls pass two-byte strings.

I believe v8/v8@a1ada77 is the responsible change upstream.

ronag reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos

targos commented Sep 20, 2024

Copy link
Copy Markdown
Member Author

I'm trying to upstream a fix for the MSVC error: https://chromium-review.googlesource.com/c/v8/v8/+/5878257

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos
targos force-pushed the v8-130 branch from 112b303 to 35d3cb9 Compare September 25, 2024 20:46
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 25, 2024
@targos targos closed this Jan 31, 2025
@targos
targos deleted the v8-130 branch January 31, 2025 11:51
@targos targos mentioned this pull request Jan 31, 2025
smartOS builds are taking more than 6 hours nodejs/build#4011
Closed
targos pushed a commit that referenced this pull request Jan 31, 2025
@StefanStojanovic @targos
deps: fix FP16 bitcasts.h
688feec
PR-URL: #53134
Refs: #52809
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: #55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
targos added a commit that referenced this pull request Jan 31, 2025
@targos
deps: always define V8_NODISCARD as no-op
e1370a1
It's causing compiler errors with some classes on Xcode 11
and the attribute should have no runtime effect.

PR-URL: #54077
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: #55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
targos pushed a commit that referenced this pull request Jan 31, 2025
@StefanStojanovic @targos
deps: define V8_PRESERVE_MOST as no-op on Windows
46db797
It's causing linker errors with node.lib in node-gyp and potentially
breaks other 3rd party tools

PR-URL: #56238
Refs: #55784
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #55014
Reviewed-By: Matteo Collina <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 2, 2025
@StefanStojanovic @nodejs-github-bot
deps: fix FP16 bitcasts.h
1533412
PR-URL: nodejs/node#53134
Refs: nodejs/node#52809
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 2, 2025
@targos @nodejs-github-bot
deps: always define V8_NODISCARD as no-op
428b034
It's causing compiler errors with some classes on Xcode 11
and the attribute should have no runtime effect.

PR-URL: nodejs/node#54077
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 2, 2025
@StefanStojanovic @nodejs-github-bot
deps: define V8_PRESERVE_MOST as no-op on Windows
09dee60
It's causing linker errors with node.lib in node-gyp and potentially
breaks other 3rd party tools

PR-URL: nodejs/node#56238
Refs: nodejs/node#55784
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 3, 2025
@StefanStojanovic @nodejs-github-bot
deps: fix FP16 bitcasts.h
884e7d0
PR-URL: nodejs/node#53134
Refs: nodejs/node#52809
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 3, 2025
@targos @nodejs-github-bot
deps: always define V8_NODISCARD as no-op
1cc5d0c
It's causing compiler errors with some classes on Xcode 11
and the attribute should have no runtime effect.

PR-URL: nodejs/node#54077
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 3, 2025
@StefanStojanovic @nodejs-github-bot
deps: define V8_PRESERVE_MOST as no-op on Windows
3040016
It's causing linker errors with node.lib in node-gyp and potentially
breaks other 3rd party tools

PR-URL: nodejs/node#56238
Refs: nodejs/node#55784
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 4, 2025
@StefanStojanovic @nodejs-github-bot
deps: fix FP16 bitcasts.h
f218ccc
PR-URL: nodejs/node#53134
Refs: nodejs/node#52809
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 4, 2025
@targos @nodejs-github-bot
deps: always define V8_NODISCARD as no-op
6ae0c95
It's causing compiler errors with some classes on Xcode 11
and the attribute should have no runtime effect.

PR-URL: nodejs/node#54077
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 4, 2025
@StefanStojanovic @nodejs-github-bot
deps: define V8_PRESERVE_MOST as no-op on Windows
fea7bba
It's causing linker errors with node.lib in node-gyp and potentially
breaks other 3rd party tools

PR-URL: nodejs/node#56238
Refs: nodejs/node#55784
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 5, 2025
@StefanStojanovic @nodejs-github-bot
deps: fix FP16 bitcasts.h
6c27b31
PR-URL: nodejs/node#53134
Refs: nodejs/node#52809
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 5, 2025
@targos @nodejs-github-bot
deps: always define V8_NODISCARD as no-op
6cbab13
It's causing compiler errors with some classes on Xcode 11
and the attribute should have no runtime effect.

PR-URL: nodejs/node#54077
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
nodejs-github-bot pushed a commit to nodejs/node-v8 that referenced this pull request Feb 5, 2025
@StefanStojanovic @nodejs-github-bot
deps: define V8_PRESERVE_MOST as no-op on Windows
f2430a1
It's causing linker errors with node.lib in node-gyp and potentially
breaks other 3rd party tools

PR-URL: nodejs/node#56238
Refs: nodejs/node#55784
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs/node#55014
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Feb 5, 2025
@StefanStojanovic @targos
deps: fix FP16 bitcasts.h
adf80de
PR-URL: nodejs#53134
Refs: nodejs#52809
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#55014
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Mar 24, 2025
2025-05-06, Version 24.0.0 (Current) #57609
Merged
@jazelly jazelly mentioned this pull request May 13, 2025
[v22.x backport] src: initialize privateSymbols for per_context #58217
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ronag ronag ronag left review comments
@mcollina mcollina mcollina approved these changes
@anonrig anonrig anonrig approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. build Issues and PRs related to Node.js builds or CI infrastructure. commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. dependencies PRs that add, update, or configure Node.js dependencies. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

11 participants

@targos @nodejs-github-bot @avivkeller @ronag @bnoordhuis @richardlau @mcollina @anonrig @StefanStojanovic @o- @etiennep-chromium
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.