Skip to content

util: reuse the hash read buffer instead of zeroing 128KB per file - #2939

Merged
sylvestre merged 3 commits into
mainfrom
reuse-hash-buffer
Oct 25, 2026
Merged

sylvestre merged 3 commits into
mainfrom
reuse-hash-buffer

Conversation

@sylvestre

@sylvestre sylvestre commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

util: reuse the hash read buffer instead of zeroing 128KB per file

Digest::reader_sync_with and Digest::update_from_file each zeroed a fresh
128KB buffer for every file they hashed. Direct mode hashes every included
header on each cache hit, and those are usually a few KB, so the memset cost
more than the hashing: it was ~15% of the server's CPU on direct-mode hits.

Keep one buffer per thread and reuse it, falling back to a fresh one if it is
already borrowed further up the stack.

Server CPU per direct-mode cache hit (perf stat, 200 hits):
C: 0.95 ms -> 0.76 ms (-20%)
C++ (STL headers): 4.21 ms -> 3.57 ms (-15%)

End to end (hyperfine, 100 runs), a C direct-mode hit goes from 2.6 ms to
2.3 ms; on the C++ one the gain is within the run-to-run noise.


util: hash through stack buffers instead of a per-thread heap buffer

The per-thread 128KB buffer stayed allocated for the life of each thread
that hashed a file, and changed the heap layout seen by later allocations.

Read the first 64KB through a 16KB stack buffer instead, and only switch
to the 128KB one for larger inputs. Headers, which direct mode hashes the
most, then only zero 16KB, and nothing stays allocated between calls.


util: hash through std::io::copy, without zeroing a buffer

The 16KB stack buffer is zeroed for every input hashed, which was 45% of
the hash_multiple_files benchmark under CodSpeed's simulation: more than
hashing the data itself.

std::io::copy reads through a stack buffer that it does not zero for
readers able to fill uninitialized memory, such as files and byte slices.
Feed the digest (and the time macro finder) through it as a Write, so that
nothing is zeroed and nothing stays allocated between calls.

Local medians (divan), main / per-thread buffer / 16KB stack buffer / this:
hash_multiple_files 35.6 / 20.2 / 20.5 / 19.6 us
hash_header_file 7.4 / 5.9 / 5.9 / 5.7 us
hash_large_data 867 / 829 / 875 / 836 us
time_macro_finder_no_macros 300 / 274 / 299 / 268 us

@sylvestre
sylvestre added this pull request to stack #2944 October 24, 2026 13:02
@sylvestre sylvestre changed the title reuse hash buffer util: reuse the hash read buffer instead of zeroing 128KB per file Oct 24, 2026
@codecov-commenter

codecov-commenter commented Oct 24, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.11111% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.68%. Comparing base (cb8a84d) to head (0220bb3).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/util.rs 91.11% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2939      +/-   ##
==========================================
+ Coverage   75.66%   75.68%   +0.01%     
==========================================
  Files          74       74              
  Lines       43955    43975      +20     
==========================================
+ Hits        33260    33281      +21     
+ Misses      10695    10694       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed

codspeed Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by ×2.5

2 warnings

⚠️ 1 benchmark is dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

⚡ 5 improved benchmarks
✅ 53 untouched benchmarks
🆕 4 new benchmarks
⏩ 6 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ CPU Simulation hash_multiple_files 937.2 µs 124.2 µs ×7.5
⚡ CPU Simulation hash_header_file 203.8 µs 53.3 µs ×3.8
⚡ CPU Simulation hash_large_data 9.3 ms 5.9 ms +57.28%
⚡ CPU Simulation time_macro_finder_with_macros 2.6 ms 1.8 ms +47.88%
⚡ CPU Simulation time_macro_finder_no_macros 3 ms 2.1 ms +43.03%
🆕 CPU Simulation preprocessor_cache_entry_decode N/A 240.2 µs N/A
🆕 CPU Simulation preprocessor_cache_lookup N/A 3.5 ms N/A
🆕 Memory preprocessor_cache_entry_decode N/A 65.1 KB N/A
🆕 Memory preprocessor_cache_lookup N/A 266 B N/A

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing reuse-hash-buffer (0220bb3) with main (a24b058)2

Open in CodSpeed

Footnotes

  1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on main (cb8a84d) during the generation of this report, so a24b058 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@sylvestre

Copy link
Copy Markdown
Collaborator Author

@codspeedbot fix this regression

@codspeed

codspeed Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

@codspeedbot fix this regression

I fixed the memory regression and pushed it to this branch as 62a4307. Most of the hashing speedup is still there; hash_multiple_files gave back part of its gain.

Cause: the 128KB per-thread heap buffer was never freed, so every thread that hashed a file kept 128KB allocated. That's the 324 B → 131,428 B jump in time_macro_finder_with_macros. All benchmarks run in one process, so the leftover allocation also changed how the heap was laid out for the benchmarks that ran after it.

Fix (src/util.rs): the thread-local is gone, and reads now go through stack buffers in two sizes:

  • The first 64KB of a file are read through a 16KB buffer. Headers usually fit in that, so they only zero 16KB instead of 128KB.
  • Only bigger files switch to the 128KB buffer, which is set up once per file.
  • The two buffers are never on the stack at the same time, and nothing stays allocated between calls.

I added test_reader_sync_across_buffer_sizes. It checks that digests match a single-pass hash, and that a __TIME__ split across a read boundary is still found, at sizes around each boundary. The existing util and preprocessor-cache tests pass, and clippy is clean.

Results (all runs in the same environment):

Benchmark Mode main PR before PR now
time_macro_finder_with_macros Memory 324 B 131,428 B 324 B
hash_multiple_files Simulation 938 µs 129 µs 223 µs (×4.2 vs main)
hash_header_file Simulation 205 µs 85 µs 88 µs (×2.3 vs main)
  • Against main, the other 27 simulation benchmarks are unchanged.
  • hash_multiple_files: it is slower than the previous PR version because each 10KB file now pays for zeroing 16KB. In simulation that costs about 9 µs per file. That's the price of not keeping memory around.
  • The other CPU changes in the report: build_workflow_incremental, strip_basedirs_multiple, the decompression benchmarks and cache_entry_roundtrip_large never call the changed code. They only moved because of the leftover allocation, and they're now back to main's numbers.

If you'd rather have the full speed on small files and keep the per-thread buffer, comment @codspeedbot try an 8KB first buffer and I'll measure that trade-off.

Summary
  • Warming up the performance wizard...
  • Analyze the regression — Memory regression: the per-thread 128KB heap buffer stays allocated. The CPU regressions (build_workflow_incremental, strip_basedirs_multiple) don't call the changed code; they ran on a different runner type.
  • Implement fix — Replaced the thread-local heap buffer with stack buffers: the first 64KB are read through a 16KB buffer, and only larger inputs switch to the 128KB one. Tests and clippy pass.
  • Validate with tests and CodSpeed — Simulation vs main: hash_multiple_files ×4.2, hash_header_file ×2.3, the rest unchanged. Memory: time_macro_finder_with_macros back to 324 B.
  • Commit and push — Pushed 62a4307 to the PR branch.
  • Processing benchmark results — Benchmarks have been successfully run and processed. The results are available in CodSpeed
  • Handling comment — Comment handling finished successfully

Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.

@sylvestre
sylvestre force-pushed the reuse-hash-buffer branch 2 times, most recently from eca2f55 to 085ad43 Compare October 25, 2026 08:52
@sylvestre
sylvestre marked this pull request as ready for review October 25, 2026 08:56
@sylvestre
sylvestre removed this pull request from stack #2944 October 25, 2026 13:14
@sylvestre
sylvestre changed the base branch from main to preprocessor-cache-benches October 25, 2026 13:14
@sylvestre
sylvestre added this pull request to stack #2952 October 25, 2026 13:14
@sylvestre
sylvestre force-pushed the reuse-hash-buffer branch 2 times, most recently from 3611a4b to 061b1ba Compare October 25, 2026 14:24
Base automatically changed from preprocessor-cache-benches to main October 25, 2026 22:08
sylvestre and others added 3 commits October 25, 2026 23:08
Digest::reader_sync_with and Digest::update_from_file each zeroed a fresh
128KB buffer for every file they hashed. Direct mode hashes every included
header on each cache hit, and those are usually a few KB, so the memset cost
more than the hashing: it was ~15% of the server's CPU on direct-mode hits.

Keep one buffer per thread and reuse it, falling back to a fresh one if it is
already borrowed further up the stack.

Server CPU per direct-mode cache hit (perf stat, 200 hits):
  C:                    0.95 ms -> 0.76 ms (-20%)
  C++ (STL headers):    4.21 ms -> 3.57 ms (-15%)

End to end (hyperfine, 100 runs), a C direct-mode hit goes from 2.6 ms to
2.3 ms; on the C++ one the gain is within the run-to-run noise.
The per-thread 128KB buffer stayed allocated for the life of each thread
that hashed a file, and changed the heap layout seen by later allocations.

Read the first 64KB through a 16KB stack buffer instead, and only switch
to the 128KB one for larger inputs. Headers, which direct mode hashes the
most, then only zero 16KB, and nothing stays allocated between calls.
The 16KB stack buffer is zeroed for every input hashed, which was 45% of
the hash_multiple_files benchmark under CodSpeed's simulation: more than
hashing the data itself.

std::io::copy reads through a stack buffer that it does not zero for
readers able to fill uninitialized memory, such as files and byte slices.
Feed the digest (and the time macro finder) through it as a Write, so that
nothing is zeroed and nothing stays allocated between calls.

Local medians (divan), main / per-thread buffer / 16KB stack buffer / this:
  hash_multiple_files          35.6 / 20.2 / 20.5 / 19.6 us
  hash_header_file              7.4 /  5.9 /  5.9 /  5.7 us
  hash_large_data               867 /  829 /  875 /  836 us
  time_macro_finder_no_macros   300 /  274 /  299 /  268 us
@sylvestre
sylvestre merged commit cf5efe6 into main Oct 25, 2026
94 checks passed
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.

2 participants