Support any layout in Matrix.apply(rowRange, colRange) gather - #148
Merged
Conversation
apply(rowRange, colRange) takes a zero-copy submatrix when both selections are contiguous runs, and otherwise gathers the selected elements into a fresh matrix. That gather was guarded on `m.isDenseColMajor` with a bare `???` for everything else, so gathering from a row-major, offset or padded matrix threw NotImplementedError. The guard was buying nothing. Its hardcoded `colpos * m.rows + rowPos` is exactly what `m.layout.linearIndex(rowPos, colpos)` evaluates to when rowStride == 1, colStride == rows and offset == 0 — i.e. under precisely the condition it tested — and linearIndex is @thin, so the general form costs nothing over the special case. The two branches therefore collapse into one that is correct for every layout. Same reasoning, and same outcome, as the layout-specific branches already removed from update(row, col, value) and reduceAlongDimension. Also moved the result array allocation inside the gather branch. It sat above the `if`, so every zero-copy submatrix call allocated a full result-sized array and immediately discarded it. Tests: the existing suite covered the gather on exactly one layout — dense column-major, the one case the guard admitted — so the new path had no coverage at all. Added three fixtures that are the same logical matrix as mat1to9 in layouts the guard rejected (dense row-major, a padded colStride, and an offset submatrix view whose backing array holds the parent's other elements, which a mis-derived index would pick up). Every gather shape is asserted against dense column-major as the reference, so the branches must agree rather than merely not throw. Also covers out-of-order and repeated indices — gather semantics the method has always had but nothing exercised — and that the result is a copy rather than a view.
Bytecode audit (Tier 1) — ✅ cleanJDK 25.0.1 (major 25), OpenJDK 64-Bit Server VM Vector lanes (
none Annotated methods (182)
Method sizes
Proposed baseline{
"jdkMajor": 25,
"c9": { "totalBytes": 36598, "distinctOps": 205 },
"annotated": {
"vecxt.IntArraysX$.contiguous([I)Z": 39,
"vecxt.IntArraysX$.select([I[I)[I": 44,
"vecxt.NDArrayDoubleOps$.binaryOpGeneral(Lvecxt/ndarray$NDArray;Lvecxt/ndarray$NDArray;Lscala/Function2;)Lvecxt/ndarray$NDArray;": 186,
"vecxt.NDArrayDoubleOps$.binaryOpInPlaceGeneral(Lvecxt/ndarray$NDArray;Lvecxt/ndarray$NDArray;Lscala/Function2;)V": 153,
"vecxt.NDArrayDoubleOps$.compareGeneral(Lvecxt/ndarray$NDArray;Lvecxt/ndarray$NDArray;Lscala/Function2;)Lvecxt/ndarray$NDArray;": 186,
"vecxt.NDArrayDoubleOps$.compareScalarGeneral(Lvecxt/ndarray$NDArray;DLscala/Function2;)Lvecxt/ndarray$NDArray;": 157,
"vecxt.NDArrayDoubleOps$.unaryOpGeneral(Lvecxt/ndarray$NDArray;Lscala/Function1;)Lvecxt/ndarray$NDArray;": 152,
"vecxt.NDArrayFloatOps$.binaryOpGeneral(Lvecxt/ndarray$NDArray;Lvecxt/ndarray$NDArray;Lscala/Function2;)Lvecxt/ndarray$NDArray;": 195,
"vecxt.NDArrayFloatOps$.binaryOpInPlaceGeneral(Lvecxt/ndarray$NDArray;Lvecxt/ndarray$NDArray;Lscala/Function2;)V": 162,
"vecxt.NDArrayFloatOps$.compareGeneral(Lvecxt/ndarray$NDArray;Lvecxt/ndarray$NDArray;Lscala/Function2;)Lvecxt/ndarray$NDArray;": 195,
"vecxt.NDArrayFloatOps$.compareScalarGeneral(Lvecxt/ndarray$NDArray;FLscala/Function2;)Lvecxt/ndarray$NDArray;": 165,
"vecxt.NDArrayIntOps$.binaryOpGeneral(Lvecxt/ndarray$NDArray;Lvecxt/ndarray$NDArray;Lscala/Function2;)Lvecxt/ndarray$NDArray;": 186,
"vecxt.NDArrayIntOps$.binaryOpInPlaceGeneral(Lvecxt/ndarray$NDArray;Lvecxt/ndarray$NDArray;Lscala/Function2;)V": 153,
"vecxt.NDArrayIntOps$.compareGeneral(Lvecxt/ndarray$NDArray;Lvecxt/ndarray$NDArray;Lscala/Function2;)Lvecxt/ndarray$NDArray;": 186,
"vecxt.NDArrayIntOps$.compareScalarGeneral(Lvecxt/ndarray$NDArray;ILscala/Function2;)Lvecxt/ndarray$NDArray;": 156,
"vecxt.NDArrayIntOps$.unaryOpGeneral(Lvecxt/ndarray$NDArray;Lscala/Function1;)Lvecxt/ndarray$NDArray;": 152,
"vecxt.doublearrays$.$div([DDII[DI)V": 116,
"vecxt.doublearrays$.$minus$bang([D)V": 98,
"vecxt.doublearrays$.$minus$eq([DD)V": 90,
"vecxt.doublearrays$.$minus$eq([D[D)V": 26,
"vecxt.doublearrays$.$minus([DDII[DI)V": 116,
"vecxt.doublearrays$.$minus([D[D)[D": 24,
"vecxt.doublearrays$.$plus$eq([DD)V": 90,
"vecxt.doublearrays$.$plus([DDII[DI)V": 116,
"vecxt.doublearrays$.$times$eq([D[D)V": 88,
"vecxt.doublearrays$.$times$times$bang([DD)V": 95,
"vecxt.doublearrays$.abs$bang([D)V": 98,
"vecxt.doublearrays$.abs([D)[D": 108,
"vecxt.doublearrays$.acos$bang([D)V": 98,
"vecxt.doublearrays$.acos([D)[D": 108,
"vecxt.doublearrays$.asin$bang([D)V": 98,
"vecxt.doublearrays$.asin([D)[D": 108,
"vecxt.doublearrays$.atan$bang([D)V": 98,
"vecxt.doublearrays$.atan([D)[D": 108,
"vecxt.doublearrays$.cbrt$bang([D)V": 98,
"vecxt.doublearrays$.cbrt([D)[D": 108,
"vecxt.doublearrays$.clamp$bang([DDD)V": 180,
"vecxt.doublearrays$.cos$bang([D)V": 98,
"vecxt.doublearrays$.cos([D)[D": 108,
"vecxt.doublearrays$.cosh$bang([D)V": 98,
"vecxt.doublearrays$.cosh([D)[D": 108,
"vecxt.doublearrays$.cumsum$bang([D)V": 27,
"vecxt.doublearrays$.cumsum([D)[D": 15,
"vecxt.doublearrays$.dot([D[D)D": 23,
"vecxt.doublearrays$.exp$bang([D)V": 98,
"vecxt.doublearrays$.exp([D)[D": 108,
"vecxt.doublearrays$.expm1$bang([D)V": 98,
"vecxt.doublearrays$.expm1([D)[D": 108,
"vecxt.doublearrays$.fillLinspace([DDD)V": 133,
"vecxt.doublearrays$.fma$bang([DDD)V": 85,
"vecxt.doublearrays$.increments([D)[D": 110,
"vecxt.doublearrays$.log$bang([D)V": 98,
"vecxt.doublearrays$.log([D)[D": 108,
"vecxt.doublearrays$.log10$bang([D)V": 98,
"vecxt.doublearrays$.log10([D)[D": 108,
"vecxt.doublearrays$.log1p$bang([D)V": 98,
"vecxt.doublearrays$.log1p([D)[D": 108,
"vecxt.doublearrays$.meanAndVariance([D)Lvecxt/MeanAndVariance;": 9,
"vecxt.doublearrays$.norm([D)D": 13,
"vecxt.doublearrays$.productSIMD([D)D": 86,
"vecxt.doublearrays$.sin$bang([D)V": 98,
"vecxt.doublearrays$.sin([D)[D": 108,
"vecxt.doublearrays$.sinh$bang([D)V": 98,
"vecxt.doublearrays$.sinh([D)[D": 108,
"vecxt.doublearrays$.sqrt$bang([D)V": 98,
"vecxt.doublearrays$.sqrt([D)[D": 108,
"vecxt.doublearrays$.sumSIMD([D)D": 85,
"vecxt.doublearrays$.sumSIMD([DII)D": 102,
"vecxt.doublearrays$.tan$bang([D)V": 98,
"vecxt.doublearrays$.tan([D)[D": 108,
"vecxt.doublearrays$.tanh$bang([D)V": 98,
"vecxt.doublearrays$.tanh([D)[D": 108,
"vecxt.doublearrays$.unary_$minus([D)[D": 108,
"vecxt.doublearrays$.variance([DLvecxt/VarianceMode;)D": 10,
"vecxt.doublearrays.meanAndVariance([DLvecxt/VarianceMode;)Lvecxt/MeanAndVariance;": 9,
"vecxt.floatarrays$.$minus$bang([F)V": 93,
"vecxt.floatarrays$.$minus$eq([FF)V": 84,
"vecxt.floatarrays$.$minus$eq([F[F)V": 26,
"vecxt.floatarrays$.$minus([F[F)[F": 24,
"vecxt.floatarrays$.$plus$eq([FF)V": 84,
"vecxt.floatarrays$.$plus$eq([F[F)V": 24,
"vecxt.floatarrays$.$times$eq([FF)V": 78,
"vecxt.floatarrays$.$times$eq([F[F)V": 84,
"vecxt.floatarrays$.$times$times$bang([FF)V": 85,
"vecxt.floatarrays$.abs$bang([F)V": 93,
"vecxt.floatarrays$.abs([F)[F": 104,
"vecxt.floatarrays$.acos$bang([F)V": 93,
"vecxt.floatarrays$.acos([F)[F": 104,
"vecxt.floatarrays$.asin$bang([F)V": 93,
"vecxt.floatarrays$.asin([F)[F": 104,
"vecxt.floatarrays$.atan$bang([F)V": 93,
"vecxt.floatarrays$.atan([F)[F": 104,
"vecxt.floatarrays$.cbrt$bang([F)V": 93,
"vecxt.floatarrays$.cbrt([F)[F": 104,
"vecxt.floatarrays$.clamp$bang([FFF)V": 172,
"vecxt.floatarrays$.cos$bang([F)V": 93,
"vecxt.floatarrays$.cos([F)[F": 104,
"vecxt.floatarrays$.cosh$bang([F)V": 93,
"vecxt.floatarrays$.cosh([F)[F": 104,
"vecxt.floatarrays$.cumsum$bang([F)V": 27,
"vecxt.floatarrays$.cumsum([F)[F": 15,
"vecxt.floatarrays$.dot([F[F)F": 23,
"vecxt.floatarrays$.exp$bang([F)V": 93,
"vecxt.floatarrays$.exp([F)[F": 104,
"vecxt.floatarrays$.expm1$bang([F)V": 93,
"vecxt.floatarrays$.expm1([F)[F": 104,
"vecxt.floatarrays$.fma$bang([FFF)V": 80,
"vecxt.floatarrays$.increments([F)[F": 97,
"vecxt.floatarrays$.log$bang([F)V": 93,
"vecxt.floatarrays$.log([F)[F": 104,
"vecxt.floatarrays$.log10$bang([F)V": 93,
"vecxt.floatarrays$.log10([F)[F": 104,
"vecxt.floatarrays$.log1p$bang([F)V": 93,
"vecxt.floatarrays$.log1p([F)[F": 104,
"vecxt.floatarrays$.norm([F)F": 13,
"vecxt.floatarrays$.productSIMD([F)F": 82,
"vecxt.floatarrays$.sin$bang([F)V": 93,
"vecxt.floatarrays$.sin([F)[F": 104,
"vecxt.floatarrays$.sinh$bang([F)V": 93,
"vecxt.floatarrays$.sinh([F)[F": 104,
"vecxt.floatarrays$.sqrt$bang([F)V": 93,
"vecxt.floatarrays$.sqrt([F)[F": 104,
"vecxt.floatarrays$.sumSIMD([F)F": 81,
"vecxt.floatarrays$.tan$bang([F)V": 93,
"vecxt.floatarrays$.tan([F)[F": 104,
"vecxt.floatarrays$.tanh$bang([F)V": 93,
"vecxt.floatarrays$.tanh([F)[F": 104,
"vecxt.floatarrays$.unary_$minus([F)[F": 104,
"vecxt.intarrays$.$bang$colon$eq([II)[Z": 94,
"vecxt.intarrays$.$bang$colon$eq([I[I)[Z": 106,
"vecxt.intarrays$.$eq$colon$eq([II)[Z": 94,
"vecxt.intarrays$.$eq$colon$eq([I[I)[Z": 106,
"vecxt.intarrays$.$greater$eq([II)[Z": 94,
"vecxt.intarrays$.$greater$eq([I[I)[Z": 106,
"vecxt.intarrays$.$greater([II)[Z": 94,
"vecxt.intarrays$.$greater([I[I)[Z": 106,
"vecxt.intarrays$.$less$eq([II)[Z": 94,
"vecxt.intarrays$.$less$eq([I[I)[Z": 106,
"vecxt.intarrays$.$less([II)[Z": 94,
"vecxt.intarrays$.$less([I[I)[Z": 106,
"vecxt.intarrays$.$minus$eq([II)V": 67,
"vecxt.intarrays$.$minus$eq([I[I)V": 84,
"vecxt.intarrays$.$minus([II)[I": 16,
"vecxt.intarrays$.$minus([I[I)[I": 24,
"vecxt.intarrays$.$plus$eq([I[I)V": 84,
"vecxt.intarrays$.$plus([I[I)[I": 24,
"vecxt.intarrays$.countsToIdx([I)[I": 70,
"vecxt.intarrays$.dot([I[I)I": 108,
"vecxt.intarrays$.increments([I)[I": 121,
"vecxt.intarrays$.maxSIMD([I)I": 82,
"vecxt.intarrays$.mean([I)D": 11,
"vecxt.intarrays$.meanAndVariance([I)Lvecxt/MeanAndVariance;": 9,
"vecxt.intarrays$.minSIMD([I)I": 82,
"vecxt.intarrays$.std([I)D": 9,
"vecxt.intarrays$.std([ILvecxt/VarianceMode;)D": 10,
"vecxt.intarrays$.stdDev([I)D": 9,
"vecxt.intarrays$.sumSIMD([I)I": 81,
"vecxt.intarrays$.variance([I)D": 9,
"vecxt.intarrays$.variance([ILvecxt/VarianceMode;)D": 10,
"vecxt.intarrays.gt([II)[Z": 9,
"vecxt.intarrays.gt([I[I)[Z": 9,
"vecxt.intarrays.gte([II)[Z": 9,
"vecxt.intarrays.gte([I[I)[Z": 9,
"vecxt.intarrays.lt([II)[Z": 9,
"vecxt.intarrays.lt([I[I)[Z": 9,
"vecxt.intarrays.lte([II)[Z": 9,
"vecxt.intarrays.lte([I[I)[Z": 9,
"vecxt.intarrays.meanAndVariance([ILvecxt/VarianceMode;)Lvecxt/MeanAndVariance;": 9,
"vecxt.intarrays.stdDev([ILvecxt/VarianceMode;)D": 9,
"vecxt.matrix$Layout.linearIndex(II)I": 19,
"vecxt.matrix$Matrix.colStride()I": 8,
"vecxt.matrix$Matrix.cols()I": 8,
"vecxt.matrix$Matrix.hasSimpleContiguousMemoryLayout()Z": 8,
"vecxt.matrix$Matrix.isDenseColMajor()Z": 8,
"vecxt.matrix$Matrix.isDenseRowMajor()Z": 8,
"vecxt.matrix$Matrix.numel()I": 8,
"vecxt.matrix$Matrix.offset()I": 8,
"vecxt.matrix$Matrix.rowStride()I": 8,
"vecxt.matrix$Matrix.rows()I": 8,
"vecxt.ndarray$.mkNDArray(Ljava/lang/Object;[I[II)Lvecxt/ndarray$NDArray;": 13,
"vecxt.ndarray.shapeArray(Lvecxt/ndarray$NDArray;)[I": 8,
"vecxt.ndarrayOps.expandDims(Lvecxt/ndarray$NDArray;I)Lvecxt/ndarray$NDArray;": 9
}
}
|
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.
apply(rowRange, colRange) takes a zero-copy submatrix when both selections are contiguous runs, and otherwise gathers the selected elements into a fresh matrix. That gather was guarded on
m.isDenseColMajorwith a bare???for everything else, so gathering from a row-major, offset or padded matrix threw NotImplementedError.The guard was buying nothing. Its hardcoded
colpos * m.rows + rowPosis exactly whatm.layout.linearIndex(rowPos, colpos)evaluates to when rowStride == 1, colStride == rows and offset == 0 — i.e. under precisely the condition it tested — and linearIndex is @thin, so the general form costs nothing over the special case. The two branches therefore collapse into one that is correct for every layout. Same reasoning, and same outcome, as the layout-specific branches already removed from update(row, col, value) and reduceAlongDimension.Also moved the result array allocation inside the gather branch. It sat above the
if, so every zero-copy submatrix call allocated a full result-sized array and immediately discarded it.Tests: the existing suite covered the gather on exactly one layout — dense column-major, the one case the guard admitted — so the new path had no coverage at all. Added three fixtures that are the same logical matrix as mat1to9 in layouts the guard rejected (dense row-major, a padded colStride, and an offset submatrix view whose backing array holds the parent's other elements, which a mis-derived index would pick up). Every gather shape is asserted against dense column-major as the reference, so the branches must agree rather than merely not throw. Also covers out-of-order and repeated indices — gather semantics the method has always had but nothing exercised — and that the result is a copy rather than a view.