Repository navigation
[SPARK-1527] change rootDir*.getName to rootDir*.getAbsolutePath - #436
advancedxy wants to merge 1 commit into
Conversation
directories are deleted when the test is finished.
|
Can one of the admins verify this patch? |
|
As discussed with @srowen on JIRA, I think maybe we should review that relative paths and absolute paths are used appropriately. |
|
JIRA issue: SPARK-1527 can one of the admin please verify this patch ?! Thanks. Niraj |
|
Jenkins, test this please. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
Rename VertexID -> VertexId in GraphX
|
@pwendell On the subject of SPARK-1623, this PR / JIRA can also go in, in my opinion. It's a related small good fix to a potential test problem. Then I think both are done. |
|
Seems like a good (and pretty minor) change. I'll pull this in, thanks. |
JIRA issue: [SPARK-1527](https://issues.apache.org/jira/browse/SPARK-1527) getName() only gets the last component of the file path. When deleting test-generated directories, we should pass the generated directory's absolute path to DiskBlockManager. Author: Ye Xianjin <[email protected]> This patch had conflicts when merged, resolved by Committer: Patrick Wendell <[email protected]> Closes #436 from advancedxy/SPARK-1527 and squashes the following commits: 4678bab [Ye Xianjin] change rootDir*.getname to rootDir*.getAbsolutePath so the temporary directories are deleted when the test is finished. (cherry picked from commit 753b04d) Signed-off-by: Patrick Wendell <[email protected]>
JIRA issue: [SPARK-1527](https://issues.apache.org/jira/browse/SPARK-1527) getName() only gets the last component of the file path. When deleting test-generated directories, we should pass the generated directory's absolute path to DiskBlockManager. Author: Ye Xianjin <[email protected]> This patch had conflicts when merged, resolved by Committer: Patrick Wendell <[email protected]> Closes apache#436 from advancedxy/SPARK-1527 and squashes the following commits: 4678bab [Ye Xianjin] change rootDir*.getname to rootDir*.getAbsolutePath so the temporary directories are deleted when the test is finished.
Rename VertexID -> VertexId in GraphX (cherry picked from commit 3d9e66d) Signed-off-by: Reynold Xin <[email protected]>
Fix devstack trove deployment failling
### What changes were proposed in this pull request? Task 212: a new kernel reaches the JIT on its first query. Task 195 found Varka's first query slower than vanilla's because a new kernel runs interpreted, and the admission check (apache#431) found why nothing simple fixes it: C2 needs hundreds of milliseconds per kernel method, so however early the compile is triggered, the query ends before it lands. So the first batches must not wait for it. This PR makes a new shape's batches take Spark's row path while a background thread gets the kernel compiled, and switches every task of the shape to the kernel once the JVM says it is compiled. **The mechanism.** - **Each cached shape carries its warm state** (`VarkaKernelWarmth`: cold, warming, compiled, released), shared by every task that runs the shape. `serveBatch` sends a batch the kernel could serve to the node's own row path while the shape is not ready, and counts it in a new metric, `numWarmupBatches`, apart from the fallbacks. A Varka node above another counts the batches the node below sends from its row path while its kernel warms as warm-up batches too. - **The first task to meet a cold shape queues its warm-up.** It copies its batch's kernel inputs, tiled to 1024 rows, and `VarkaKernelWarmup` - one daemon thread per JVM - runs a fresh instance of the kernel on the copy in short calls: 32 rows plus the batch's length modulo 32, which keeps the batch's own remainder past its last lane group, so the epilogues see what real batches give them. Short calls advance HotSpot's invocation counters hundreds of times faster per row than 625-iteration calls on real batches. - **Both of the kernel's drivers are warmed.** A kernel has one driver for batches whose inputs have no nulls and one for batches with them, and the claiming batch says nothing about which the later batches need. The calls alternate between the two; the masked driver's calls pass each input with and without validity, so every per-input null test is profiled both ways, and the dense driver's calls read the values under the batch's nulls after they are replaced with a valid value of the same input. Where no kernel input is nullable, no batch can reach the masked driver and only the dense one is warmed. - **The switch is the JVM's verdict, not a count.** An uncompiled kernel boxes a vector per operation; a compiled one allocates only the memory segments its driver makes per column. A probe block of sixteen calls is clean when it is within the species-pollution check's allowance plus 256 bytes per column per call and a quarter of the first block; two clean blocks are the verdict. A shape without a verdict sixty seconds after its warm-up was queued, a shape leaving the cache, or a failing kernel is released, and its tasks run the kernel, which the warm-up's calls have already profiled. A shape that finds the queue full hands its claim back for a later batch. - **C1 is kept off the warmed kernel classes.** Building this exposed a HotSpot trap that the per-batch path has too: a kernel method that crosses its first threshold while C2's queue is long is compiled by C1 at tier 2 instead of tier 3, fits there where tier 3 does not ("out of virtual registers"), and is then stranded on boxing C1 code for the life of its class - from tier 2 the policy climbs only to tier 3, which fails, and tier-2 code does not profile. `-XX:+PrintTieredEvents` showed it, and an in-process compile-queue dump showed nothing pending. `VarkaKernelCompileDirective` adds one compiler directive per JVM, `c1: { Exclude: true }`, so every warmed kernel method goes from the interpreter to C2. The directive has a price: the failed tier-3 request is also what creates a method's profile, so a kernel fed only by its own batches would reach C2 about a hundred batches later. A directive matches by class name, so the decision to warm is made when a kernel is emitted and carried in the shape key, and a warmed kernel gets a name of its own (`VarkaFusedProjection_w...`) that the directive matches and no other kernel has. A session with the warm-up off - or a JVM that cannot warm: C1 only, a JVMCI compiler, no allocation accounting, a directive it did not accept - emits its kernels under the plain name and compiles them exactly as before. - **`spark.sql.codegen.varka.warmup.enabled`**, on by default. Test sessions and benchmark sessions pin it off where they check that a kernel served a shape's first batch. **The results** (the quiet laptop, 512-bit lanes, best of five, ms for 100k rows; measured at the review's commit, and `PLAN_TASK_212.md` 10.8 scores the five predictions registered for it in 10.7): | Entries | vanilla first | vanilla second | Varka first, before | warm-up first | once compiled | kernel ready after | | --: | --: | --: | --: | --: | --: | --: | | 16 | 115 | 52 | 232 | 169 | 26 | 1.4 to 1.8 s | | 54 | 448 | 427 | 694 | 342 | 27 | 5.3 to 6.0 s | | 100 | 867 | 812 | 1261 | 619 | 37 | 11.1 to 12.2 s | - Past the cliff the first run is 0.71 to 0.76 times vanilla's; below it, 1.40 to 1.82 times (the per-row projection is slower than vanilla's whole-stage loop - a follow-up candidate). - Every warm-up verdict at every rung and both widths was a compile. Over two million rows the kernel compiled from the warm-up runs within 3% of one compiled from its own batches, so the short calls cost C2's code nothing. - Warming both drivers made every verdict 1.6 to 2.0 times later than before the review's fixes: this benchmark's column has a null in every batch, so the dense driver it now compiles as well serves none of its batches. The first query is unchanged; the time until the kernel takes over roughly doubled. Row 213 (apache#436) is for this case - the Arrow cache's per-batch null counts would say the dense driver is not needed. - At 128-bit lanes the gain is larger: 165 against 741 ms at 16 entries, 588 against 4422 at 100. - The directive's A/B (`VarkaWarmupDirectiveBenchmark`, three fresh JVMs each way): without it all six kernels were stranded and still boxing at the 60-second deadline, the 16-entry one included; with it all six compiled, in 1.9 to 6.2 s. - A session with the warm-up off no longer pays for the directive: the per-batch arm's two-million-row averages at 512 bits fell from 1726 to 471 ms at 54 entries and from 3416 to 1152 at 100. **The review's fixes** (a `/code-review max` of this branch; `PLAN_TASK_212.md` 10.7): the directive's scope and the two drivers above, which were the two findings that undercut the design as first written - the directive, once added, reached the kernels of sessions with the warm-up off in the same JVM, and the warm-up compiled only the driver of its claiming batch. Also: no warm-up where the JVM cannot reach C2 or refused the directive, and the directive file's path is passed in quotes; the deadline counts from the queueing; a dead worker is replaced and inherits no task's thread-locals or class loader; a batch declined while it is copied for a warm-up counts as declined; the directive benchmark's child JVM finds its class under `spark-submit` and cannot hang the parent; the planner's size admission builds the same key as the evaluator, so a shape is emitted once; the README's quick start names the warm-up. ### Why are the changes needed? The second post states Varka's first-query cost as one of its bounds. Before this PR that bound read "slower than vanilla until the JIT notices, which a short query never lets it do"; the numbers above are what it reads now. ### Does this PR introduce _any_ user-facing change? Yes, for sessions with Varka enabled: a new shape's first batches run on Spark's per-row path until its kernel is compiled, visible as `numWarmupBatches` in the SQL UI, and a background thread per executor JVM runs the warm-ups. The results are unchanged - the row path is the engine the kernels are checked against. `spark.sql.codegen.varka.warmup.enabled=false` restores the previous behaviour. ### How was this patch tested? - `VarkaKernelWarmupSuite` (catalyst): the batch copy's tiling, validity bits included, and its filling of null rows; a warm-up of a real emitted kernel reaching COMPILED with the allocation evidence; a warm-up claimed by a batch with no nulls, some nulls or only nulls leaving both drivers compiled, measured by what 1024-row calls of each kind allocate; a shape leaving the cache stopping its warm-up at once; the claim protocol; the directive installed in a tiered JVM, asserted rather than assumed; and only a warmed kernel's class name matching it. - `VarkaWarmupEndToEndSuite`: a projection-to-rows, a filter-to-rows and a columnar noop write each run their first query on the row path and the next on the compiled kernel, with the row engine's answers both times; a projection over a filter counts the filter's warm-up batches as its own warm-up, with no non-Arrow fallback, until both kernels serve; a batch declined during the copy counts as declined. - Before the review's fixes, every Varka suite in catalyst (418) and sql (394), the opt-in JFR compilation-watch tests, `SetCommandSuite` and `SparkConfigBindingPolicySuite`. After them, `VarkaKernelWarmupSuite`, `VarkaShapeCacheSuite`, `VarkaWarmupEndToEndSuite`, `VarkaFilterExecSuite`, `VarkaKernelEvaluatorSuite`, `VarkaAutoRegistrationSuite` and `VarkaDifferentialSuite` (32 and 145 tests). - `dev/scalastyle`, `dev/lint-java`, the 100-column and non-ASCII scans, the quote check and the Varka pre-commit checks pass. - The benchmarks above, re-run at the review's commit on the quiet laptop with the canary passing. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5)
JIRA issue: SPARK-1527
getName() only gets the last component of the file path. When deleting test-generated directories,
we should pass the generated directory's absolute path to DiskBlockManager.