perf: fuse chained std.map calls into ComposedMappedArr#902
Closed
He-Pin wants to merge 2 commits into
Closed
Conversation
Motivation: When `std.map(f, std.map(g, arr))` is evaluated, two MappedArr views are nested. Each element access traverses two cache layers and two indirection levels. Common in Jsonnet when splitting logic across multiple map calls. Modification: In `Arr.mapped()`, detect when the source is already a `MappedArr` with live state and create a `ComposedMappedArr` that applies both functions in sequence from the original source, eliminating one cache layer and one level of indirection per element access. Result: Native: lazy_array_sparse_indexing 20.5 → 19.7ms (-3.9%) Existing test coverage: lazy_array_views.jsonnet tests chained maps.
Motivation: ComposedMappedArr was using the outer map's callPos for both inner and outer function calls, causing error stack frames to point to the wrong source position when the inner function fails. Modification: - Store separate outerCallPos and innerCallPos - Make MappedArr.callPos accessible as private[Val] for extraction - Add comprehensive fused map chain tests (full materialization, repeated indexed access, reverse, foldl) Result: Correct error positions when inner function of a fused map chain throws.
He-Pin
force-pushed
the
perf/map-map-view-fusion
branch
from
4d40b1d to
225dfbf
Compare
Contributor
Author
|
I think this scenario should theoretically be very rare, so I'll disable it for now. |
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
When
std.map(f, std.map(g, arr))is evaluated, twoMappedArrviews are nested. Each element access traverses two cache layers and two levels of indirection. This pattern is common in Jsonnet when splitting transformation logic across multiple map calls for readability.Modification
In
Arr.mapped(), detect when the source is already aMappedArrwith live state and create aComposedMappedArrthat applies both functions in sequence from the original source array. This eliminates one cache layer and one level of indirection per element access.Key design decisions:
outerCallPosandinnerCallPosfor correct error stack framesinner.source != nullto avoid using a fully-consumed (released) inner viewLazyViewArr— repeated access to the same index returns the cached result without recomputationMappedArrfields (source,func,callPos) widened toprivate[Val]for extractionResult
Scala Native A/B (Scala Native 0.5.12, macOS arm64, hyperfine --warmup 3):
JVM JMH (JDK 21, G1GC, -Xmx4G, @fork(1) @WarmUp(1) @measurement(1)):
Comprehensive test coverage added in
lazy_array_views.jsonnet: full materialization, repeated indexed access, reverse, and foldl on fused chains.References