Conversation
Parse each path's array index component once at the gate and group path indexes by wanted element index, instead of rescanning every pending path and re-running strconv.Atoi on each of them for every matching element. Group storage is a map[int]int of list heads plus a stack-backed link buffer with a lazy heap fallback above 128 pending paths, built in reverse path order so callbacks still fire in ascending path order per element. Dense extraction (compact fixture, go1.26.3 darwin/arm64, medians): 128 indexes 50.6us -> 11.5us (4.4x), allocations unchanged (11); 1024 indexes 2.62ms -> 98us (27x), allocs unchanged (22). Array index membership is snapshotted when entering an array; mutating the paths slice from a callback no longer redirects pending matches (documented by TestEachKeyPathsSnapshotSemantics). Adds regression tests: duplicate paths, extreme/negative/overflow index components, dense boundary sweep 62..1024 with strict callback-order assertions, and EachKeyErr array semantics (shared indexes, terminal strings, missing trailing keys, error/io.EOF stops).
An address-taken [128]int stack buffer is zeroed by the compiler at every '[' token, costing ~1KB of memset per visited array even when no path targets it; that measured 10-13% slower than master on array-heavy documents with object-only paths. Allocate the link buffer on first path registration instead: untouched arrays now cost nothing (measured at parity with master, 0 allocations), and arrays with pending paths allocate one []int of len(paths). Dense numbers unchanged at scale: 1024 indexes 22 allocs (same as master), 128 indexes 12 allocs (+1 x 1KB versus master), speedup retained (~4.2x at 128, ~25x at 1024 on the compact fixture).
… benchmarks Array bookkeeping now activates only when a path actually contains an array component, computed once per call: documents whose paths never target an array skip the per-'[' path scan and map creation entirely, measuring faster than master on array-heavy documents with object-only paths, where master builds a map and zeroes a flag buffer at every '[' token. The heads map and link buffer are allocated lazily on first registration, so sparse two-index extraction over object elements runs faster with zero allocations, and links are int32, halving the buffer. Adds permanent regime benchmarks to issues_56_107_229_test.go covering dense, sparse (scalar and object elements), idle arrays, shared terminal paths, flat objects, and the EachKeyErr dense case. Medians, go1.26.3 darwin/arm64, -cpu=1, paired against master: dense 128 is 4.2x faster (12.3us vs 51.1us), dense 1024 25.6x (103us vs 2.65ms), idle arrays 8-18% faster, realistic sparse extraction ~6% faster, flat objects at parity. Known residual costs, disclosed in the PR: arrays of tiny scalar elements with few wanted indexes measure up to ~7% slower, and degenerate fixtures with many identical terminal paths sharing one index measure 14-23% slower (0.5-3us absolute).
The sparse benchmarks used components "3" and "16380" without brackets, which are object keys, not array indexes: both benchmarks produced zero callbacks and measured array skipping instead of extraction. Use bracketed selectors so they measure what their names claim. With valid fixtures, sparse extraction measures faster than master on both scalar elements (~12%, 186us vs 211us over 16384 elements) and object elements (~3%), replacing the earlier claim of a small scalar regression that the broken fixture had produced. The anyArrayPath precompute snapshots the routing decision at call entry, which expands the observable difference for callbacks that mutate the paths slice: converting a non-array component into an index before the array is reached is no longer observed (master re-evaluated per array). Document the contract explicitly, paths are treated as immutable for the duration of the call, and extend TestEachKeyPathsSnapshotSemantics with the pre-entry mutation case as its witness.
Extract the inlined array-path precheck and maintain scanning, validation, index grouping, skip logic, and callback walks in one template. Generate specialized EachKey and EachKeyErr functions with no runtime dispatch. Regenerate with go generate ./.... Use generation after helper extraction, a shared core with specialized array walks, and a generic core failed the timing gate. Keep go.mod at Go 1.13 and use only the standard library for generation. Validation: all ten benchmark regimes retain identical bytes and allocation counts. Alternating baseline/candidate samples show no timing change beyond noise; longer checks confirm the largest positive median shifts. Escape analysis preserves stack-backed path buffers and non-escaping array callbacks. The differential harness matches 126 ordered traces and returns. The full suite has only the pre-existing TestOracleSetPr286Regression failures, identical on b8bb21d and master. Vet adds no warnings. Generation is reproducible and the output is gofmt clean.
Verification of the generation refactor flagged two comments that were rewritten instead of preserved: the six-line note explaining cursor position after the match block, and the empty-component guard comment. Restore both texts byte-for-byte from the pre-refactor source. Because the template shares one body, both variants now carry them; the texts are unchanged from the originals, EachKeyErr simply gains the same accurate documentation. go generate reproduces eachkey.go exactly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Bulk extraction of array elements by index is the core use case of
EachKey/EachKeyErr, and it is their slowest path: for every matching element, the callback loop rescans all pending paths and re-runsstrconv.Atoion each path's index component, in both functions.What changes
Performance. The array branch now parses each pending path's index component once and groups path indexes by wanted element index:
map[int]int32list heads plus an int32 link buffer, both allocated lazily on first registration at a given array. Grouping activates only when some path contains an array component, computed once per call, so documents whose paths never target an array skip the per-[scan and map creation entirely. The per-element callback walks only the paths waiting on the current element; prepending in reverse path order preserves ascending path order per element. The oldpIdxFlagsbookkeeping is removed from both functions.Structure.
EachKeyandEachKeyErrwere 215 near-identical lines each. They now live in a generatedeachkey.go, produced bygo generatefrom a single template (internal/eachkey/eachkey.go.tmpl) plus a dependency-free generator (internal/eachkey/main.go); the shared scanner exists once and the two callback variants are stamped out from it. Helper extraction, a shared core, and generics were all tried first and rejected on measured timing grounds; code generation is the only design that was byte-for-byte performance-neutral.go.modstays atgo 1.13, the generator adds no exported API and no dependencies, and regeneration is exactly reproducible.Benchmarks. Permanent regime benchmarks live in
issues_56_107_229_test.go(dense 128/1,024, sparse scalar and object, idle 62/128, shared terminal 128/1,024, flat objects,EachKeyErrdense), so every number below is reproducible withgo test -bench.Semantics note (please read)
EachKey/EachKeyErrtreatpathsas immutable for the duration of the call: array-index membership is captured when an array is entered, and the routing decision is captured at call entry. A callback that mutates thepathsslice mid-iteration is not observed; previously it could redirect later matches as an artifact of re-reading components during iteration. That was never a documented contract, andTestEachKeyPathsSnapshotSemanticspins both cases. If the old observable behavior matters to anyone, speak up here.Benchmarks
go1.26.3, darwin/arm64, paired alternating runs,
-cpu=1 -benchmem, five or more pairs per side, plus independent verification.Allocation changes: dense 128 goes from 11 to 14 allocs (+704 B) and dense 1,024 from 22 to 24 (+3.3 KB), from the lazily created heads map and link buffer; idle, flat, and no-registration regimes allocate nothing. The refactoring commit was gated separately: identical allocation counts on all ten benchmarks and no timing regression surviving an overlap-range test across seven to twenty-two interleaved pairs.
Known regressions, stated plainly
{"array":["x"]}with N copies of{"array","[0]"}) measure 15-32% slower, 0.4 to 5 us absolute. This is grouping registration overhead, the trade buying the 4x to 26x dense win, and the in-repoBenchmarkEachKeySharedTerminal*benchmarks pin it.Tests
Full suite passes except
TestOracleSetPr286Regression, which fails identically on current master. New regression tests: duplicate paths deliver the same value to every copy; extreme index components never match and never suppress a sibling lookup; dense array-path sweep at 62/63/64/127/128/129/1024 with strict document-order assertions;EachKeyErrarray semantics (shared indexes, terminal strings, missing trailing keys, error stop within one element, io.EOF graceful stop); snapshot semantics for both mutation cases.Verification
Six independent verification passes were run against this branch: differential harnesses comparing 55,000 to 57,000 ordered execution records per revision (zero panics; identical to master everywhere except the documented and tested snapshot semantics), link-encoding soundness confirmed structurally, paired benchmarks of every claim with corrections the process itself caught and fixed (invalid sparse fixtures, an understated shared-terminal cost, an eagerly zeroed buffer, an expanded mutation surface, and two rewritten comments in the generated code, all remediated in-tree), and code-generation reproducibility confirmed byte-for-byte. No functional defects remain open.