Skip to content
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

[v14.x] fs: remove custom Buffer pool for streams #38397

Closed
wants to merge 1 commit into from

Conversation

targos
Copy link
Member

@targos targos commented Apr 25, 2021

The performance benefit of using a custom pool are negligable.
Furthermore, it causes problems with Workers and transferrable.
Rather than further adding complexity for compat with Workers,
just remove the pooling logic.

Refs: #33880 (comment)
Fixes: #31733

PR-URL: #33981

The performance benefit of using a custom pool are negligable.
Furthermore, it causes problems with Workers and transferrable.
Rather than further adding complexity for compat with Workers,
just remove the pooling logic.

Refs: nodejs#33880 (comment)
Fixes: nodejs#31733

PR-URL: nodejs#33981
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@targos
Copy link
Member Author

targos commented Apr 25, 2021

@ronag

@nodejs-github-bot
Copy link
Collaborator

@targos targos added fs Issues and PRs related to the fs subsystem / file system. stream Issues and PRs related to the stream subsystem. v14.x labels Apr 25, 2021
@nodejs-github-bot
Copy link
Collaborator

targos pushed a commit that referenced this pull request Apr 26, 2021
The performance benefit of using a custom pool are negligable.
Furthermore, it causes problems with Workers and transferrable.
Rather than further adding complexity for compat with Workers,
just remove the pooling logic.

Refs: #33880 (comment)
Fixes: #31733

PR-URL: #33981
Backport-PR-URL: #38397
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@targos
Copy link
Member Author

targos commented Apr 26, 2021

Landed in 443cace

@targos targos closed this Apr 26, 2021
@targos targos deleted the backport-33981-v14.x branch April 26, 2021 07:17
targos added a commit that referenced this pull request May 18, 2021
Refs: https://github.com/unicode-org/icu/releases/tag/release-69-1

Backport-PR-URL: #38397
PR-URL: #38178
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos added a commit that referenced this pull request Jun 11, 2021
Refs: https://github.com/unicode-org/icu/releases/tag/release-69-1

Backport-PR-URL: #38397
PR-URL: #38178
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants