Support: persist scene-test kernel compilation cache - #1675
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds persistent, content-sensitive caching for compiled scene-test ChangesScene-Test Compilation Cache
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CI as Hardware CI
participant Pytest as pytest
participant Warmup as scene_test_compile
participant Cache as scene_test_cache
participant Compiler as scene_test.py
CI->>CI: Restore build/cache/kernels
CI->>Warmup: Run selected tests with --collect-only
Warmup->>Pytest: Collect scene-test items
Pytest-->>Warmup: Return collected classes
Warmup->>Compiler: Compile selected classes
Compiler->>Cache: Load or compile ChipCallable
Cache-->>Compiler: Return cached artifact
CI->>Pytest: Execute hardware tests
Pytest->>Cache: Reuse compiled artifacts
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@simpler_setup/scene_test.py`:
- Around line 1032-1055: The artifact metadata built in the compile_artifact_key
call must include a compiler cache token covering the selected compiler version,
relevant compile/link flags, and a schema token for KernelCompiler behavior.
Update the surrounding artifact-key generation to use this token, and add a
regression test proving that changing the schema token invalidates the artifact
key.
In `@simpler_setup/tools/scene_test_compile.py`:
- Around line 24-25: Update the warm-up filtering around item.iter_markers() to
evaluate active pytest skipif conditions, not only markers named "skip", and
skip compilation whenever pytest would skip the collected item. Add a regression
test covering a truthy pytest.mark.skipif condition while preserving compilation
for inactive conditions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bdf8b3c9-b100-41e8-8b49-59279b55f34a
📒 Files selected for processing (11)
.github/workflows/_st-npu.ymldocs/ci.mddocs/investigations/2026-07-qwen-scene-test-406s-decomposition.mddocs/testing.mdsimpler_setup/kernel_compiler.pysimpler_setup/scene_test.pysimpler_setup/scene_test_cache.pysimpler_setup/tools/README.mdsimpler_setup/tools/scene_test_compile.pytests/ut/py/test_scene_test_cache.pytests/ut/py/test_scene_test_compile.py
| artifact_key = compile_artifact_key( | ||
| { | ||
| "cache_key": cache_key, | ||
| "platform": platform, | ||
| "runtime": runtime, | ||
| "host_platform": sys.platform, | ||
| "host_machine": host_platform.machine(), | ||
| "sanitizers": KernelCompiler._sanitizers, | ||
| "orchestration": { | ||
| "function_name": orch["function_name"], | ||
| "config_name": orch.get("config_name", ""), | ||
| "signature": orch.get("signature", []), | ||
| }, | ||
| "incores": [ | ||
| { | ||
| "func_id": k["func_id"], | ||
| "core_type": k["core_type"], | ||
| "signature": k.get("signature", []), | ||
| } | ||
| for k in incores | ||
| ], | ||
| }, | ||
| compilation_units, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Include compiler identity in the artifact key.
The key omits the effective compiler identity and compilation flags. The workflow restore prefix can restore artifacts from an earlier ref. If KernelCompiler behavior, the CANN toolchain, or compiler flags change without a source change, this code can reuse an incompatible compiled binary.
Add a compiler cache token to the metadata. Include the selected compiler version, relevant compile/link flags, and a schema token that changes with compiler behavior. Add a regression test for invalidation after that token changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@simpler_setup/scene_test.py` around lines 1032 - 1055, The artifact metadata
built in the compile_artifact_key call must include a compiler cache token
covering the selected compiler version, relevant compile/link flags, and a
schema token for KernelCompiler behavior. Update the surrounding artifact-key
generation to use this token, and add a regression test proving that changing
the schema token invalidates the artifact key.
eacc55c to
98b2456
Compare
98b2456 to
6d10af8
Compare
Store content-addressed ChipCallable artifacts with transitive include, compiler identity, flag, schema, ABI, and integrity invalidation. Warm selected callables before task-submit without acquiring an NPU. Compilation is serial by default; simpler CI opts into eight host workers and preserves artifacts through actions/cache. Tests cover reuse, invalidation, corruption, concurrency, skip selection, and configurable warm-up parallelism. Refs hw-native-sys#1604
1 participant
Summary
ChipCallableartifacts underbuild/cache/kernels, with source/include-closure, compiler identity/flags/schema, and ABI invalidation, plus integrity validation, atomic publication, and per-key locking--compile-workers 8beforetask-submitwithout acquiring an NPUactions/cache, while keeping the existing batch-level device acquisition and separate SDMA executionLock-boundary measurement
qwen3_14b_decode::StressBatch16Seq3500on a2a3, with both device jobs acquiring immediately:task-submitwallThe cold-cache path moves compilation out of the device allocation rather than shortening first-run wall time. Device-lock wall falls by 77.42 s (58.8%); restored caches also reduce total wall time.
Fixture construction and golden computation remain inside the locked pytest invocation and are intentionally outside this PR's scope.
Testing
--compile-workers 8and rejection of zero workersqwen3_14b_decode::StressBatch16Seq3500: baseline, cold warm-up, cached device run, and hot warm-up all passedRefs #1604