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.

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

Fix memory leak in new Response(latin1String).arrayBuffer() and new Response(latin1String).json() #3656

Merged
merged 1 commit into from Jul 17, 2023
Merged

Fix memory leak in new Response(latin1String).arrayBuffer() and new Response(latin1String).json() #3656

merged 1 commit into from Jul 17, 2023

Conversation

Jarred-Sumner
Copy link
Collaborator 

❯ mem bun --smol response-json.mjs # After
cpu: Apple M1 Max
runtime: bun 0.6.15 (arm64-darwin)

benchmark                      time (avg)             (min … max)       p75       p99      p995
----------------------------------------------------------------- -----------------------------
Response.json(obj)           9.38 µs/iter     (5.75 µs … 1.86 ms)   6.42 µs  28.04 µs 268.13 µs
Response.json(obj).json()   20.32 µs/iter     (12.92 µs … 1.8 ms)  16.38 µs  51.46 µs 496.08 µs

Peak memory usage: 52 MB

❯ mem ~/.bun/bin/bun --smol response-json.mjs # Before
cpu: Apple M1 Max
runtime: bun 0.6.15 (arm64-darwin)

benchmark                      time (avg)             (min … max)       p75       p99      p995
----------------------------------------------------------------- -----------------------------
Response.json(obj)           6.79 µs/iter      (6.6 µs … 7.31 µs)   6.86 µs   7.31 µs   7.31 µs
Response.json(obj).json()   19.35 µs/iter    (13.04 µs … 1.06 ms)  17.08 µs     52 µs 392.83 µs

Peak memory usage: 219 MB
❯ mem bun --smol response-arrayBuffer.mjs # After
cpu: Apple M1 Max
runtime: bun 0.6.15 (arm64-darwin)

benchmark                                                        time (avg)             (min … max)       p75       p99      p995
--------------------------------------------------------------------------------------------------- -----------------------------
new Response().arrayBuffer() (new string each call, latin1)   12.32 µs/iter      (625 ns … 1.53 ms)      1 µs 555.88 µs 695.71 µs
new Response().arrayBuffer() (new string each call, utf16)    12.59 µs/iter     (1.71 µs … 1.54 ms)   2.13 µs 444.75 µs    623 µs
new Response().arrayBuffer() (existing string, latin1)         6.62 µs/iter     (6.09 µs … 7.35 µs)   6.84 µs   7.35 µs   7.35 µs

Peak memory usage: 49 MB

❯ mem ~/.bun/bin/bun --smol response-arrayBuffer.mjs # Before
cpu: Apple M1 Max
runtime: bun 0.6.15 (arm64-darwin)

benchmark                                                        time (avg)             (min … max)       p75       p99      p995
--------------------------------------------------------------------------------------------------- -----------------------------
new Response().arrayBuffer() (new string each call, latin1)   13.71 µs/iter      (541 ns … 3.13 ms)   1.83 µs 576.38 µs 713.58 µs
new Response().arrayBuffer() (new string each call, utf16)    12.95 µs/iter     (1.67 µs … 2.25 ms)   2.08 µs 458.63 µs  627.5 µs
new Response().arrayBuffer() (existing string, latin1)         4.34 µs/iter      (291 ns … 2.98 ms)    417 ns   4.38 µs 374.79 µs

Peak memory usage: 293 MB

@Jarred-Sumner
Fix memory leak in await new Response(latin1String).arrayBuffer() a…
f869cb3 
…nd `await Response.json(obj).json()`

❯ mem bun --smol response-arrayBuffer.mjs
cpu: Apple M1 Max
runtime: bun 0.6.15 (arm64-darwin)

benchmark                                                        time (avg)             (min … max)       p75       p99      p995
--------------------------------------------------------------------------------------------------- -----------------------------
new Response().arrayBuffer() (new string each call, latin1)    12.9 µs/iter      (625 ns … 4.18 ms)      1 µs 567.17 µs 711.79 µs
new Response().arrayBuffer() (new string each call, utf16)    12.85 µs/iter     (1.67 µs … 1.56 ms)   2.17 µs 462.75 µs 621.13 µs
new Response().arrayBuffer() (existing string, latin1)         6.53 µs/iter     (6.21 µs … 7.07 µs)   6.64 µs   7.07 µs   7.07 µs

Peak memory usage: 49 MB

bun on  jarred/memory-leak-fix took 2s
❯ mem bun response-arrayBuffer.mjs
cpu: Apple M1 Max
runtime: bun 0.6.15 (arm64-darwin)

benchmark                                                        time (avg)             (min … max)       p75       p99      p995
--------------------------------------------------------------------------------------------------- -----------------------------
new Response().arrayBuffer() (new string each call, latin1)     1.2 µs/iter     (1.04 µs … 1.42 µs)   1.22 µs   1.42 µs   1.42 µs
new Response().arrayBuffer() (new string each call, utf16)     2.74 µs/iter     (2.42 µs … 6.37 µs)   2.68 µs   6.37 µs   6.37 µs
new Response().arrayBuffer() (existing string, latin1)       746.37 ns/iter   (643.82 ns … 1.04 µs) 776.11 ns   1.04 µs   1.04 µs

Peak memory usage: 104 MB

bun on  jarred/memory-leak-fix took 2s
❯ mem ~/.bun/bin/bun response-arrayBuffer.mjs
cpu: Apple M1 Max
runtime: bun 0.6.15 (arm64-darwin)

benchmark                                                        time (avg)             (min … max)       p75       p99      p995
--------------------------------------------------------------------------------------------------- -----------------------------
new Response().arrayBuffer() (new string each call, latin1)    1.69 µs/iter      (1.56 µs … 2.1 µs)   1.73 µs    2.1 µs    2.1 µs
new Response().arrayBuffer() (new string each call, utf16)     2.65 µs/iter     (2.47 µs … 3.17 µs)   2.69 µs   3.17 µs   3.17 µs
new Response().arrayBuffer() (existing string, latin1)       667.67 ns/iter   (547.67 ns … 1.28 µs) 694.21 ns   1.28 µs   1.28 µs

Peak memory usage: 2735 MB

bun on  jarred/memory-leak-fix took 2s
❯ mem ~/.bun/bin/bun --smol response-arrayBuffer.mjs
cpu: Apple M1 Max
runtime: bun 0.6.15 (arm64-darwin)

benchmark                                                        time (avg)             (min … max)       p75       p99      p995
--------------------------------------------------------------------------------------------------- -----------------------------
new Response().arrayBuffer() (new string each call, latin1)   13.51 µs/iter       (541 ns … 3.2 ms)   1.92 µs 553.42 µs 709.92 µs
new Response().arrayBuffer() (new string each call, utf16)    13.07 µs/iter     (1.71 µs … 3.43 ms)   2.13 µs 451.21 µs 651.67 µs
new Response().arrayBuffer() (existing string, latin1)         6.25 µs/iter     (5.79 µs … 6.81 µs)    6.4 µs   6.81 µs   6.81 µs

Peak memory usage: 292 MB
@github-actions
Copy link

❌ @Jarred-Sumner 8 files with test failures on bun-darwin-aarch64:

  • test/cli/install/bun-link.test.ts
  • test/cli/install/bun-run.test.ts
  • test/js/bun/jsc/bun-jsc.test.ts
  • test/js/bun/net/socket.test.ts
  • test/js/bun/spawn/spawn.test.ts
  • test/js/bun/test/test-test.test.ts
  • test/js/node/watch/fs.watch.test.ts
  • test/js/web/fetch/fetch-leak.test.js

View test output

#167216522b7b9803bcc22e943d35667077ec9dc2

@github-actions
Copy link

❌ @Jarred-Sumner 6 files with test failures on linux-x64-baseline:

  • test/cli/install/bun-link.test.ts
  • test/cli/install/bun-run.test.ts
  • test/js/bun/jsc/bun-jsc.test.ts
  • test/js/node/process/process.test.js
  • test/js/node/watch/fs.watch.test.ts
  • test/js/web/fetch/fetch-leak.test.js

View test output

#167216522b7b9803bcc22e943d35667077ec9dc2

@github-actions
Copy link

❌ @Jarred-Sumner 7 files with test failures on linux-x64:

  • test/cli/install/bun-link.test.ts
  • test/cli/install/bun-run.test.ts
  • test/cli/run/run-quote.test.ts
  • test/js/bun/jsc/bun-jsc.test.ts
  • test/js/node/process/process.test.js
  • test/js/node/watch/fs.watch.test.ts
  • test/js/web/fetch/fetch-leak.test.js

View test output

#167216522b7b9803bcc22e943d35667077ec9dc2

@github-actions
Copy link

❌ @Jarred-Sumner 9 files with test failures on bun-darwin-x64-baseline:

  • test/cli/install/bun-link.test.ts
  • test/cli/install/bun-run.test.ts
  • test/js/bun/jsc/bun-jsc.test.ts
  • test/js/bun/spawn/spawn-streaming-stdin.test.ts
  • test/js/bun/spawn/spawn-streaming-stdout.test.ts
  • test/js/bun/sqlite/sqlite.test.js
  • test/js/bun/util/sleepSync.test.ts
  • test/js/third_party/webpack/webpack.test.ts
  • test/js/web/timers/setTimeout.test.js

View test output

#167216522b7b9803bcc22e943d35667077ec9dc2

@Jarred-Sumner Jarred-Sumner merged commit 36a25c3 into main Jul 17, 2023
15 of 20 checks passed
@Jarred-Sumner Jarred-Sumner deleted the jarred/memory-leak-fix branch July 17, 2023 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@Jarred-Sumner