Skip to content

Conversation

@Vaiz
Copy link
Contributor

@Vaiz Vaiz commented Feb 9, 2026

BytesBuf::frozen is used only in ensure_frozen() and in debug asserts, there is not much value in caching it and keeping it up to date

@Vaiz Vaiz requested review from Copilot and sandersaares February 9, 2026 16:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the cached BytesBuf::frozen field and computes frozen length on demand to simplify invariants and state updates.

Changes:

  • Dropped the frozen: usize struct field and its update sites.
  • Added a frozen() helper to compute frozen length from len and the (logically) first span builder.
  • Updated debug output/assertions and adjusted the BytesBuf size test accordingly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sandersaares
Copy link
Member

there is not much value in caching it and keeping it up to date

Can you quantify this with benchmark before/after numbers?

@Vaiz
Copy link
Contributor Author

Vaiz commented Feb 10, 2026

there is not much value in caching it and keeping it up to date

Can you quantify this with benchmark before/after numbers?

hmm, I got a steady 5% drop on consume_many_spans bench with 50'000 samples

 cargo bench -p bytesbuf --quiet --bench buf --all-features  -- consume_* --baseline consume_50000_og
BytesBuf/consume_one_span
                        time:   [95.115 ns 95.233 ns 95.355 ns]
                        change: [−2.7936% −2.6326% −2.4536%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1665 outliers among 50000 measurements (3.33%)
  822 (1.64%) high mild
  843 (1.69%) high severe
BytesBuf/consume_max_inline_spans
                        time:   [86.757 ns 86.819 ns 86.883 ns]
                        change: [+0.0717% +0.1750% +0.2800%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 4613 outliers among 50000 measurements (9.23%)
  16 (0.03%) low mild
  2083 (4.17%) high mild
  2514 (5.03%) high severe
BytesBuf/consume_many_spans
                        time:   [196.09 ns 196.38 ns 196.70 ns]
                        change: [+4.5611% +4.7347% +4.9335%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 2202 outliers among 50000 measurements (4.40%)
  2 (0.00%) low mild
  506 (1.01%) high mild
  1694 (3.39%) high severe

@Vaiz Vaiz closed this Feb 11, 2026
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