Skip to content

Conversation

@anonrig
Copy link
Member

v8 fast path was not triggering before this. I've updated the implementation and added proper test for it.

cc @nodejs/cpp-reviewers @nodejs/performance

@anonriganonrig requested review from jasnell, ronag and targosApril 27, 2025 17:45
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/security-wg

@nodejs-github-botnodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 27, 2025
@anonriganonrigforce-pushed the yagiz/internal-module-stat branch from b563e9e to aa493c2CompareApril 27, 2025 17:56
@anonriganonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Apr 27, 2025
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 27, 2025
@nodejs-github-bot
Copy link
Collaborator

@codecov
Copy link

codecovbot commented Apr 27, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.19%. Comparing base (e0cf8ae) to head (aa493c2).
⚠️ Report is 1036 commits behind head on main.

Files with missing linesPatch %Lines
src/node_file.cc57.14%0 Missing and 3 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #58054 +/- ## ======================================= Coverage 90.18% 90.19% ======================================= Files 630 630 Lines 186393 186392 -1 Branches 36595 36592 -3 ======================================= + Hits 168103 168119 +16 + Misses 11090 11089 -1 + Partials 7200 7184 -16 
Files with missing linesCoverage Δ
lib/fs.js98.26% <100.00%> (ø)
lib/internal/fs/promises.js98.24% <100.00%> (ø)
lib/internal/modules/cjs/loader.js98.26% <100.00%> (ø)
lib/internal/modules/esm/resolve.js96.17% <ø> (-0.01%)⬇️
lib/internal/modules/package_json_reader.js99.36% <ø> (-0.01%)⬇️
src/node_external_reference.h100.00% <ø> (ø)
src/node_file.cc76.94% <57.14%> (+0.21%)⬆️

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@anonriganonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 27, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 29, 2025
@nodejs-github-botnodejs-github-bot merged commit 5ed1bcb into nodejs:mainApr 29, 2025
64 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 5ed1bcb

@nodejs-github-botnodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label May 1, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/58054 ✔ Done loading data for nodejs/node/pull/58054 ----------------------------------- PR info ------------------------------------ Title src: fix internalModuleStat v8 fast path (#58054) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch anonrig:yagiz/internal-module-stat -> nodejs:main Labels lib / src, author ready, needs-ci Commits 1 - src: fix internalModuleStat v8 fast path Committers 1 - Yagiz Nizipli <[email protected]> PR-URL: https://github.com/nodejs/node/pull/58054 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Robert Nagy <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/58054 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Robert Nagy <[email protected]> -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 27 Apr 2025 17:45:21 GMT ✔ Approvals: 2 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/58054#pullrequestreview-2797706038 ✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/58054#pullrequestreview-2797709654 ⚠ This PR was merged on Tue, 29 Apr 2025 17:53:13 GMT ✔ Last GitHub CI successful ℹ Last Full PR CI on 2025-04-27T18:10:22Z: https://ci.nodejs.org/job/node-test-pull-request/66486/ - Querying data for job/node-test-pull-request/66486/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/14777283946

RafaelGSS pushed a commit that referenced this pull request May 1, 2025
PR-URL: #58054 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
PR-URL: #58054 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
Renegade334 pushed a commit to Renegade334/node that referenced this pull request Jul 13, 2025
Renegade334 added a commit to Renegade334/node that referenced this pull request Jul 13, 2025
PR-URL: nodejs#58054 Backport-PR-URL: https://github.com/nodejs/node/pull/XXXXX Co-authored-by: René <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
Renegade334 pushed a commit to Renegade334/node that referenced this pull request Jul 14, 2025
PR-URL: nodejs#58054 Backport-PR-URL: nodejs#59065 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
Renegade334 pushed a commit to Renegade334/node that referenced this pull request Jul 16, 2025
PR-URL: nodejs#58054 Backport-PR-URL: nodejs#59065 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Robert Nagy <[email protected]>
@aduh95aduh95 added the backport-open-v22.x Indicate that the PR has an open backport label Jul 21, 2025
aduh95 pushed a commit to Renegade334/node that referenced this pull request Jul 28, 2025
PR-URL: nodejs#58054 Backport-PR-URL: nodejs#59065 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
codebytere added a commit to electron/electron that referenced this pull request Aug 4, 2025
codebytere added a commit to electron/electron that referenced this pull request Aug 4, 2025
jkleinsc pushed a commit to electron/electron that referenced this pull request Aug 4, 2025
* chore: bump node in DEPS to v22.18.0 * crypto: fix inclusion of OPENSSL_IS_BORINGSSL define nodejs/node#58845 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58960 * permission: propagate permission model flags on spawn nodejs/node#58853 * esm: syncify default path of ModuleLoader\.load nodejs/node#57419 * src: remove fast API for InternalModuleStat nodejs/node#58489 * src: simplify adding fast APIs to ExternalReferenceRegistry nodejs/node#58896 * chore: fixup patch indices * src: fix internalModuleStat v8 fast path nodejs/node#58054 * test: add tests to ensure that node.1 is kept in sync with cli.md nodejs/node#58878 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58942 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <[email protected]>
codebytere added a commit to electron/electron that referenced this pull request Aug 5, 2025
* chore: bump node in DEPS to v22.18.0 * crypto: fix inclusion of OPENSSL_IS_BORINGSSL define nodejs/node#58845 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58960 * permission: propagate permission model flags on spawn nodejs/node#58853 * esm: syncify default path of ModuleLoader\.load nodejs/node#57419 * src: remove fast API for InternalModuleStat nodejs/node#58489 * src: simplify adding fast APIs to ExternalReferenceRegistry nodejs/node#58896 * chore: fixup patch indices * src: fix internalModuleStat v8 fast path nodejs/node#58054 * test: add tests to ensure that node.1 is kept in sync with cli.md nodejs/node#58878 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58942 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <[email protected]>
codebytere added a commit to electron/electron that referenced this pull request Aug 5, 2025
chore: bump node to v22.18.0 (main) (#47937) * chore: bump node in DEPS to v22.18.0 * crypto: fix inclusion of OPENSSL_IS_BORINGSSL define nodejs/node#58845 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58960 * permission: propagate permission model flags on spawn nodejs/node#58853 * esm: syncify default path of ModuleLoader\.load nodejs/node#57419 * src: remove fast API for InternalModuleStat nodejs/node#58489 * src: simplify adding fast APIs to ExternalReferenceRegistry nodejs/node#58896 * chore: fixup patch indices * src: fix internalModuleStat v8 fast path nodejs/node#58054 * test: add tests to ensure that node.1 is kept in sync with cli.md nodejs/node#58878 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58942 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <[email protected]>
@richardlaurichardlau added backported-to-v22.x PRs backported to the v22.x-staging branch. and removed backport-open-v22.x Indicate that the PR has an open backport labels Sep 19, 2025
kigh-ota pushed a commit to kigh-ota/electron that referenced this pull request Sep 30, 2025
* chore: bump node in DEPS to v22.18.0 * crypto: fix inclusion of OPENSSL_IS_BORINGSSL define nodejs/node#58845 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58960 * permission: propagate permission model flags on spawn nodejs/node#58853 * esm: syncify default path of ModuleLoader\.load nodejs/node#57419 * src: remove fast API for InternalModuleStat nodejs/node#58489 * src: simplify adding fast APIs to ExternalReferenceRegistry nodejs/node#58896 * chore: fixup patch indices * src: fix internalModuleStat v8 fast path nodejs/node#58054 * test: add tests to ensure that node.1 is kept in sync with cli.md nodejs/node#58878 * crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4 nodejs/node#58942 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.backported-to-v22.xPRs backported to the v22.x-staging branch.commit-queue-failedAn error occurred while landing this pull request using GitHub Actions.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@anonrig@nodejs-github-bot@jasnell@ronag@RafaelGSS@richardlau@aduh95