CI: reduce packaging, profiling, and unit-test overhead - #1805
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 change updates runtime build reuse, conditional compiler verbosity, packaging dependency validation, isolated packaging smoke checks, and several GitHub Actions workflows for profiling and hardware tests. Documentation now reflects minimum dependency versions and the packaging verification matrix. ChangesBuild and CI updates
Estimated code review effort: 3 (Moderate) | ~25 minutes 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: 1
🔇 Additional comments (19)
.github/workflows/_packaging.yml (2)
44-57: LGTM!
73-73: LGTM!docs/getting-started.md (1)
110-119: LGTM!docs/python-packaging.md (2)
126-126: LGTM!
161-165: LGTM!tools/verify_packaging.sh (3)
32-34: LGTM!
44-88: LGTM!
91-145: LGTM!simpler_setup/runtime_builder.py (1)
309-315: LGTM!Also applies to: 326-328, 405-411
tests/ut/py/test_build_runtimes.py (1)
12-49: LGTM!tests/ut/py/test_runtime_builder.py (1)
282-301: LGTM!simpler_setup/runtime_compiler.py (1)
443-445: LGTM!tests/ut/py/test_runtime_compiler.py (1)
14-35: LGTM!.github/workflows/_profiling-flags-smoke.yml (1)
74-80: 🚀 Performance & Scalability
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the helper-library cache lifetime.
Line 82 starts a new
.venv/bin/pythonprocess for each architecture and profiling combination. A cache held only in process state cannot survive between those invocations. If reuse is process-local, this loop order does not share helper builds betweena2a3simanda5sim; use one build process for both platforms or a persistent cache keyed byDEFS..github/workflows/_ut-no-hardware.yml (2)
52-52: 🩺 Stability & Availability
⚠️ Unverified finding
Sandbox verification was unavailable.Verify that
libgtest-devprovides every required test target.The workflow now installs only
libgtest-devon Linux. Iftests/ut/cpplinksGTest::gmock,gmock_main, or another component not supplied by this package, CMake configuration or linking will fail.
64-67: 🎯 Functional Correctness
⚠️ Unverified finding
Sandbox verification was unavailable.Verify that
build_package_simbuilds both simulator artifacts.The comment states that worker tests require
a2a3simanda5sim, but the command names only one build target. Confirm thatbuild_package_simexpands to both artifacts before relying on the later test jobs..github/workflows/_ut-npu-a2a3.yml (3)
49-54: 🩺 Stability & Availability
⚠️ Unverified finding
Sandbox verification was unavailable.Confirm that
test_comm_lifecycleis the aggregate hardware-test target.Line 54 builds only
test_comm_lifecycle, but Line 69 runs every CTest case with therequires_hardwareorrequires_hardware_a2a3label. Iftest_comm_lifecycleis a single executable, CTest will attempt to run other unbuilt binaries after device allocation.
56-61: LGTM!
72-72: 🩺 Stability & Availability
⚠️ Unverified finding
Sandbox verification was unavailable.Verify the Python interpreter used inside
task-submit.The outer shell activates
.venv, but Line 72 invokespythoninside a task-managed command string. Iftask-submitdoes not preservePATHand the virtual environment, pytest can run with system Python and miss the installed project dependencies. Use.venv/bin/pythonor activate the environment inside the submitted command if inheritance is not guaranteed.
🤖 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/build_runtimes.py`:
- Around line 171-172: Update the process-global helper build flow around
builder.get_binaries so helpers consumed by mixed onboard and simulation
runtimes are built with one sanitizer-compatible toolchain, independent of
platforms[0] ordering; alternatively key helper outputs by toolchain. Preserve
compatible compiler and sanitizer-runtime selection for every consumer, and add
tests covering both mixed platform orders.
🪄 Autofix
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: cfa5ea73-6069-4ef4-aa7b-16a57c3a7c47
📒 Files selected for processing (13)
.github/workflows/_packaging.yml.github/workflows/_profiling-flags-smoke.yml.github/workflows/_ut-no-hardware.yml.github/workflows/_ut-npu-a2a3.ymldocs/getting-started.mddocs/python-packaging.mdsimpler_setup/build_runtimes.pysimpler_setup/runtime_builder.pysimpler_setup/runtime_compiler.pytests/ut/py/test_build_runtimes.pytests/ut/py/test_runtime_builder.pytests/ut/py/test_runtime_compiler.pytools/verify_packaging.sh
545f435 to
058c06f
Compare
doraemonmj
changed the title
52a72eb to
af0fd17
Compare
- Reuse process-global runtime helpers and limit no-hardware UT builds to the simulator platforms they exercise - Generate per-target profiling headers so all six flag combinations and both simulator architectures reuse unaffected objects - Use an empty job-local ccache across packaging modes, normalize macOS compiler inputs, and fall back when ccache is unavailable - Keep sanitizer builds on one host ABI and isolate sanitizer state between tests - Build A3 C++ UT before NPU allocation, then run Python and C++ hardware UT under one allocation - Validate packaging installs outside the source tree and keep every mode fully clean - Upload POD diagnostics best-effort so artifact failures cannot replace the test result
2 participants
Summary
Scope and safety
Observed CI result
Run 31666256787, before the macOS target-normalization experiment:
Validation