Skip to content

Keep metric field lookup nosplit - #14879

Open
tamird wants to merge 1 commit into
google:masterfrom
tamird:metric-field-lookup-nosplit
Open

tamird wants to merge 1 commit into
google:masterfrom
tamird:metric-field-lookup-nosplit

Conversation

@tamird

@tamird tamird commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Metric sampling has required allocation-free, nosplit field lookup since 1c9ce54. The large-field optimization in 79b3802 replaces linear search with a Go map, whose runtime lookup can split the stack even when fieldMapper.lookupSingle is marked nosplit. Checkescape has missed this because map lookups are not explicit SSA calls.

For fields with more than 48 values, sort retained field values with their declaration indices by pointer address and use a nosplit binary search. This preserves declaration order, pointer identity, and lookup behavior when a FieldValue's string changes. Small fields retain linear search. Pointer ordering uses the same nonmoving heap assumption as pipe lock ordering; storing the pointers in the constructor map ensures caller-local values escape to the heap. The lookup uses a local search loop because slices.BinarySearchFunc can split the stack.

Large-field lookup becomes O(log n). An ARM64 microbenchmark with 1,024 field values changes from about 6.7 to 12.5 ns/op, without allocating. Add the benchmark and extend existing field tests across the lookup threshold, including declaration order, pointer identity, and mutation of the value's string. Fix panic assertions that previously succeeded even when no panic occurred.

This fixes the metric consumer before a separate checkescape change begins detecting runtime calls implicit in map operations.

Assisted-by: Codex

Metric sampling has required allocation-free, nosplit field lookup since
1c9ce54. The large-field optimization in 79b3802 replaces linear
search with a Go map, whose runtime lookup can split the stack even when
fieldMapper.lookupSingle is marked nosplit. Checkescape has missed this
because map lookups are not explicit SSA calls.

For large fields, sort retained field values with their declaration
indices by pointer address and use a nosplit binary search. Keep
declaration order, pointer identity, and lookup behavior when a
FieldValue's string changes. Retain linear search for small fields.
Pointer ordering uses the same nonmoving heap assumption as pipe lock
ordering. Store the pointers in the constructor map so caller-local
values escape to the nonmoving heap.

The large-field lookup becomes O(log n). An ARM64 microbenchmark with
1,024 field values changes from about 6.7 to 12.5 ns/op, without
allocating. Add the benchmark and extend existing field tests across the
lookup threshold. Fix panic assertions that previously succeeded even
when no panic occurred.

Assisted-by: Codex
@tamird

tamird commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

@EtiennePerot could you please TAL as the author of the commits mentioned in the description? I discovered this while investigating failures in #14502.

@tamird

tamird commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

@konstantin-s-bogom mind having a look here?

@tamird

tamird commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

@EtiennePerot could you please take a look? once this lands I will put up another PR to fix the gap in checkescape that admitted this use of maps under nosplit.

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.

1 participant