-
Notifications
You must be signed in to change notification settings - Fork 272
build: build native library once and share across CI test jobs #3249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Same optimization as Linux: build libcomet.dylib once and share across all 18 test jobs instead of rebuilding 18 times. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| rust-version: ${{ env.RUST_VERSION }} | ||
| jdk-version: ${{ matrix.profile.java_version }} | ||
|
|
||
| - name: Download native library |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
- Add build-native job that builds libcomet.so once - Add skip-native-build input to setup-spark-builder action - All ~48 test jobs now download pre-built artifact instead of rebuilding Eliminates 47 redundant Rust builds. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The linux-test-rust job caches native/target which contains JNI-related build artifacts. Without Java version in the cache key, Java 11 and 17 jobs could restore incompatible cached artifacts from each other. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3249 +/- ##
============================================
+ Coverage 56.12% 60.00% +3.87%
- Complexity 976 1428 +452
============================================
Files 119 170 +51
Lines 11743 15796 +4053
Branches 2251 2608 +357
============================================
+ Hits 6591 9478 +2887
- Misses 4012 5000 +988
- Partials 1140 1318 +178 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| jdk-version: ${{ matrix.java_version }} | ||
|
|
||
| - name: Cache Cargo | ||
| uses: actions/cache@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use ${{ runner.os }}-cargo-ci- generated by build-native here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linux-test-rust job uses debug builds (cargo test) while build-native uses the CI profile, so their native/target artifacts are incompatible. Sharing the same cache key would cause cache thrashing, where each job overwrites the other's compiled artifacts, if I'm understanding this correctly.
wForget
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @andygrove , this change looks good to me.
The cache storage is approaching its limit: https://github.com/apache/datafusion-comet/actions/caches
Thanks. #3251 will help with this. |

Summary
This PR optimizes CI build times by building the native Rust library once and sharing it across all test jobs, instead of rebuilding it in every job.
Changes
PR Build Linux (
pr_build_linux.yml):build-nativejob that buildslibcomet.soonce and uploads as artifactPR Build macOS (
pr_build_macos.yml):build-nativejob that buildslibcomet.dylibonceSpark SQL Tests (
spark_sql_test.yml):build-nativejob that builds onceskip-native-buildinput tosetup-spark-builderactionShared Changes:
native/Cargo.toml(no LTO, parallel codegen) with same overflow behavior as releaseskip-native-buildinput tojava-testactionEstimated Savings
Additionally, the CI Cargo profile compiles ~30-50% faster than full release (no LTO).