Skip to content

perf: raise default max frame size to 64 KiB - #272

Open
anmonteiro wants to merge 1 commit into
perf/batch-window-updatesfrom
perf/default-max-frame-size-64k
Open

perf: raise default max frame size to 64 KiB#272
anmonteiro wants to merge 1 commit into
perf/batch-window-updatesfrom
perf/default-max-frame-size-64k

Conversation

@anmonteiro

Copy link
Copy Markdown
Owner

Summary

  • raise the default Config.read_buffer_size / advertised SETTINGS_MAX_FRAME_SIZE from 16 KiB to 64 KiB
  • update config docs to match the new default
  • make the affected tests explicit about whether they rely on the protocol minimum or the library default

Why

On the large-upload benchmark, the server was still advertising the protocol-minimum MAX_FRAME_SIZE (16 KiB). That forces clients such as curl/nghttp2 to fragment a 1.5 GiB upload into a very large number of DATA frames, which amplifies parser, flow-control, and write-loop overhead.

64 KiB was the best point I measured locally on this workload:

  • 16 KiB: much slower
  • 64 KiB: best throughput
  • 256 KiB and 1 MiB: slightly worse again

So this is not just “bigger is better”; 64 KiB looks like the right default tradeoff here.

Validation

  • dune build --display=short @runtest-test_h2 @runtest-test_h2_client @runtest-test_h2_server spec/eio_h2spec.exe
  • all tests passed

Benchmark

Using the existing 1.5 GiB curl upload benchmark against the Eio server, with the same local gluten checkout in both variants:

  • baseline (perf/batch-window-updates): 3.391s, 452.94 MiB/s; 3.409s, 450.62 MiB/s
  • 64 KiB default max frame size: 0.813s, 1890.22 MiB/s; 0.799s, 1921.55 MiB/s

This is roughly a 4.2x throughput improvement on this workload.

Notes

This changes the default connection preface behavior: default connections now send a non-empty SETTINGS frame advertising a larger MAX_FRAME_SIZE. The tests updated here cover the two places that were implicitly assuming the old 16 KiB default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant