chore(deps): update Mooncake to v0.3.9 - #1448
Conversation
|
👋 Hi staryxchen! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe Mooncake version was advanced to v0.3.9. The build script now reads Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitlab/build.sh:
- Around line 285-286: The build currently sets MOONCAKE_VERSION by querying
remote tags (MOONCAKE_VERSION=$(git ls-remote ...)) which makes builds
non-deterministic; change this to pin the version from ATTRIBUTIONS-CPP.md (set
MOONCAKE_VERSION to the fixed tag v0.3.9), replacing the dynamic git ls-remote
pipeline with a hardcoded assignment and keep the echo "MOONCAKE_VERSION:
${MOONCAKE_VERSION}" line to preserve logging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5347de3c-c648-4cf0-9f5a-8eda1b21499d
📒 Files selected for processing (2)
.gitlab/build.shATTRIBUTIONS-CPP.md
|
Do you know, by any chance, how to tune the number of jobs in mooncake build in a way that does not OOM? Right now mooncake is the only dependency we build with -j4 due to OOM issues when linking. While for everything else we use NPROC. It's being built by CI on a lot of different machines and -j4 is quite slow. The number of CPUs is high on many of these nodes but memory is limited. |
|
Hi @ovidiusm I'll work on optimizing the build parallelism (e.g., using separate job pools for compile vs. link to avoid OOM during linking while keeping compilation fast). My plan is to first push the relevant changes upstream in the Mooncake community, and then contribute the build optimization back here to nixl once that's landed. |
Auto-detect available memory and CPU cores at configure time to calculate safe parallel job limits. With Ninja generator, creates separate job pools for compilation (~1.5GB/job) and linking (~4GB/job) so high-core machines can compile fast without OOM during linking. - New mooncake-common/limit_jobs.cmake module - Include from common.cmake for all build modes - Switch Dockerfile to Ninja for automatic job pool enforcement - User can override via -DPARALLEL_COMPILE_JOBS / -DPARALLEL_LINK_JOBS Addresses feedback from ai-dynamo/nixl#1448 where Mooncake was the only dependency forced to build with -j4. Signed-off-by: staryxchen <staryxchen@tencent.com>
|
/build |
|
/ok to test 32a119a |
|
Please update branch and then update CI_IMAGE_TAG in:
|
- Dynamically resolve Mooncake version from git tags instead of static clone - Bump Mooncake version in ATTRIBUTIONS-CPP.md from v0.3.2.post1 to v0.3.9 Signed-off-by: staryxchen <staryxchen@tencent.com>
Use a fixed default version (v0.3.9) instead of dynamically resolving the latest tag at build time. This ensures build reproducibility and keeps the version in sync with ATTRIBUTIONS-CPP.md. The version can still be overridden via the MOONCAKE_VERSION environment variable. Signed-off-by: staryxchen <staryxchen@tencent.com>
32a119a to
a836fb9
Compare
|
Hi @ovidiusm, I wasn't sure exactly what "update branch" meant in this context, so I went ahead and rebased onto the latest main. Please let me know if there's anything else needed! |
|
Yes, thanks, please also set CI_IMAGE_TAG to 20260323-1 in the 2 yaml files |
Signed-off-by: staryxchen <staryxchen@tencent.com>
|
Done! Updated |
|
/build |
|
/ok to test 3706e16 |
|
Hey @ovidiusm 👋, following up on your note in this PR about the I just landed kvcache-ai/Mooncake#1718 upstream, which adds I've drafted a small patch for nixl (branch
On a 16-core / 26 GB machine this gives compile=16, link=6 instead of the previous make -j4 for everything. Question for you: would you prefer to:
Happy to open a PR either way — just wanted to check your preference first! |
What
git cloneATTRIBUTIONS-CPP.mdfromv0.3.2.post1tov0.3.9Why
The previous build script always cloned the default branch of Mooncake, which could lead to version drift and non-reproducible builds. By resolving the version from git tags, we ensure a specific release version is used.
How
.gitlab/build.shto usegit ls-remote --tagsto find the latest Mooncake version tag and clone that specific versionThis is a clean re-implementation of #1313 on the latest main branch (the original PR had accumulated conflicts).
Summary by CodeRabbit
Chores
Documentation