fix spurious recompilation on incremental editable builds#669
Open
matthiasdiener wants to merge 1 commit into
Open
fix spurious recompilation on incremental editable builds#669matthiasdiener wants to merge 1 commit into
matthiasdiener wants to merge 1 commit into
Conversation
Micky774
approved these changes
Jul 13, 2026
| # scratch. Rooting it at a stable location lets the underlying ninja skip | ||
| # unchanged objects. Mirrors the persistent CMake build dir above. | ||
| root_dir = Path(__file__).resolve().parent.parent | ||
| build_temp = root_dir / "build" / "torch_temp" |
There was a problem hiding this comment.
Nit: naming — torch_temp reads as PyTorch-specific, but get_build_ext() is also called from transformer_engine/jax/setup.py:60, so JAX ext objects will land under build/torch_temp/ too. Since both frameworks share this dir, a framework-neutral name like ext_temp (or framework_temp) would be less confusing when someone stumbles into it later. No functional impact — ninja tracks objects by source path so pytorch/jax objects won't collide.
|
Reviewed the two-file diff. Both fixes look correct and targeted:
Copyright headers: OK (both files have current AMD end-year 2026; NVIDIA line preserved). Verdict: LGTM, already approved by @Micky774. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Two unrelated causes made
pip install -e .recompile from scratch evenwith no source changes:
PyTorch extension: setuptools' editable_wheel points build_ext.build_temp
at an ephemeral /tmp/*.build-temp dir, so all csrc objects recompiled every
build. Override finalize_options() to pin build_temp to build/torch_temp,
giving ninja a persistent object cache.
CK grouped gemm: ck_grouped_gemm_common.h had a malformed relative include
("../../common/util/system.h", which resolves to a nonexistent path). This
forced hipify's output-dir-dependent fallback resolution, so the pytorch and
common hipify passes emitted different include paths and the generated
_hip.h flip-flopped every build, retriggering all 8 CK objects. Correct the
include to "../../util/system.h" so both passes agree.
Companion to #666 and Micky774/QoLA#9.
With all three PRs, total rebuild time without source code changes is reduced from ~8 minutes to ~6 seconds, with zero recompilations.
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: