Comfy-Org / Comfy-Org/ComfyUI_frontend

changeTracker: memoize the previous execution-graph projection instead of recomputing it

Open
#14,468 1 comment 1 reaction 1 assignee Claimed by @jaeone94 View on GitHub
perf:speed
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

Follow-up to #14041 (merged). Minor performance item raised in review, deliberately deferred.

## Problem / Goal

In `updateModified`, both execution-graph projections are rebuilt on every captured change:

```ts
const executionGraphChanged =
!!previousState &&
isAutoQueueOnChange() &&
!_.isEqual(
getExecutionGraphState(previousState),
getExecutionGraphState(this.activeState)
)
```

`getExecutionGraphState(previousState)` is, by construction, the same value computed as the *current* projection on the immediately preceding call. It is discarded and recomputed.

Each projection walks the whole graph: an `Object.fromEntries` rebuild of the graph, one per node plus one per input and output slot, an `_.sortBy` over nodes, a `JSON.stringify` per link for the sort key, and full recursion into every subgraph definition. So the redundant half is roughly one full graph traversal per edit.

Measured earlier against this repo's own fixtures, the pair costs about 1.1ms at 245 nodes, 4.8ms at 1000, and 14ms at 3000. Halving it saves roughly 26%.

Importance is much reduced now that #14041 landed the `isAutoQueueOnChange()` gate, which removes this work entirely for the default `disabled` mode. This only affects users who actively run Run (on change).

## Proposed Solution

Memoize the last projection on the tracker, keyed on **object identity** so a miss can only ever cost a recompute and never serve a stale result:

```ts
private lastProjection?: { state: ComfyWorkflowJSON; projection: unknown }

private executionStateOf(state: ComfyWorkflowJSON): unknown {
if (this.lastProjection?.state === state) return this.lastProjection.projection
const projection = getExecutionGraphState(state)
this.lastProjection = { state, projection }
return projection
}
```

Identity is a sound key because `activeState` is only ever replaced with a fresh object, never mutated in place. Note the 50ms `squashState` swap installs a new object without going through the comparison, so edits spaced beyond the debounce window will legitimately miss and recompute — the realistic saving is therefore below the 26% ceiling, which applies to rapid bursts (typing, repeated nudges).

## Acceptance Criteria

- [ ] The previous-state projection is reused when the state object is identical
- [ ] A cache miss recomputes rather than serving a stale projection
- [ ] No behavioral change to when `executionGraphChanged` fires (existing projection tests still pass unchanged)
- [ ] Benchmark or test demonstrating the reduced traversal count on a repeat edit

---
Raised in review of #14041 (round 3).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.