[SPARK-56508][SQL][TESTS][FOLLOWUP] Stabilize VectorizedRleValuesReaderBenchmark Group runBooleanBenchmark/runIntegerBenchmark with a pre-warm pass#55497
Closed
LuciferYang wants to merge 5 commits intoapache:masterfrom
Conversation
…erBenchmark Group A/B with a pre-warm pass ### What changes were proposed in this pull request? Add a pre-warm pass (3 iterations of fresh-reader + initFromPage + decode) before the cold-reader `benchmark.addCase` call in `runBooleanBenchmark` (Group A) and `runIntegerBenchmark` (Group B) of `VectorizedRleValuesReaderBenchmark`. ### Why are the changes needed? Reviewer feedback on SPARK-56522 (PR apache#55386) flagged first-case `Best Time(ms) = 0` variance in Groups A/B: the first case in each group pays for tiered-compilation transitions on sub-millisecond iterations, producing inconsistent baseline numbers between re-runs. Groups C/D don't show this because their setup reuses a pre-warmed reader before each `addCase`. The cold-reader variants in Groups A/B instantiate a fresh reader per iteration, so the shared pre-warm (`warmReader.readBooleans` / `warmReader.readIntegers`) doesn't fully cover the allocation + `initFromPage` path that `cold reader` exercises. Running the cold-reader code path explicitly 3 times before `addCase` lets HotSpot settle on C2 before measurement. ### Does this PR introduce _any_ user-facing change? No. Benchmark-only change. ### How was this patch tested? - Compile: `build/sbt sql/Test/compile` clean - Will regenerate result files on GHA to verify reduced first-case variance ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.7
…uet.VectorizedRleValuesReaderBenchmark (JDK 17, Scala 2.13, split 1 of 1)
…uet.VectorizedRleValuesReaderBenchmark (JDK 25, Scala 2.13, split 1 of 1)
…uet.VectorizedRleValuesReaderBenchmark (JDK 21, Scala 2.13, split 1 of 1)
…uet.VectorizedRleValuesReaderBenchmark (JDK 17, Scala 2.13, split 1 of 1)
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.
What changes were proposed in this pull request?
Add a pre-warm pass (3 iterations of fresh-reader + initFromPage + decode) before the cold-reader
benchmark.addCasecall inrunBooleanBenchmarkandrunIntegerBenchmarkofVectorizedRleValuesReaderBenchmark.Why are the changes needed?
Reviewer feedback on SPARK-56522 (PR #55386) flagged first-case
Best Time(ms) = 0variance in GroupsrunBooleanBenchmark/runIntegerBenchmark: the first case in each group pays for tiered-compilation transitions on sub-millisecond iterations, producing inconsistent baseline numbers between re-runs.Groups
runNullableBatchBenchmarkdon't show this because their setup reuses a pre-warmed reader before eachaddCase. The cold-reader variants in GroupsrunBooleanBenchmark/runIntegerBenchmarkinstantiate a fresh reader per iteration, so the shared pre-warm (warmReader.readBooleans/warmReader.readIntegers) doesn't fully cover the allocation +initFromPagepath thatcold readerexercises. Running the cold-reader code path explicitly 3 times beforeaddCaselets HotSpot settle on C2 before measurement.Does this PR introduce any user-facing change?
No. Benchmark-only change.
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.7