-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Rewrite the internal Web Stream native bindings to use less memory #16349
Conversation
Updated 12:47 AM PT - Jan 26th, 2025
❌ @Jarred-Sumner, your commit 2827f0b has 5 failures in
🧪 try this PR locally: bunx bun-pr 16349 |
totally drive-by comment, but a PR title like "Streams: Fix memory leak with |
Additionally, JSC's GC was not being informed of allocations in |
fbcc4f7
to
0768baf
Compare
74f8731
to
186d5a6
Compare
What does this PR do?
Fixes #12198
Closes #16075
The usage of
type: "bytes"
in our lazy internal ReadableStream sources led to a memory leak because each read from the stream sometimes returns a larger & different buffer than the one the user provides.How did you verify your code works?
The version using Node APIs uses the same amount of memory as the version using Bun/Web APIs.
Will follow up with a memory leak test once a release build is generated in CI.