Skip to content

stream: fix Writable.destroy performance regression - #50409

Closed
ronag wants to merge 1 commit into
nodejs:mainfrom
nxtedition:writable-destroy-pref
Closed

stream: fix Writable.destroy performance regression#50409
ronag wants to merge 1 commit into
nodejs:mainfrom
nxtedition:writable-destroy-pref

Conversation

@ronag

Copy link
Copy Markdown
Member

No description provided.

@nodejs-github-bot

nodejs-github-bot commented Oct 26, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 26, 2023
@ronag
ronagforce-pushed the writable-destroy-pref branch from fe83419 to c3403b4CompareOctober 26, 2023 13:53
@ronag

Copy link
Copy Markdown
MemberAuthor

@ronagronag added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Oct 26, 2023

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 26, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@rluvatonrluvaton added the performance Issues and PRs related to the performance of Node.js. label Oct 26, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ronagronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 27, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 28, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/50409
✔ Done loading data for nodejs/node/pull/50409
----------------------------------- PR info ------------------------------------
Title stream: fix Writable.destroy performance regression (#50409)
Author Robert Nagy (@ronag)
Branch ronag:writable-destroy-pref -> nodejs:main
Labels performance, author ready, needs-ci
Commits 1
- stream: fix Writable.destroy performance regression
Committers 1
- Robert Nagy PR-URL: https://github.com/nodejs/node/pull/50409
Reviewed-By: Raz Luvaton Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50409
Reviewed-By: Raz Luvaton Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum --------------------------------------------------------------------------------
ℹ This PR was created on Thu, 26 Oct 2023 13:53:21 GMT
✔ Approvals: 3
✔ - Raz Luvaton (@rluvaton): https://github.com/nodejs/node/pull/50409#pullrequestreview-1699727857
✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/50409#pullrequestreview-1699783397
✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/50409#pullrequestreview-1700754946
✘ Last GitHub CI failed
ℹ Last Benchmark CI on 2023-10-26T13:58:47Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1460/
ℹ Last Full PR CI on 2023-10-27T10:42:36Z: https://ci.nodejs.org/job/node-test-pull-request/55266/
- Querying data for job/node-test-pull-request/55266/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/6677348142

@nodejs-github-botnodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 28, 2023
@ronagronag added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 28, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 28, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/50409
✔ Done loading data for nodejs/node/pull/50409
----------------------------------- PR info ------------------------------------
Title stream: fix Writable.destroy performance regression (#50409)
Author Robert Nagy (@ronag)
Branch ronag:writable-destroy-pref -> nodejs:main
Labels performance, author ready, needs-ci
Commits 1
- stream: fix Writable.destroy performance regression
Committers 1
- Robert Nagy PR-URL: https://github.com/nodejs/node/pull/50409
Reviewed-By: Raz Luvaton Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50409
Reviewed-By: Raz Luvaton Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell --------------------------------------------------------------------------------
ℹ This PR was created on Thu, 26 Oct 2023 13:53:21 GMT
✔ Approvals: 4
✔ - Raz Luvaton (@rluvaton): https://github.com/nodejs/node/pull/50409#pullrequestreview-1699727857
✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/50409#pullrequestreview-1699783397
✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/50409#pullrequestreview-1700754946
✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/50409#pullrequestreview-1702808873
✘ Last GitHub CI failed
ℹ Last Benchmark CI on 2023-10-28T14:03:30Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1460/
ℹ Last Full PR CI on 2023-10-28T14:03:30Z: https://ci.nodejs.org/job/node-test-pull-request/55266/
- Querying data for job/node-test-pull-request/1460/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/6678407207

@nodejs-github-botnodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 28, 2023
@Trott

Copy link
Copy Markdown
Member

For whatever reason, this is failing the coverage-without-intl job over and over on GitHub Actions. I applied this change on top of current main (preserving commit authorship) and that passed over in #50478. So I'll run a full CI there. If the folks who approved this PR could also approve that one so it can land (and this one can be closed), that would be great.

@mcollina@jasnell@benjamingr@rluvaton

@Trott

Copy link
Copy Markdown
Member

#50478 is exactly this PR but rebased. CI is passing on that PR and it is ready to land, but needs one more approval. If someone who approved this PR would approve that one, we could add the commit-queue label to that PR and close this one.

@ronagronag closed this Oct 31, 2023
Trott pushed a commit to Trott/io.js that referenced this pull request Oct 31, 2023
Ref: nodejs#50409
PR-URL: nodejs#50478
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Nov 11, 2023
Ref: #50409
PR-URL: #50478
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Nov 14, 2023
Ref: #50409
PR-URL: #50478
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
Ref: #50409
PR-URL: #50478
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
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.commit-queue-failedAn error occurred while landing this pull request using GitHub Actions.needs-ciPRs that need a full CI run.performanceIssues and PRs related to the performance of Node.js.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@ronag@nodejs-github-bot@Trott@mcollina@jasnell@benjamingr@Uzlopak@rluvaton