X Tutup
The Wayback Machine - https://web.archive.org/web/20250814212343/https://github.com/nodejs/node/pull/59228
Skip to content

benchmark: remove deprecated _extend from benchmark #59228

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

Merged
merged 2 commits into from
Aug 11, 2025

Conversation

RafaelGSS
Copy link
Member

as titled

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. v8 engine Issues and PRs related to the V8 dependency. labels Jul 26, 2025
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
@RafaelGSS RafaelGSS force-pushed the remove-deprecated-extend branch from 1b08898 to 8d62018 Compare July 28, 2025 13:52
@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 28, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 2, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/59228
✔  Done loading data for nodejs/node/pull/59228
----------------------------------- PR info ------------------------------------
Title      benchmark: remove deprecated _extend from benchmark (#59228)
Author     Rafael Gonzaga <rafael.nunu@hotmail.com> (@RafaelGSS)
Branch     RafaelGSS:remove-deprecated-extend -> nodejs:main
Labels     v8 engine, benchmark, author ready
Commits    1
 - benchmark: remove deprecated _extend from benchmark
Committers 1
 - RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/59228
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/59228
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - benchmark: remove deprecated _extend from benchmark
   ℹ  This PR was created on Sat, 26 Jul 2025 17:54:21 GMT
   ✔  Approvals: 1
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/59228#pullrequestreview-3058409096
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/16697407158

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Aug 2, 2025
@RafaelGSS RafaelGSS added review wanted PRs that need reviews. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 5, 2025
@legendecas
Copy link
Member

Should the benchmark be removed entirely? The benchmark is comparing util._extend with Object.assign and {...} spread syntax, the later two are implemented in V8, and removing util._extend meaning that non of the cases are implemented in node.

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

I'd also be fine with removing the benchmark entirely

@RafaelGSS
Copy link
Member Author

Should the benchmark be removed entirely? The benchmark is comparing util._extend with Object.assign and {...} spread syntax, the later two are implemented in V8, and removing util._extend meaning that non of the cases are implemented in node.

That's right. Also, this kind of benchmark is completely useless since JIT will take place. That's more suitable on https://github.com/RafaelGSS/nodejs-bench-operations.

Let me remove it.

@RafaelGSS
Copy link
Member Author

PTAL @legendecas @jasnell

@RafaelGSS RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 11, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 11, 2025
@nodejs-github-bot nodejs-github-bot merged commit 48aa9c7 into nodejs:main Aug 11, 2025
21 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 48aa9c7

RafaelGSS added a commit that referenced this pull request Aug 11, 2025
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: #59228
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
RafaelGSS added a commit that referenced this pull request Aug 12, 2025
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: #59228
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
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. benchmark Issues and PRs related to the benchmark subsystem. review wanted PRs that need reviews. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
X Tutup