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 — lib: improve esm resolve performance by anonrig · Pull Request #46652 · nodejs/node

Conversation

@anonrig

@anonrig anonrig commented Feb 14, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

I couldn't find a benchmark or had the time to write it, but in theory, it should be a lot faster.

cc @nodejs/performance @nodejs/loaders

Refs: nodejs/performance#39

Sorry, something went wrong.

Uh oh!

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

MoLow, lin72h, benjamingr, and metcoder95 reacted with thumbs up emoji tonivj5 and bricss reacted with eyes emoji
All reactions
  • 👍 4 reactions
  • 👀 2 reactions
@anonrig anonrig added the performance Issues and PRs related to the performance of Node.js. label Feb 14, 2023
@nodejs-github-bot

nodejs-github-bot commented Feb 14, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/modules
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 esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Feb 14, 2023
GeoffreyBooth
GeoffreyBooth previously requested changes Feb 14, 2023
View reviewed changes

@GeoffreyBooth GeoffreyBooth 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.

On quick glance this seems fine, but we need to land #44710 first. Just putting a hold on this until that gets landed.

Sorry, something went wrong.

Uh oh!

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

All reactions
@GeoffreyBooth GeoffreyBooth added the loaders Issues and PRs related to ES module loaders. label Feb 14, 2023
targos
targos reviewed Feb 14, 2023
View reviewed changes
Comment thread lib/internal/modules/esm/resolve.js Resolved

Uh oh!

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

@anonrig
anonrig force-pushed the perf-improve-esm-resolve branch from d5f008b to 76fb5d6 Compare February 14, 2023 16:38
GeoffreyBooth
GeoffreyBooth reviewed Feb 14, 2023
View reviewed changes

@GeoffreyBooth GeoffreyBooth 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.

I chatted with @anonrig and they offered to rebase #44710 on top of this if this is ready first, which seems fine to me. I’d still like to ensure this doesn’t slow down #44710 landing though; the rebased version of that branch shouldn’t have any new test failures.

Sorry, something went wrong.

Uh oh!

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

All reactions
@GeoffreyBooth
GeoffreyBooth self-requested a review February 14, 2023 16:45
@GeoffreyBooth
GeoffreyBooth dismissed their stale review February 14, 2023 16:46

Discussion

aduh95
aduh95 approved these changes Feb 14, 2023
View reviewed changes

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

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.

I'd like to see a benchmark for this, but the theory looks solid. This can land before or after the off-thread PR, the conflicts should be minimal :)

Sorry, something went wrong.

Uh oh!

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

All reactions
@arcanis

arcanis commented Feb 14, 2023

Copy link
Copy Markdown
Contributor

Shouldn't it be factorized with the cjs/loader implementation (note that since it isn't in this PR, the esm implementation doesn't has a cache)?

All reactions

Sorry, something went wrong.

Uh oh!

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

@anonrig

anonrig commented Feb 14, 2023

Copy link
Copy Markdown
Member Author

Shouldn't it be factorized with the cjs/loader implementation (note that since it isn't in this PR, the esm implementation doesn't has a cache)?

I agree, but I prefer to do it in a different pull request, and keep this minimal.

All reactions

Sorry, something went wrong.

Uh oh!

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

@mscdex mscdex added the needs-citgm PRs that need a CITGM CI run. label Feb 14, 2023
targos
targos approved these changes Feb 15, 2023
View reviewed changes
mcollina
mcollina approved these changes Feb 15, 2023
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
RafaelGSS
RafaelGSS approved these changes Feb 15, 2023
View reviewed changes
tony-go
tony-go approved these changes Feb 16, 2023
View reviewed changes
@anonrig anonrig 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 Feb 17, 2023
@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 Feb 17, 2023
@nodejs-github-bot

nodejs-github-bot commented Feb 17, 2023

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

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

This was referenced Feb 18, 2023
CI Reliability 2023-02-18 nodejs/reliability#499
Open
CI Reliability 2023-02-19 nodejs/reliability#500
Open
jasnell
jasnell approved these changes Feb 20, 2023
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Feb 21, 2023

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

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

@github-actions github-actions Bot mentioned this pull request Feb 23, 2023
CI Reliability 2023-02-23 nodejs/reliability#504
Open
31 tasks
UlisesGascon
UlisesGascon approved these changes Mar 12, 2023
View reviewed changes
@GeoffreyBooth

GeoffreyBooth commented Mar 13, 2023

Copy link
Copy Markdown
Member

Before we land this shouldn’t there be some attempt at confirming that this does, in fact, make things faster? And especially not slower. Even just running a benchmark on someone’s local machine.

abenhamdine, benjamingr, and ErickWendel reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

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

benjamingr
benjamingr previously requested changes Mar 13, 2023
View reviewed changes

@benjamingr benjamingr left a comment •
edited
Loading

Uh oh!

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

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.

To strengthen @GeoffreyBooth 's point
Making sure benchmarks are run in some form before this lands. Please dismiss my review when that happens @anonrig (or whomever)

Sorry, something went wrong.

Uh oh!

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

GeoffreyBooth and ljharb reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@anonrig

anonrig commented Apr 23, 2023 •
edited by aduh95
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

@GeoffreyBooth Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1332/

Results
                                                                              confidence improvement accuracy (*)    (**)   (***)
esm/esm-loader-defaultResolve.js specifier='./relative-existing.js' n=1000             *      9.41 %       ±8.24% ±10.97% ±14.29%
esm/esm-loader-defaultResolve.js specifier='./relative-nonexistent.js' n=1000        ***     15.64 %       ±6.89%  ±9.20% ±12.03%
esm/esm-loader-defaultResolve.js specifier='node:os' n=1000                                  -0.28 %       ±8.78% ±11.68% ±15.20%
esm/esm-loader-defaultResolve.js specifier='node:prefixed-nonexistent' n=1000                -3.20 %       ±7.41%  ±9.87% ±12.85%
esm/esm-loader-defaultResolve.js specifier='unprefixed-existing' n=1000               **     13.37 %       ±8.00% ±10.65% ±13.88%
esm/esm-loader-defaultResolve.js specifier='unprefixed-nonexistent' n=1000             *      9.01 %       ±7.15%  ±9.52% ±12.39%
GeoffreyBooth and StefanStojanovic reacted with hooray emoji
All reactions
  • 🎉 2 reactions

Sorry, something went wrong.

Uh oh!

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

aduh95
aduh95 approved these changes Apr 23, 2023
View reviewed changes
@aduh95 aduh95 added the commit-queue PRs queued for automated landing through the Commit Queue. label Apr 23, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Apr 23, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 3e9ed7e into nodejs:main Apr 23, 2023
@nodejs-github-bot

nodejs-github-bot commented Apr 23, 2023

Copy link
Copy Markdown
Collaborator

Landed in 3e9ed7e

All reactions

Sorry, something went wrong.

Uh oh!

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

This was referenced Apr 24, 2023
CI Reliability 2023-04-24 nodejs/reliability#545
Open
CI Reliability 2023-04-25 nodejs/reliability#546
Open
CI Reliability 2023-04-26 nodejs/reliability#547
Open
CI Reliability 2023-04-27 nodejs/reliability#548
Open
targos pushed a commit that referenced this pull request May 2, 2023
@anonrig @targos
lib: improve esm resolve performance
21ff331
PR-URL: #46652
Refs: nodejs/performance#39
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Erick Wendel <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@targos targos mentioned this pull request May 2, 2023
v20.1.0 release proposal #47820
Merged
@danielleadams danielleadams added the backport-requested-v18.x label Jul 4, 2023
codebytere added a commit to electron/electron that referenced this pull request Nov 6, 2023
@codebytere
chore: address changes to CJS/ESM loading
21d21f3
* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)
targos pushed a commit that referenced this pull request Nov 10, 2023
@anonrig @targos
lib: improve esm resolve performance
9a4b57d
PR-URL: #46652
Refs: nodejs/performance#39
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Erick Wendel <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
codebytere added a commit to electron/electron that referenced this pull request Nov 14, 2023
@codebytere
chore: address changes to CJS/ESM loading
c1ec1eb
* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)
codebytere added a commit to electron/electron that referenced this pull request Nov 15, 2023
@codebytere
chore: address changes to CJS/ESM loading
c7004c1
* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)
codebytere added a commit to electron/electron that referenced this pull request Nov 16, 2023
@codebytere
chore: address changes to CJS/ESM loading
b69759d
* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)
codebytere added a commit to electron/electron that referenced this pull request Nov 21, 2023
@codebytere
chore: address changes to CJS/ESM loading
efaecec
* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)
codebytere added a commit to electron/electron that referenced this pull request Nov 22, 2023
@codebytere
chore: address changes to CJS/ESM loading
42cb35a
* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)
codebytere added a commit to electron/electron that referenced this pull request Nov 28, 2023
@codebytere
chore: address changes to CJS/ESM loading
10fa7c0
* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)
@targos targos mentioned this pull request Nov 28, 2023
v18.19.0 release proposal #50953
Merged
codebytere added a commit to electron/electron that referenced this pull request Nov 29, 2023
@codebytere
chore: address changes to CJS/ESM loading
f9e4508
* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)
jkleinsc pushed a commit to electron/electron that referenced this pull request Nov 30, 2023
@codebytere @VerteDinde @MarshallOfSound
chore: upgrade to Node.js v20 (#40545)
9c94fd7
* chore: upgrade to Node.js v20

* src: allow embedders to override NODE_MODULE_VERSION

nodejs/node#49279

* src: fix missing trailing ,

nodejs/node#46909

* src,tools: initialize cppgc

nodejs/node#45704

* tools: allow passing absolute path of config.gypi in js2c

nodejs/node#49162

* tools: port js2c.py to C++

nodejs/node#46997

* doc,lib: disambiguate the old term, NativeModule

nodejs/node#45673

* chore: fixup Node.js BSSL tests

* nodejs/node#49492
* nodejs/node#44498

* deps: upgrade to libuv 1.45.0

nodejs/node#48078

* deps: update V8 to 10.7

nodejs/node#44741

* test: use gcUntil() in test-v8-serialize-leak

nodejs/node#49168

* module: make CJS load from ESM loader

nodejs/node#47999

* src: make BuiltinLoader threadsafe and non-global

nodejs/node#45942

* chore: address changes to CJS/ESM loading

* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)

* bootstrap: optimize modules loaded in the built-in snapshot

nodejs/node#45849

* test: mark test-runner-output as flaky

nodejs/node#49854

* lib: lazy-load deps in modules/run_main.js

nodejs/node#45849

* url: use private properties for brand check

nodejs/node#46904

* test: refactor `test-node-output-errors`

nodejs/node#48992

* assert: deprecate callTracker

nodejs/node#47740

* src: cast v8::Object::GetInternalField() return value to v8::Value

nodejs/node#48943

* test: adapt test-v8-stats for V8 update

nodejs/node#45230

* tls: ensure TLS Sockets are closed if the underlying wrap closes

nodejs/node#49327

* test: deflake test-tls-socket-close

nodejs/node#49575

* net: fix crash due to simultaneous close/shutdown on JS Stream Sockets

nodejs/node#49400

* net: use asserts in JS Socket Stream to catch races in future

nodejs/node#49400

* lib: fix BroadcastChannel initialization location

nodejs/node#46864

* src: create BaseObject with node::Realm

nodejs/node#44348

* src: implement DataQueue and non-memory resident Blob

nodejs/node#45258

* sea: add support for V8 bytecode-only caching

nodejs/node#48191

* chore: fixup patch indices

* gyp: put filenames in variables

nodejs/node#46965

* build: modify js2c.py into GN executable

* fix: (WIP) handle string replacement of fs -> original-fs

* [v20.x] backport vm-related memory fixes

nodejs/node#49874

* src: make BuiltinLoader threadsafe and non-global

nodejs/node#45942

* src: avoid copying string in fs_permission

nodejs/node#47746

* look upon my works ye mighty

and dispair

* chore: patch cleanup

* [api] Remove AllCan Read/Write

https://chromium-review.googlesource.com/c/v8/v8/+/5006387

* fix: missing include for NODE_EXTERN

* chore: fixup patch indices

* fix: fail properly when js2c fails in Node.js

* build: fix js2c root_gen_dir

* fix: lib/fs.js -> lib/original-fs.js

* build: fix original-fs file xforms

* fixup! module: make CJS load from ESM loader

* build: get rid of CppHeap for now

* build: add patch to prevent extra fs lookup on esm load

* build: greatly simplify js2c modifications

Moves our original-fs modifications back into a super simple python script action, wires up the output of that action into our call to js2c

* chore: update to handle moved internal/modules/helpers file

* test: update @types/node test

* feat: enable preventing cppgc heap creation

* feat: optionally prevent calling V8::EnableWebAssemblyTrapHandler

* fix: no cppgc initialization in the renderer

* gyp: put filenames in variables

nodejs/node#46965

* test: disable single executable tests

* fix: nan tests failing on node headers missing file

* tls,http2: send fatal alert on ALPN mismatch

nodejs/node#44031

* test: disable snapshot tests

* nodejs/node#47887
* nodejs/node#49684
* nodejs/node#44193

* build: use deps/v8 for v8/tools

Node.js hard depends on these in their builtins

* test: fix edge snapshot stack traces

nodejs/node#49659

* build: remove js2c //base dep

* build: use electron_js2c_toolchain to build node_js2c

* fix: don't create SafeSet outside packageResolve

Fixes failure in parallel/test-require-delete-array-iterator:

=== release test-require-delete-array-iterator ===
Path: parallel/test-require-delete-array-iterator
node:internal/per_context/primordials:426
    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
                     ^

TypeError: object is not iterable (cannot read property Symbol(Symbol.iterator))
    at new Set (<anonymous>)
    at new SafeSet (node:internal/per_context/primordials:426:22)

* fix: failing crashReporter tests on Linux

These were failing because our change from node::InitializeNodeWithArgs to
node::InitializeOncePerProcess meant that we now inadvertently called
PlatformInit, which reset signal handling. This meant that our intentional
crash function ElectronBindings::Crash no longer worked and the renderer process
no longer crashed when process.crash() was called. We don't want to use Node.js'
default signal handling in the renderer process, so we disable it by passing
kNoDefaultSignalHandling to node::InitializeOncePerProcess.

* build: only create cppgc heap on non-32 bit platforms

* chore: clean up util:CompileAndCall

* src: fix compatility with upcoming V8 12.1 APIs

nodejs/node#50709

* fix: use thread_local BuiltinLoader

* chore: fixup v8 patch indices

---------

Co-authored-by: Keeley Hammond <[email protected]>
Co-authored-by: Samuel Attard <[email protected]>
MrHuangJser pushed a commit to MrHuangJser/electron that referenced this pull request Dec 11, 2023
@codebytere @VerteDinde
@MarshallOfSound MrHuangJser
chore: upgrade to Node.js v20 (electron#40545)
34651d1
* chore: upgrade to Node.js v20

* src: allow embedders to override NODE_MODULE_VERSION

nodejs/node#49279

* src: fix missing trailing ,

nodejs/node#46909

* src,tools: initialize cppgc

nodejs/node#45704

* tools: allow passing absolute path of config.gypi in js2c

nodejs/node#49162

* tools: port js2c.py to C++

nodejs/node#46997

* doc,lib: disambiguate the old term, NativeModule

nodejs/node#45673

* chore: fixup Node.js BSSL tests

* nodejs/node#49492
* nodejs/node#44498

* deps: upgrade to libuv 1.45.0

nodejs/node#48078

* deps: update V8 to 10.7

nodejs/node#44741

* test: use gcUntil() in test-v8-serialize-leak

nodejs/node#49168

* module: make CJS load from ESM loader

nodejs/node#47999

* src: make BuiltinLoader threadsafe and non-global

nodejs/node#45942

* chore: address changes to CJS/ESM loading

* module: make CJS load from ESM loader (nodejs/node#47999)
* lib: improve esm resolve performance (nodejs/node#46652)

* bootstrap: optimize modules loaded in the built-in snapshot

nodejs/node#45849

* test: mark test-runner-output as flaky

nodejs/node#49854

* lib: lazy-load deps in modules/run_main.js

nodejs/node#45849

* url: use private properties for brand check

nodejs/node#46904

* test: refactor `test-node-output-errors`

nodejs/node#48992

* assert: deprecate callTracker

nodejs/node#47740

* src: cast v8::Object::GetInternalField() return value to v8::Value

nodejs/node#48943

* test: adapt test-v8-stats for V8 update

nodejs/node#45230

* tls: ensure TLS Sockets are closed if the underlying wrap closes

nodejs/node#49327

* test: deflake test-tls-socket-close

nodejs/node#49575

* net: fix crash due to simultaneous close/shutdown on JS Stream Sockets

nodejs/node#49400

* net: use asserts in JS Socket Stream to catch races in future

nodejs/node#49400

* lib: fix BroadcastChannel initialization location

nodejs/node#46864

* src: create BaseObject with node::Realm

nodejs/node#44348

* src: implement DataQueue and non-memory resident Blob

nodejs/node#45258

* sea: add support for V8 bytecode-only caching

nodejs/node#48191

* chore: fixup patch indices

* gyp: put filenames in variables

nodejs/node#46965

* build: modify js2c.py into GN executable

* fix: (WIP) handle string replacement of fs -> original-fs

* [v20.x] backport vm-related memory fixes

nodejs/node#49874

* src: make BuiltinLoader threadsafe and non-global

nodejs/node#45942

* src: avoid copying string in fs_permission

nodejs/node#47746

* look upon my works ye mighty

and dispair

* chore: patch cleanup

* [api] Remove AllCan Read/Write

https://chromium-review.googlesource.com/c/v8/v8/+/5006387

* fix: missing include for NODE_EXTERN

* chore: fixup patch indices

* fix: fail properly when js2c fails in Node.js

* build: fix js2c root_gen_dir

* fix: lib/fs.js -> lib/original-fs.js

* build: fix original-fs file xforms

* fixup! module: make CJS load from ESM loader

* build: get rid of CppHeap for now

* build: add patch to prevent extra fs lookup on esm load

* build: greatly simplify js2c modifications

Moves our original-fs modifications back into a super simple python script action, wires up the output of that action into our call to js2c

* chore: update to handle moved internal/modules/helpers file

* test: update @types/node test

* feat: enable preventing cppgc heap creation

* feat: optionally prevent calling V8::EnableWebAssemblyTrapHandler

* fix: no cppgc initialization in the renderer

* gyp: put filenames in variables

nodejs/node#46965

* test: disable single executable tests

* fix: nan tests failing on node headers missing file

* tls,http2: send fatal alert on ALPN mismatch

nodejs/node#44031

* test: disable snapshot tests

* nodejs/node#47887
* nodejs/node#49684
* nodejs/node#44193

* build: use deps/v8 for v8/tools

Node.js hard depends on these in their builtins

* test: fix edge snapshot stack traces

nodejs/node#49659

* build: remove js2c //base dep

* build: use electron_js2c_toolchain to build node_js2c

* fix: don't create SafeSet outside packageResolve

Fixes failure in parallel/test-require-delete-array-iterator:

=== release test-require-delete-array-iterator ===
Path: parallel/test-require-delete-array-iterator
node:internal/per_context/primordials:426
    constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
                     ^

TypeError: object is not iterable (cannot read property Symbol(Symbol.iterator))
    at new Set (<anonymous>)
    at new SafeSet (node:internal/per_context/primordials:426:22)

* fix: failing crashReporter tests on Linux

These were failing because our change from node::InitializeNodeWithArgs to
node::InitializeOncePerProcess meant that we now inadvertently called
PlatformInit, which reset signal handling. This meant that our intentional
crash function ElectronBindings::Crash no longer worked and the renderer process
no longer crashed when process.crash() was called. We don't want to use Node.js'
default signal handling in the renderer process, so we disable it by passing
kNoDefaultSignalHandling to node::InitializeOncePerProcess.

* build: only create cppgc heap on non-32 bit platforms

* chore: clean up util:CompileAndCall

* src: fix compatility with upcoming V8 12.1 APIs

nodejs/node#50709

* fix: use thread_local BuiltinLoader

* chore: fixup v8 patch indices

---------

Co-authored-by: Keeley Hammond <[email protected]>
Co-authored-by: Samuel Attard <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
lib: improve esm resolve performance
3fe9d3f
PR-URL: nodejs/node#46652
Refs: nodejs/performance#39
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Erick Wendel <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
lib: improve esm resolve performance
dbc1b16
PR-URL: nodejs/node#46652
Refs: nodejs/performance#39
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Erick Wendel <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joyeecheung joyeecheung joyeecheung left review comments
@mcollina mcollina mcollina approved these changes
@jasnell jasnell jasnell approved these changes
@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes
@benjamingr benjamingr benjamingr approved these changes
@UlisesGascon UlisesGascon UlisesGascon approved these changes
@aduh95 aduh95 aduh95 approved these changes
@RafaelGSS RafaelGSS RafaelGSS approved these changes
@ErickWendel ErickWendel Awaiting requested review from ErickWendel
+2 more reviewers
@targos targos targos approved these changes
@tony-go tony-go tony-go approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders. needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run. performance Issues and PRs related to the performance of Node.js.

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.

18 participants

@anonrig @nodejs-github-bot @arcanis @GeoffreyBooth @aduh95 @richardlau @StefanStojanovic @mcollina @jasnell @benjamingr @targos @joyeecheung @UlisesGascon @ErickWendel @tony-go @RafaelGSS @mscdex @danielleadams
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.