Skip to content

HistoryBuf::write: Fix the performance regression from the storage refactor - #691

Open
zeenix wants to merge 6 commits into
rust-embedded:mainfrom
zeenix:historybuf-write-perf
Open

zeenix wants to merge 6 commits into
rust-embedded:mainfrom
zeenix:historybuf-write-perf

Conversation

@zeenix

@zeenix zeenix commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Attribution: this PR was implemented by LLMs, Claude Fable 5.1 and then Claude Opus 5.5 for
the review follow-up, running in Claude Code under my direction and review. Every commit carries
Assisted-by: trailers, and this description was generated by the same models.

Supersedes #687 and addresses #598.

Since the view-types refactor in 0.9.0, LLVM compiles the wrap-around reset of write_at in
HistoryBuf::write to conditional moves, so each write depends on the previous one. Calling
core::hint::cold_path in that branch brings back a real branch. cold_path needs Rust 1.95,
hence the MSRV bump. That in turn needs a newer pinned nightly in CI, and with newer Miri the build
script's LL/SC probe succeeds on x86_64, so two small prep commits deal with those.

This also rejects zero capacity in HistoryBuf::new_with at compile time, as new already does
(from #687, thanks @wyf-777). The compile-fail tests for both constructors are cfg(doctest)
doctests rather than cfail UI tests: trybuild only runs cargo check, which never evaluates
these post-monomorphization assertions, so UI tests for them compile successfully.

Measurements

Median write throughput in GB/s, 8 MiB input, x86_64, rustc 1.98. All three versions run the
same benchmark code as benches/history_buf.rs, built out of tree so that 0.8.0 can be included.
LTO means lto = "fat" with codegen-units = 1.

Window 0.8.0 (LTO) main (LTO) PR (LTO) 0.8.0 (default) main (default) PR (default)
8 2.1 1.5 3.6 2.1 3.0 3.6
9 4.1 1.5 3.7 3.1 3.0 3.5
16 2.1 1.5 4.0 1.5 3.0 3.7
31 4.2 1.5 4.0 2.7 3.0 3.5
32 2.1 1.5 3.8 2.2 3.0 3.6
64 2.2 1.5 4.1 2.2 2.8 2.8
100 4.1 1.5 4.1 2.6 2.4 2.4
128 2.2 1.5 4.3 2.3 2.4 2.4
1000 2.2 1.5 4.3 1.2 2.3 2.3
1024 2.2 1.5 4.3 1.6 2.3 2.3

With LTO, this PR is 2.4 to 2.8 times faster than main at every size. It matches or beats 0.8.0
except at sizes 9 and 31, where it is 6 to 11% slower. 0.8.0 itself swings between about 2 and
4 GB/s depending on the size. With the default profile, this PR is about 20% faster than main up
to size 32 and unchanged from 64 up.

For write_and_search, this PR is within 6% of main at every size with LTO. With the default
profile, individual sizes move between 14% slower and 40% faster than main. Main shows the same
two plateaus at other sizes, and forcing 64-byte loop alignment reshuffles which sizes are fast,
so this looks like code layout rather than a change in the work done.

Credit to @BVollmerhaus for the report and bisect, and @sgued for the bounds-check analysis.

Generated by Claude Fable 5.1 and Claude Opus 5.5.

🤖 Generated with Claude Code

@sgued sgued left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review in progress, I pressed ctrl-enter by mistake*

I don't fully understand the performance gains here.
I'm looking at the assembly of the following:

use heapless::HistoryBuffer;

#[unsafe(no_mangle)]
pub fn write_some_value(buf: &mut HistoryBuffer<u8, 31>, value: u8) {
    buf.write(value);
}

Comment thread CHANGELOG.md Outdated
Comment thread src/history_buf.rs Outdated
Comment thread src/history_buf.rs
@sgued

sgued commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Ok, I think I see some weirdness that needs further exploration.

In #598 (comment) I tested the benchmark but I think it's only because of the missing black_box that it brought back performance by removing

I do reproduce your benchmark results, but there seems to be variations depending on the NEEDLE_SIZE. Can you please add some benchmarks where NEEDLE_SIZE is larger and some where it's not a power of 2 ?

@zeenix
zeenix force-pushed the historybuf-write-perf branch from 3815ccf to d63eec8 Compare September 26, 2026 19:33
Under Miri, `RUSTC` is Miri itself, which doesn't do codegen, so the probe
succeeds on any target and the pool tests then hit ARM inline assembly.

Assisted-by: Claude Opus 5.5 (claude-opus-5-5)
The old one predates the upcoming MSRV bump to 1.95.

Assisted-by: Claude Opus 5.5 (claude-opus-5-5)
Prep for using `core::hint::cold_path`.

Assisted-by: Claude Opus 5.5 (claude-opus-5-5)
Covers the sliding-window search from issue rust-embedded#598 over several window sizes.

Assisted-by: Claude Fable 5.1 (claude-fable-5-1)
Assisted-by: Claude Opus 5.5 (claude-opus-5-5)
Like `new`. A zero-capacity buffer makes `recent_index` underflow.

Originally implemented by wyf-777 in PR rust-embedded#687.

Assisted-by: Claude Fable 5.1 (claude-fable-5-1)
Assisted-by: Claude Opus 5.5 (claude-opus-5-5)
Without the hint, LLVM turns the reset of `write_at` into conditional
moves, making each write depend on the previous one. This recovers most
of the throughput lost since 0.9.0 (rust-embedded#598).

Assisted-by: Claude Fable 5.1 (claude-fable-5-1)
Assisted-by: Claude Opus 5.5 (claude-opus-5-5)
@zeenix
zeenix force-pushed the historybuf-write-perf branch from 73fa65b to 761d12f Compare September 26, 2026 19:46
@zeenix

zeenix commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

I do reproduce your benchmark results, but there seems to be variations depending on the NEEDLE_SIZE. Can you please add some benchmarks where NEEDLE_SIZE is larger and some where it's not a power of 2 ?

Done, along with other changes you asked for. Here are the results from my machine:

  • Write only, fat LTO: 2.4 to 2.8 times faster than main at every size. It matches or beats 0.8.0 except at sizes 9 and 31, where it is 6 to 11% slower.
  • Write only, default release profile: about 20% faster than main up to size 32, unchanged from 64 up.
  • Write plus search: within 6% of main with LTO. On the default profile individual sizes swing between 14% slower and 40% faster, on main and the branch alike.

So very weird indeed. 🤷

This branch has not been deployed

No deployments
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