stream: fix Writable.destroy performance regression #50409
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review requested:
|
ronag
force-pushed
the
writable-destroy-pref
branch
from
fe83419
to
c3403b4
Compare
rluvaton
approved these changes
Oct 26, 2023
ronag
added
request-ci
Uzlopak
approved these changes
Oct 26, 2023
github-actions
bot
removed
the
request-ci
rluvaton
added
the
performance
benjamingr
approved these changes
Oct 26, 2023
19 tasks
ronag
added
the
commit-queue
22 tasks
nodejs-github-bot
removed
the
commit-queue
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/.ncuhttps://github.com/nodejs/node/actions/runs/6677348142 |
nodejs-github-bot
added
the
commit-queue-failed
jasnell
approved these changes
Oct 28, 2023
ronag
added
commit-queue
nodejs-github-bot
removed
the
commit-queue
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/.ncuhttps://github.com/nodejs/node/actions/runs/6678407207 |
nodejs-github-bot
added
the
commit-queue-failed
This was referenced Oct 29, 2023
Trott
pushed a commit
to Trott/io.js
that referenced
this pull request
Oct 30, 2023
|
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. |
17 tasks
|
#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 |
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>
alexfernandez
pushed a commit
to alexfernandez/node
that referenced
this pull request
Nov 1, 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>
anonrig
pushed a commit
to anonrig/node
that referenced
this pull request
Nov 9, 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.