Skip to content

table: reserve packed repeated enum capacity - #522

Closed
fallintoplace wants to merge 1 commit into
anthropics:mainfrom
fallintoplace:perf/table-packed-enum-reserve
Closed

fallintoplace wants to merge 1 commit into
anthropics:mainfrom
fallintoplace:perf/table-packed-enum-reserve

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

  • Reserve capacity while decoding packed repeated enums in the table codec.

Why

  • The table path appended enum values without a capacity hint.

Implementation

  • Use a bounded payload hint for open enums and known closed-enum values.
  • Keep unknown closed-enum values on the existing unknown-field path.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@fallintoplace fallintoplace changed the title perf(table): reserve packed repeated enum capacity table: reserve packed repeated enum capacity Oct 3, 2026
@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code] Thanks for this. The gap it targets is real: generated (unrolled) decoders call reserve once per packed record (buffa-codegen/src/impl_message.rs, the packed arms), and the table decoder pushes one value at a time. I'm closing this version, for these reasons:

  • reserve_exact runs once per packed record, and only when spare capacity is zero. A field sent as N one-element packed records then asks for exactly one more slot N times, where push on main doubles. When the allocator cannot grow in place that is quadratic copying, on input the sender chooses. The unrolled decoder uses the amortising reserve.
  • enum_store_result is a safe fn that dereferences a raw pointer; it needs to be unsafe fn with a # Safety section, or take a reference.
  • The caps (64 KiB for open enums, 16 values for closed) differ from the unrolled decoder, which the module docs say the table decoder matches, and the PR does not say where they come from.
  • The six new tests pass on main unchanged: they check decoded values, and do not assert that capacity grew.
  • There is no measurement, and benchmarks/ has no table build or repeated-enum shape to take one from.
  • Every hunk in shape.rs conflicts with the open table stack (table: let a table message hold messages that have no table #475 to conformance: run the protobuf suite against the table codec #478), which lands first.

A follow-up would be welcome once that stack is in: one amortising reserve per packed record, sized as the unrolled decoder sizes it, with a test that fails without it and a benchmark that shows the effect.

@iainmcgin iainmcgin closed this Oct 6, 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