Skip to content

Add SourceMap Benchmark #58125

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

Conversation

miguelmarcondesf
Copy link
Contributor

@miguelmarcondesf miguelmarcondesf commented May 2, 2025

Add SourceMap Benchmark

This PR introduces a new benchmark suite for the lib/internal/source_map module to help evaluate and explore the performance characteristics of the SourceMap.

I'm still learning how everything fits together, but this is a first step toward understanding where small improvements could improve performance.

cc @BridgeAR

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/performance
  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label May 2, 2025
@miguelmarcondesf miguelmarcondesf force-pushed the performance-improvements-sourcemap branch from 5e438af to 4147ff3 Compare May 2, 2025 20:07
@miguelmarcondesf miguelmarcondesf changed the title Performance improvements sourcemap Add SourceMap Benchmark May 2, 2025
@miguelmarcondesf
Copy link
Contributor Author

Local running with n: 1e5 for all cases:

image

@miguelmarcondesf miguelmarcondesf marked this pull request as ready for review May 3, 2025 21:12
@anonrig
Copy link
Member

anonrig commented May 3, 2025

You need to add a test for these benchmarks. You can find similar ones in test folder.

@miguelmarcondesf
Copy link
Contributor Author

You need to add a test for these benchmarks. You can find similar ones in test folder.

Thank you, just pushed that!

@miguelmarcondesf miguelmarcondesf force-pushed the performance-improvements-sourcemap branch from 5749fce to bc9fbe9 Compare May 6, 2025 16:42
Copy link

codecov bot commented May 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.09%. Comparing base (04cb572) to head (c071304).
Report is 93 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58125      +/-   ##
==========================================
+ Coverage   90.07%   90.09%   +0.02%     
==========================================
  Files         640      640              
  Lines      188262   188262              
  Branches    36912    36911       -1     
==========================================
+ Hits       169580   169623      +43     
+ Misses      11399    11363      -36     
+ Partials     7283     7276       -7     

see 37 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.

@geeksilva97 geeksilva97 requested a review from H4ad May 20, 2025 14:16
@miguelmarcondesf miguelmarcondesf force-pushed the performance-improvements-sourcemap branch 2 times, most recently from f1040cd to 421ae12 Compare June 5, 2025 18:55
Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the change! Overall LGTM % nits.

@miguelmarcondesf miguelmarcondesf force-pushed the performance-improvements-sourcemap branch from 02b5acc to 7eab302 Compare June 18, 2025 23:50
@legendecas legendecas added request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 20, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 20, 2025
@nodejs-github-bot
Copy link
Collaborator

@miguelmarcondesf miguelmarcondesf force-pushed the performance-improvements-sourcemap branch from c7232d1 to ac7a40e Compare June 20, 2025 20:30
@miguelmarcondesf miguelmarcondesf force-pushed the performance-improvements-sourcemap branch from ac7a40e to c071304 Compare June 23, 2025 20:37
@RafaelGSS RafaelGSS added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 1, 2025
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 1, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/58125
✔  Done loading data for nodejs/node/pull/58125
----------------------------------- PR info ------------------------------------
Title      Add SourceMap Benchmark (#58125)
Author     Miguel Marcondes Filho <[email protected]> (@miguelmarcondesf)
Branch     miguelmarcondesf:performance-improvements-sourcemap -> nodejs:main
Labels     author ready, needs-ci, commit-queue-squash
Commits    3
 - benchmark: add source map and source map cache
 - benchmark: use fixtures for sourcemap cache
 - benchmark: use fixtures and avoid dead code for sourcemap
Committers 1
 - Miguel Marcondes <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/58125
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Edy Silva <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/58125
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Edy Silva <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 02 May 2025 17:48:50 GMT
   ✔  Approvals: 4
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/58125#pullrequestreview-2946108239
   ✔  - Edy Silva (@geeksilva97): https://github.com/nodejs/node/pull/58125#pullrequestreview-2950130654
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/58125#pullrequestreview-2952013937
   ✔  - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs/node/pull/58125#pullrequestreview-2975274650
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2025-06-23T13:44:33Z: https://ci.nodejs.org/job/node-test-pull-request/67620/
   ⚠  Commits were pushed after the last Full PR CI run:
   ⚠  - benchmark: add source map and source map cache
   ⚠  - benchmark: use fixtures for sourcemap cache
   ⚠  - benchmark: use fixtures and avoid dead code for sourcemap
- Querying data for job/node-test-pull-request/67620/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/15999945215

@geeksilva97 geeksilva97 added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 1, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 1, 2025
@nodejs-github-bot

This comment was marked as outdated.

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2025
@nodejs-github-bot

This comment was marked as outdated.

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 4, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 4, 2025
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@RafaelGSS RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 7, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 7, 2025
@nodejs-github-bot nodejs-github-bot merged commit 9c025a9 into nodejs:main Jul 7, 2025
80 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 9c025a9

RafaelGSS pushed a commit that referenced this pull request Jul 8, 2025
PR-URL: #58125
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Edy Silva <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants