Skip to content

buffer: move SlowBuffer to EOL #58008

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

buffer: move SlowBuffer to EOL #58008

wants to merge 2 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 24, 2025

SlowBuffer has been deprecated for many years now. Let's remove it.

@jasnell
buffer: move SlowBuffer to EOL
6be6ddc 
`SlowBuffer` has been deprecated for many years now. Let's remove it.
@jasnell jasnell added buffer Issues and PRs related to the buffer subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. deprecations Issues and PRs related to deprecations. labels Apr 24, 2025
@jasnell jasnell requested review from mcollina, anonrig and cjihrig April 24, 2025 14:52
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/performance
  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. config Issues or PRs related to the config subsystem needs-ci PRs that need a full CI run. labels Apr 24, 2025
jasnell
jasnell commented Apr 24, 2025
View reviewed changes
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/66438/

anonrig
anonrig approved these changes Apr 24, 2025
View reviewed changes
@marco-ippolito
Copy link
Member

With a quick github search it seems its safe to remove
https://github.com/search?q=SlowBuffer+language%3AJavaScript&type=code&l=JavaScript

@codecov Codecov
Copy link

codecov bot commented Apr 24, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.27%. Comparing base (b665127) to head (921e0a0).
Report is 16 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #58008   +/-   ##
=======================================
  Coverage   90.27%   90.27%           
=======================================
  Files         630      630           
  Lines      186158   186142   -16     
  Branches    36475    36474    -1     
=======================================
+ Hits       168047   168048    +1     
- Misses      10980    10982    +2     
+ Partials     7131     7112   -19
Files with missing lines Coverage Δ
lib/buffer.js 100.00% <100.00%> (ø)
src/node_options.cc 85.23% <ø> (-0.13%) ⬇️

... and 28 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.

@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 24, 2025
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
RaisinTen
RaisinTen approved these changes Apr 25, 2025
View reviewed changes
Copy link
Member

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

Looks good, just 1 comment about the tests

test/pummel/test-buffer-large-size-slowbuffer.js
Copy link
Member

@RaisinTen RaisinTen Apr 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This test can be deleted because now it is the same as test/pummel/test-buffer-large-size-buffer-alloc-unsafe-slow.js. Both of these were created in #57789 when a test containing same calls to SlowBuffer as well as allocUnsafeSlow was split into separate files for reliability.

mcollina
mcollina approved these changes Apr 25, 2025
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 26, 2025 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/66470/ 💛

@bjohansebas bjohansebas added the needs-citgm PRs that need a CITGM CI run. label Apr 26, 2025
@jasnell
Copy link
Member Author

jasnell commented Apr 27, 2025

Running citgm on this one just to be thorough but given the github codesearch results for "SlowBuffer" show no significant hits anywhere, I think it's safe to go ahead and merge. We can revert if citgm ends up showing anything but it would be an extreme surprise if it did.

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/

jasnell added a commit that referenced this pull request Apr 27, 2025
@jasnell
buffer: move SlowBuffer to EOL
647175e 
`SlowBuffer` has been deprecated for many years now. Let's remove it.

PR-URL: #58008
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@jasnell
Copy link
Member Author

jasnell commented Apr 27, 2025

Landed in 647175e

@jasnell jasnell closed this Apr 27, 2025
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@jasnell @RafaelGSS
buffer: move SlowBuffer to EOL
1c1c504 
`SlowBuffer` has been deprecated for many years now. Let's remove it.

PR-URL: #58008
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
RafaelGSS added a commit that referenced this pull request May 1, 2025
@RafaelGSS
2025-05-07, Version 24.0.0 (Current)
fd0f28c 
Semver-Major Commits:

assert,util:
  * (SEMVER-MAJOR) Revert "assert,util: revert recursive breaking change (Ruben Bridgewater) #57622
buffer:
  * (SEMVER-MAJOR) move SlowBuffer to EOL (James M Snell) #58008
  * (SEMVER-MAJOR) make `buflen` in integer range (zhenweijin) #51821
build:
  * (SEMVER-MAJOR) downgrade armv7 support to experimental (Michaël Zasso) #58071
  * (SEMVER-MAJOR) bump supported macOS version to 13.5 (Michaël Zasso) #57115
  * (SEMVER-MAJOR) increase minimum Xcode version to 16.1 (Michaël Zasso) #56824
  * (SEMVER-MAJOR) link V8 with atomic library (Michaël Zasso) #55014
  * (SEMVER-MAJOR) remove support for ppc 32-bit (Michaël Zasso) #55014
  * (SEMVER-MAJOR) reset embedder string to "-node.0" (Michaël Zasso) #55014
child_process:
  * (SEMVER-MAJOR) deprecate passing `args` to `spawn` and `execFile` (Daniel Venable) #57199
deps:
  * (SEMVER-MAJOR) V8: cherry-pick f915fa4c9f41 (Olivier Flückiger) #55014
  * (SEMVER-MAJOR) V8: cherry-pick 0d5d6e71bbb0 (Yagiz Nizipli) #55014
  * (SEMVER-MAJOR) V8: cherry-pick 0c11feeeca4a (Michaël Zasso) #55014
  * (SEMVER-MAJOR) define V8_PRESERVE_MOST as no-op on Windows (Stefan Stojanovic) #55014
  * (SEMVER-MAJOR) always define V8_NODISCARD as no-op (Michaël Zasso) #55014
  * (SEMVER-MAJOR) fix FP16 bitcasts.h (Stefan Stojanovic) #55014
  * (SEMVER-MAJOR) patch V8 to support compilation with MSVC (StefanStojanovic) #55014
  * (SEMVER-MAJOR) patch V8 to avoid duplicated zlib symbol (Michaël Zasso) #55014
  * (SEMVER-MAJOR) disable V8 concurrent sparkplug compilation (Michaël Zasso) #55014
  * (SEMVER-MAJOR) always define V8_EXPORT_PRIVATE as no-op (Michaël Zasso) #55014
  * (SEMVER-MAJOR) update V8 to 13.0.245.25 (Michaël Zasso) #55014
  * (SEMVER-MAJOR) upgrade npm to 11.0.0 (npm team) #56274
  * (SEMVER-MAJOR) update undici to 7.0.0 (Node.js GitHub Bot) #56070
fs:
  * (SEMVER-MAJOR) remove ability to call truncate with fd (Yagiz Nizipli) #57567
  * (SEMVER-MAJOR) deprecate passing invalid types in `fs.existsSync` (Carlos Espa) #55753
  * (SEMVER-MAJOR) runtime deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49686
  * (SEMVER-MAJOR) remove `dirent.path` (Antoine du Hamel) #55548
http:
  * (SEMVER-MAJOR) remove outgoingmessage _headers and _headersList (Yagiz Nizipli) #57551
http2:
  * (SEMVER-MAJOR) session tracking and graceful server close (Kushagra Pandey) #57586
lib:
  * (SEMVER-MAJOR) remove obsolete Cipher export (James M Snell) #57266
  * (SEMVER-MAJOR) unexpose six process bindings (Michaël Zasso) #57149
  * (SEMVER-MAJOR) make ALS default to AsyncContextFrame (Stephen Belanger) #55552
  * (SEMVER-MAJOR) runtime deprecate SlowBuffer (Rafael Gonzaga) #55175
net:
  * (SEMVER-MAJOR) make _setSimultaneousAccepts() end-of-life deprecated (Yagiz Nizipli) #57550
readline:
  * (SEMVER-MAJOR) add stricter validation for functions called after closed (Dario Piotrowicz) #57680
  * (SEMVER-MAJOR) fix unicode line separators being ignored (Dario Piotrowicz) #57591
repl:
  * (SEMVER-MAJOR) runtime deprecate instantiating without new (Aviv Keller) #54869
src:
  * (SEMVER-MAJOR) update GetForegroundTaskRunner override (Etienne Pierre-doray) #55014
  * (SEMVER-MAJOR) update NODE_MODULE_VERSION to 134 (Michaël Zasso) #55014
  * (SEMVER-MAJOR) drop --experimental-permission in favour of --permission (Rafael Gonzaga) #56240
  * (SEMVER-MAJOR) add async context frame to AsyncResource (Gerhard Stöbich) #56082
  * (SEMVER-MAJOR) nuke deprecated and un-used enum members in `OptionEnvvarSettings` (Juan José) #53079
stream:
  * (SEMVER-MAJOR) catch and forward error from dest.write (jakecastelli) #55270
test:
  * (SEMVER-MAJOR) disable fast API call count checks (Michaël Zasso) #55014
test_runner:
  * (SEMVER-MAJOR) remove promises returned by t.test() (Colin Ihrig) #56664
  * (SEMVER-MAJOR) remove promises returned by test() (Colin Ihrig) #56664
  * (SEMVER-MAJOR) automatically wait for subtests to finish (Colin Ihrig) #56664
timers:
  * (SEMVER-MAJOR) check for immediate instance in clearImmediate (Gürgün Dayıoğlu) #57069
  * (SEMVER-MAJOR) set several methods EOL (Yagiz Nizipli) #56966
tls:
  * (SEMVER-MAJOR) remove deprecated tls.createSecurePair (Jonas) #57361
  * (SEMVER-MAJOR) make server.prototype.setOptions end-of-life (Yagiz Nizipli) #57339
tools:
  * (SEMVER-MAJOR) update V8 gypfiles for 13.0 (Michaël Zasso) #55014
url:
  * (SEMVER-MAJOR) expose urlpattern as global (Jonas) #56950
  * (SEMVER-MAJOR) runtime deprecate url.parse (Yagiz Nizipli) #55017
zlib:
  * (SEMVER-MAJOR) deprecate classes usage without `new` (Yagiz Nizipli) #55718

PR-URL: #57609
Signed-off-by: RafaelGSS <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request May 1, 2025
Open
5 tasks
RafaelGSS added a commit that referenced this pull request May 1, 2025
@RafaelGSS
2025-05-07, Version 24.0.0 (Current)
18fb425 
Semver-Major Commits:

assert,util:
  * (SEMVER-MAJOR) Revert "assert,util: revert recursive breaking change (Ruben Bridgewater) #57622
buffer:
  * (SEMVER-MAJOR) move SlowBuffer to EOL (James M Snell) #58008
  * (SEMVER-MAJOR) make `buflen` in integer range (zhenweijin) #51821
build:
  * (SEMVER-MAJOR) downgrade armv7 support to experimental (Michaël Zasso) #58071
  * (SEMVER-MAJOR) bump supported macOS version to 13.5 (Michaël Zasso) #57115
  * (SEMVER-MAJOR) increase minimum Xcode version to 16.1 (Michaël Zasso) #56824
  * (SEMVER-MAJOR) link V8 with atomic library (Michaël Zasso) #55014
  * (SEMVER-MAJOR) remove support for ppc 32-bit (Michaël Zasso) #55014
  * (SEMVER-MAJOR) reset embedder string to "-node.0" (Michaël Zasso) #55014
child_process:
  * (SEMVER-MAJOR) deprecate passing `args` to `spawn` and `execFile` (Daniel Venable) #57199
deps:
  * (SEMVER-MAJOR) V8: cherry-pick f915fa4c9f41 (Olivier Flückiger) #55014
  * (SEMVER-MAJOR) V8: cherry-pick 0d5d6e71bbb0 (Yagiz Nizipli) #55014
  * (SEMVER-MAJOR) V8: cherry-pick 0c11feeeca4a (Michaël Zasso) #55014
  * (SEMVER-MAJOR) define V8_PRESERVE_MOST as no-op on Windows (Stefan Stojanovic) #55014
  * (SEMVER-MAJOR) always define V8_NODISCARD as no-op (Michaël Zasso) #55014
  * (SEMVER-MAJOR) fix FP16 bitcasts.h (Stefan Stojanovic) #55014
  * (SEMVER-MAJOR) patch V8 to support compilation with MSVC (StefanStojanovic) #55014
  * (SEMVER-MAJOR) patch V8 to avoid duplicated zlib symbol (Michaël Zasso) #55014
  * (SEMVER-MAJOR) disable V8 concurrent sparkplug compilation (Michaël Zasso) #55014
  * (SEMVER-MAJOR) always define V8_EXPORT_PRIVATE as no-op (Michaël Zasso) #55014
  * (SEMVER-MAJOR) update V8 to 13.0.245.25 (Michaël Zasso) #55014
  * (SEMVER-MAJOR) upgrade npm to 11.0.0 (npm team) #56274
  * (SEMVER-MAJOR) update undici to 7.0.0 (Node.js GitHub Bot) #56070
fs:
  * (SEMVER-MAJOR) remove ability to call truncate with fd (Yagiz Nizipli) #57567
  * (SEMVER-MAJOR) deprecate passing invalid types in `fs.existsSync` (Carlos Espa) #55753
  * (SEMVER-MAJOR) runtime deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49686
  * (SEMVER-MAJOR) remove `dirent.path` (Antoine du Hamel) #55548
http:
  * (SEMVER-MAJOR) remove outgoingmessage _headers and _headersList (Yagiz Nizipli) #57551
http2:
  * (SEMVER-MAJOR) session tracking and graceful server close (Kushagra Pandey) #57586
lib:
  * (SEMVER-MAJOR) remove obsolete Cipher export (James M Snell) #57266
  * (SEMVER-MAJOR) unexpose six process bindings (Michaël Zasso) #57149
  * (SEMVER-MAJOR) make ALS default to AsyncContextFrame (Stephen Belanger) #55552
  * (SEMVER-MAJOR) runtime deprecate SlowBuffer (Rafael Gonzaga) #55175
net:
  * (SEMVER-MAJOR) make _setSimultaneousAccepts() end-of-life deprecated (Yagiz Nizipli) #57550
readline:
  * (SEMVER-MAJOR) add stricter validation for functions called after closed (Dario Piotrowicz) #57680
  * (SEMVER-MAJOR) fix unicode line separators being ignored (Dario Piotrowicz) #57591
repl:
  * (SEMVER-MAJOR) runtime deprecate instantiating without new (Aviv Keller) #54869
src:
  * (SEMVER-MAJOR) update GetForegroundTaskRunner override (Etienne Pierre-doray) #55014
  * (SEMVER-MAJOR) update NODE_MODULE_VERSION to 134 (Michaël Zasso) #55014
  * (SEMVER-MAJOR) drop --experimental-permission in favour of --permission (Rafael Gonzaga) #56240
  * (SEMVER-MAJOR) add async context frame to AsyncResource (Gerhard Stöbich) #56082
  * (SEMVER-MAJOR) nuke deprecated and un-used enum members in `OptionEnvvarSettings` (Juan José) #53079
stream:
  * (SEMVER-MAJOR) catch and forward error from dest.write (jakecastelli) #55270
test:
  * (SEMVER-MAJOR) disable fast API call count checks (Michaël Zasso) #55014
test_runner:
  * (SEMVER-MAJOR) remove promises returned by t.test() (Colin Ihrig) #56664
  * (SEMVER-MAJOR) remove promises returned by test() (Colin Ihrig) #56664
  * (SEMVER-MAJOR) automatically wait for subtests to finish (Colin Ihrig) #56664
timers:
  * (SEMVER-MAJOR) check for immediate instance in clearImmediate (Gürgün Dayıoğlu) #57069
  * (SEMVER-MAJOR) set several methods EOL (Yagiz Nizipli) #56966
tls:
  * (SEMVER-MAJOR) remove deprecated tls.createSecurePair (Jonas) #57361
  * (SEMVER-MAJOR) make server.prototype.setOptions end-of-life (Yagiz Nizipli) #57339
tools:
  * (SEMVER-MAJOR) update V8 gypfiles for 13.0 (Michaël Zasso) #55014
url:
  * (SEMVER-MAJOR) expose urlpattern as global (Jonas) #56950
  * (SEMVER-MAJOR) runtime deprecate url.parse (Yagiz Nizipli) #55017
zlib:
  * (SEMVER-MAJOR) deprecate classes usage without `new` (Yagiz Nizipli) #55718

PR-URL: #57609
Signed-off-by: RafaelGSS <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@lpinca lpinca lpinca approved these changes

@anonrig anonrig anonrig approved these changes

@cjihrig cjihrig cjihrig approved these changes

@ronag ronag ronag approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

@bjohansebas bjohansebas bjohansebas approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. config Issues or PRs related to the config subsystem deprecations Issues and PRs related to deprecations. needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@jasnell @nodejs-github-bot @marco-ippolito @mcollina @lpinca @anonrig @cjihrig @ronag @RaisinTen @bjohansebas