perf(client): normalize streamed rows in a single pass (#229) - #230
Merged
Conversation
ScrollApi.scroll re-normalized every row on the stream with O(cols^2) ListMap scans (fields.map(row.getOrElse) linear gets, a full filterNot second walk with a Set.contains per key, and a double ListMap rebuild) - measured at ~7% of total sidecar CPU on the arrow#160 benchmark. The same shape ran per row in normalizeRow on the window-enrichment stream, the window one-shot loop and the parse path's final rows.map. New ElasticConversion.rowNormalizer(requestedFields), built once per stream/loop and applied per row: - stream-constant work hoisted: field-order array, name -> position java.util.HashMap, EntityContext decision, duplicate-name detection (degenerate duplicate output names fall back to the legacy semantics via normalizeRowOrdered, with the name set still hoisted); - a single walk per row, splitting entries into a positional array and an ordered extras buffer, with one ListMap build; - zero-rebuild passthrough: a row already carrying the requested fields in order (extras may trail, already in final position) is returned as the same instance; an in-order strict prefix under EntityContext too. Output contract unchanged: requested fields first in SQL SELECT order, missing ones null-filled (or skipped under EntityContext), then the row's extra entries in their original order. New RowNormalizerSpec (15 tests) pins the contract, the passthrough identities, the duplicate fallback and .toList equality with the legacy normalizeRow across a shape battery in both contexts, including one normalizer instance reused across a heterogeneous row stream. Guard suites green on real ES 6.8 (rest+jest), 7.17, 8.18, 9.0; core 761; cross-compiled 2.12 + 2.13. Closes #229 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Closes #229
Problem
ScrollApi.scrollre-normalized every row on the stream with per-row collection work measured at ~7% of total sidecar CPU on the arrow#160 benchmark (post-#227 image, J2 = JOIN + GROUP BY over an 11M-row extraction):Set$Set3.contains3.91% (top non-Arrow, non-Jackson method), plusStrictOptimizedIterableOps.filterImplandListMapBuilderallocations. Per row the old code did:fields.map(f => row.getOrElse(f, null))— eachgeta linear scan of theListMap⇒ O(cols²)String.equals;row.filterNot(...)— a full second walk with aSet.containsper key;ListMap(ordered: _*) ++ extra— a freshListMapbuild plus O(n) appends —for rows that in the common case already carry exactly the requested keys. The same shape existed in
normalizeRow, called per row from the window-enrichment stream, the window one-shot loop, and the parse path's finalrows.map.Fix
New
ElasticConversion.rowNormalizer(requestedFields)— built once per stream/loop, applied per row:java.util.HashMap, theEntityContextdecision, and a duplicate-name check (degenerate duplicate output names fall back to the legacynormalizeRowsemantics).ListMapis built once.EntityContext(skip-missing semantics) also passes through.EntityContext) — then the row's extra entries in their original order.Rewired call sites (normalizer hoisted at each):
ScrollApi.scrollstream normalization (the #229 site),ScrollApi.scrollWithWindowEnrichmentper-row map,SearchApi.enrichResponseWithWindowValuesloop, and the parse path's finalrows.map.normalizeRowis unchanged and remains the single-row entry; its scaladoc now warns loops/streams to hoist arowNormalizer.Tests
RowNormalizerSpec(core, no Docker): SELECT-order restoration, null-fill vsEntityContextskip, extras ordering (leading/interleaved), present-but-null kept in both contexts, empty row/fields, passthrough identity (theSameInstanceAs), duplicate-fields fallback, and a shape battery asserting.toListequality with the legacynormalizeRowin both contexts.ScrollCompletenessSpec,SelectCompletenessSpec,LimitCompletenessSpec,GroupByCompletenessSpec,WindowPartitionCompletenessSpec,WindowFunctionSpec,HitMetadataSpecon ES 6.8 (rest + jest), 7.17, 8.18, 9.0.scalafmtCheckAll+headerCheckpass.Context
Third leg of the arrow#160 extraction-cost campaign (with elasticsql #227/#228 and arrow #161): this is the SoftClient4ES-side residual identified in the post-#227 JFR profile.
🤖 Generated with Claude Code