Skip to content

[Fix] Break JIT cache-key dependency-traversal recursion cycles#803

Merged
coderfeli merged 1 commit into
mainfrom
pr/fix-jit-cache-key-recursion
Jul 7, 2026
Merged

[Fix] Break JIT cache-key dependency-traversal recursion cycles#803
coderfeli merged 1 commit into
mainfrom
pr/fix-jit-cache-key-recursion

Conversation

@sjfeng1999

Copy link
Copy Markdown
Collaborator

_collect_dependency_sources used a visited set to guard against cycles, but the JitFunction branch went through
val._ensure_cache_manager() -> _jit_function_cache_key -> _collect_dependency_sources(<fresh visited>), bypassing the caller's visited. During that computation manager_key is still None, so there was no reentrancy guard: any jit self-reference, jit<->jit mutual reference, or a .launch attribute name colliding with a same-named top-level @flyc.jit would recurse until RecursionError.

Add a per-instance _computing_manager_key flag, set while a JitFunction computes its own key. When the dependency traversal meets a JitFunction already computing its key (a cycle), emit a stable jit-recursive:<name>:<label> placeholder instead of reentering. Each cycle member's own source still enters the key via its own _get_func_source, so the placeholder only breaks the cycle.

Acyclic code never hits the placeholder branch, so existing cache keys (and on-disk cache) are unchanged.

Add regression tests for top-level launch attribute collision, jit self-reference, and jit<->jit mutual reference.

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a recursion bug in JIT cache-key dependency traversal: _collect_dependency_sources() could re-enter cache-key computation via JitFunction._ensure_cache_manager() using a fresh visited set, causing unbounded recursion (e.g., self-reference, mutual jit references, or .launch name collisions with a top-level @flyc.jit named launch).

Changes:

  • Add a reentrancy guard (_computing_manager_key) to JitFunction and emit a stable jit-recursive:<name>:<label> placeholder when a cycle is detected during dependency collection.
  • Wrap manager-key computation in try/finally to ensure the guard is cleared even on errors.
  • Add regression tests covering .launch attribute collision, jit self-reference, and mutual jit references; improve _load_mod() to dedent module bodies for readability.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
python/flydsl/compiler/jit_function.py Adds cycle-breaking logic for dependency-source collection and a per-instance “computing manager key” guard.
tests/unit/test_jit_cache_key_completeness.py Adds regression tests for recursion-cycle scenarios and updates module loader to dedent test module sources.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread python/flydsl/compiler/jit_function.py Outdated
@sjfeng1999
sjfeng1999 force-pushed the pr/fix-jit-cache-key-recursion branch from f26b7d4 to 9e7558a Compare July 6, 2026 04:59
`_collect_dependency_sources` used a `visited` set to guard against
cycles, but the JitFunction branch went through
`val._ensure_cache_manager()` -> `_jit_function_cache_key` ->
`_collect_dependency_sources(<fresh visited>)`, bypassing the caller's
`visited`. During that computation `manager_key` is still `None`, so
there was no reentrancy guard: any jit self-reference, jit<->jit mutual
reference, or a `.launch` attribute name colliding with a same-named
top-level `@flyc.jit` would recurse until `RecursionError`.

Guard reentrancy with a thread-local set of the `id(func)` whose keys are
currently being computed on this thread: `_jit_function_cache_key` adds
its func on entry and discards it in `finally`, and the dependency
traversal, when it meets a JitFunction already being keyed (a cycle),
emits a stable `jit-recursive:<name>:<label>` placeholder instead of
reentering. Each cycle member's own source still enters the key via its
own `_get_func_source`, so the placeholder only breaks the cycle.

The stack is thread-local (not a per-instance flag) so that a JitFunction
being keyed on one thread is not mistaken for a cycle by a concurrent
compilation on another thread — that would otherwise substitute the
placeholder for a real dependency and make keys timing-dependent.

Acyclic code never hits the placeholder branch, so existing cache keys
(and on-disk cache) are unchanged.

Add regression tests for top-level `launch` attribute collision, jit
self-reference, jit<->jit mutual reference, thread-local isolation of the
guard, and no-leak cleanup of the in-progress stack.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sjfeng1999
sjfeng1999 force-pushed the pr/fix-jit-cache-key-recursion branch from 9e7558a to 7880701 Compare July 6, 2026 05:01
@coderfeli
coderfeli merged commit 04265dd into main Jul 7, 2026
24 of 27 checks passed
@coderfeli
coderfeli deleted the pr/fix-jit-cache-key-recursion branch July 7, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants