Skip to content

Reuse graph node results within one GraphEvaluator evaluation - #274

Merged
timkpaine merged 2 commits into
mainfrom
tkp/graph-node-reuse
Sep 30, 2026
Merged

timkpaine merged 2 commits into
mainfrom
tkp/graph-node-reuse

Conversation

@timkpaine

Copy link
Copy Markdown
Member

GraphEvaluator evaluates each node of the dependency graph once, in topological order. A node's __call__ usually calls its declared dependencies again to use their results, though, and those nested calls go back through the evaluator stack. Without a caching evaluator with cacheable=True, every nested call runs the dependency again. A chain source <- universe <- task where each __call__ calls its dependency runs source three times and universe twice in a single evaluation.

Turning caching on isn't always possible. Long graphs that make many non-graph calls (for example, thousands of per-item HTTP requests inside one node) disable caching to bound memory, which leaves declared dependencies running repeatedly.

This change keeps each graph node's result for the duration of one graph evaluation:

  • a nested call whose effective evaluation key matches a node of the graph being evaluated returns that node's result;
  • calls that are not graph nodes are evaluated as before and never stored, so memory stays bounded by the graph size;
  • results are released in a finally block when the evaluation ends, so nothing is shared across evaluations and cacheable semantics are unchanged.

Tests cover a diamond whose nodes call their dependencies (each node runs once per evaluation with cacheable=False, and again on the next evaluation) and a model that calls a non-dependency twice (runs twice). The cache how-to and the built-in models reference describe the behavior.

When a node's __call__ calls one of its declared dependencies again, return the result already computed for that graph node instead of evaluating it again, independent of cacheable. Results are released when the evaluation finishes, and calls that are not graph nodes are not retained.

Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    1 files  ±0      1 suites  ±0   2m 15s ⏱️ -11s
1 461 tests +3  1 459 ✅ +3  2 💤 ±0  0 ❌ ±0 
1 467 runs  +3  1 465 ✅ +3  2 💤 ±0  0 ❌ ±0 

Results for commit 3a11775. ± Comparison against base commit 249b88d.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.79%. Comparing base (249b88d) to head (3a11775).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #274      +/-   ##
==========================================
+ Coverage   93.78%   93.79%   +0.01%     
==========================================
  Files         190      190              
  Lines       21499    21552      +53     
  Branches     1410     1413       +3     
==========================================
+ Hits        20162    20215      +53     
  Misses       1062     1062              
  Partials      275      275              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread ccflow/evaluators/common.py Outdated
Volatile nodes are still evaluated in the graph pre-pass, but their results are not stored, so each consumer that calls them recomputes.

Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
@ptomecek

ptomecek commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up, not blocking: worth doing the same for RayGraphEvaluator with a run-scoped plasma cache, so non-volatile nodes get reused within a run instead of recomputed. Harder there (cross-task), but the next logical step.

@timkpaine
timkpaine merged commit a1f1ad4 into main Sep 30, 2026
20 checks passed
@timkpaine
timkpaine deleted the tkp/graph-node-reuse branch September 30, 2026 02:24
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.

2 participants