Skip to content

table: keep dense lookup for wide messages - #517

Open
fallintoplace wants to merge 1 commit into
anthropics:mainfrom
fallintoplace:perf/table-wide-dense-lookup
Open

fallintoplace wants to merge 1 commit into
anthropics:mainfrom
fallintoplace:perf/table-wide-dense-lookup

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

Keep dense lookup for low field numbers in messages with 255 or more fields.

Why

Wide messages currently fall back to binary search for every field.

Implementation

Keep indices for fields below 64 in the dense lookup table.

Benchmark

Not applicable.

@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.

@iainmcgin

Copy link
Copy Markdown
Collaborator

[claude code] Thanks for this. The change is correct: the dense array is filled only for field numbers below 64, entries are sorted by field number, so such a field is at index 62 or lower and its index-plus-one always fits a u8, whatever the number of entries. The removed assert was redundant with the const checks that remain.

It is on hold for ordering, not for content. It conflicts with the open table codec stack (#475 to #478) in Table::new and in two test files, and that stack merges first. Once it is in I will rebase this PR and push one maintainer commit, then approve it:

  • reword the two comments that still describe the 255-entry limit (buffa-test/build.rs, and the W254/W255/W256 tests, which then stop straddling a boundary);
  • u8::try_from in place of the as u8 cast in the codegen-side mirror;
  • drop the Changed fragment: the table codec is unreleased, so the stack's Added fragment covers it.

There is no action for you in the meantime.

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