Skip to content

Replace the autolabel debounce with a burst-served label cache and idle refresh - #6

Open
charlesangus wants to merge 4 commits into
masterfrom
label-cache
Open

charlesangus wants to merge 4 commits into
masterfrom
label-cache

Conversation

@charlesangus

Copy link
Copy Markdown
Owner

Summary

Replaces the 100 ms autolabel debounce (94e313b) with a label cache designed around how Nuke actually drives the autolabel, measured on 3k–10k node scripts with real X input.

What the profiling showed (Nuke 17.0v3; details on the profiling-harness branch, .profiling/RESULTS-2026-09-13.md):

  • Nuke never re-requests labels on redraw. It asks on a real knob change (one or two requests) or in a whole-script pass — every node, back to back — after a viewer input change or any knob change followed by a frame step.
  • The cost that matters is not building a label but handing Nuke a changed string: each one triggers a main-loop stall proportional to script size (~70 ms at 3k nodes, ~200 ms at 10k). Stock labels dodge it only because their text has no knob values. With the debounce, a slider drag cost 120–260 ms per pointer event on those scripts (stock: 15–25 ms), and once the stall exceeded 100 ms every move rebuilt anyway. The debounce also dropped trailing edits (two edits 50 ms apart left the first value on screen).
  • knobChanged fires per node per pointer move when dragging a selection (123k times for one drag of 3k nodes), so it is not used.

The new scheme (AutolabelReplacement.create_autolabel and helpers):

  • Bursts of back-to-back requests are answered from a per-node cache, so whole-script passes cost microseconds per node.
  • A changed label string is never returned while the user interacts: the previous text is shown and the node marked stale.
  • Once label traffic has been quiet for ≥ 0.4 s (5× the measured stall, capped at 1.5 s), stale nodes get a dope_sheet change-and-revert under Undo.disable() — exactly one relabel, no undo entry, no visible change — which releases the new text.
  • Frame-dependent nodes (keys, expressions, TCL in the label) are cached per frame and refreshed the same way after a scrub.
  • Cache keyed by fullName(); onDestroy (once per deleted node) stops a reused name inheriting the old label.
  • Config and preference saves invalidate the cache and refresh every label, so changes show immediately.

Measured, same Nuke session, implementations interleaved (ms per pointer event, 10 002 nodes): slider drag 13–20 vs stock 13–23 (was 154–266); timeline scrub 22–32 vs 139–186; whole-script pass 171 ms vs 700 ms; node drags unchanged.

Notes for review

  • The dope_sheet poke is a workaround for the lack of any API to re-request one node's label; verified on 17.0v3 only. If it ever stops triggering a relabel the fallback is a stale label until Nuke's next natural request.
  • Labels now update ~0.4 s after you stop editing rather than immediately; README's Performance section documents this and the PDF is rebuilt.
  • The temporary profiling instrumentation is removed; the harness and raw results live on profiling-harness.

Testing

  • ruff check ., pytest tests/ (77 passed; 15 new in tests/test_label_cache.py)
  • Real Nuke under Xvfb: 3 002 and 10 002 node scripts, stock vs this branch interleaved per gesture, no tracebacks, final values correct after every drag.

…le refresh

Profiling with real X input against 3k-10k node scripts showed the cost
that matters is not building a label but handing Nuke a *changed* label
string: every change costs a main-loop stall proportional to the script
size (~70 ms at 3k nodes, ~200 ms at 10k). The 100 ms debounce made
slider drags 120-260 ms per pointer event on those scripts (stock:
15-25 ms) and, once the stall exceeded its window, rebuilt on every
move anyway. It also dropped trailing edits (two edits 50 ms apart left
the first value on screen).

Nuke asks for a label on a real knob change (one or two requests) or in
a whole-script pass (every node back to back, after a viewer input
change or any knob change followed by a frame step). The new scheme:

- bursts of requests are answered from a per-node cache, so whole-script
  passes cost microseconds per node instead of a build
- a changed label string is never returned while the user interacts;
  the previous text is shown and the node is marked stale
- once label traffic has been quiet for >= 0.4 s (5x the measured
  stall), stale nodes get a dope_sheet change-and-revert (one relabel,
  no undo entry, no visible change), which releases the new text
- frame-dependent nodes (keys, expressions, TCL labels) are cached per
  frame and refreshed the same way after a scrub
- no knobChanged hook: dragging a 3k-node selection fires it 123k times;
  onDestroy (once per deleted node) keeps a reused name from inheriting
  the old label

Measured in one Nuke 17 session with the implementations interleaved
(ms per pointer event, 10 002 nodes): slider drag 13-20 vs stock 13-23
(was 154-266); timeline scrub 22-32 vs 139-186; whole-script pass
171 ms vs 700 ms; node drags unchanged.

Config and preference saves now invalidate the cache and refresh every
label, so changes show immediately. The temporary profiling
instrumentation is removed; the harness lives in .profiling (ignored).

@charlesangus charlesangus left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reviewed by Codex (codex: available — 5h 19%, 7d 38%, cap 90%, plan plus) via cat-pm. Found one high-, three medium-, and four low-severity issues.

Comment thread labelmaker.py Outdated
node_label_value = nuke.value("this.label", "")
with contextlib.suppress(RuntimeError):
node_label_value = nuke.tcl("subst", node_label_value)
self.node_label_value = node_label_value or ""

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

high — The value saved for frame-dependency detection is the result after nuke.tcl("subst", ...). Substitution normally removes the [ syntax, so label knobs such as [frame] are cached as frame-independent. During bursts, affected nodes can then keep returning an old frame's label without ever being marked stale.

Suggested fix: Preserve the raw label-knob string before substitution and make _frame_dependent() test that raw value. Add a test using a raw [frame] label.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 1527d28: label_readout_creator now keeps the raw knob value (node_label_raw) before nuke.tcl("subst", ...) and _frame_dependent tests that. Test test_tcl_in_the_label_knob_is_cached_per_frame runs the real build with a [frame] label and asserts the entry is cached with a frame.

Comment thread labelmaker.py
if node is None:
continue
knob = node.knob("dope_sheet")
if knob is None:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

medium — A stale node without dope_sheet is silently skipped after _refresh_stale_labels() has already removed it from _stale. Its old _shown value therefore remains, and subsequent changed builds repeat the same hold/skip cycle, so the displayed label can remain permanently stale.

Suggested fix: Do not defer changed strings for nodes that cannot be poked, and retain skipped burst entries until a natural request can rebuild and show them. Alternatively, implement a verified safe fallback poke knob.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 1527d28, slightly more broadly than suggested: a node without dope_sheet is never served from the cache and never has a changed string held back (_pokeable), so it is rebuilt and shown on every request. Viewers/backdrops are a handful per script, so the cost is negligible and there is no path that can leave them stale.

Comment thread labelmaker.py Outdated
now = time.perf_counter()
self._note_stall(now)
in_burst = self._track_burst(now)
full_name = nuke.thisNode().fullName()

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

medium — The cache is keyed only by the mutable fullName(), while cleanup occurs only on destruction. Renaming a node or enclosing group leaves the old keys behind. If that name is reused—or a group is renamed back—a large burst can serve the obsolete entry and, because bursts over 200 are not marked stale, leave the wrong label indefinitely.

Suggested fix: Track each node's previous full name by stable node identity. On every callback, migrate or evict entries when its full name changes; clear that identity mapping on destruction and invalidation.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 1527d28 without identity tracking: an onCreate callback (fires once per node, unlike knobChanged) evicts every cache entry under the created node's fullName(), so a name freed by a rename or delete cannot hand a new node an obsolete label. A renamed node itself is relabelled by Nuke on rename and rebuilds under its new name.

Comment thread labelmaker_prefs_dialog.py Outdated
labelmaker_config.reload_composed_config()
labelmaker.autolabeller_singleton.config = labelmaker_config.composed_config_singleton
labelmaker.autolabeller_singleton.set_enabled(self.labelmaker_enabled_checkbox.isChecked())
if self.labelmaker_enabled_checkbox.isChecked():

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

medium — Disabling Labelmaker unregisters the callback but deliberately skips any relabel poke. Since redraws do not cause label requests, nodes can continue displaying the last Labelmaker strings indefinitely instead of immediately reverting to Nuke's default autolabel.

Suggested fix: On the enabled-to-disabled transition, unregister first and poke all nodes without adding _forced entries, so Nuke rebuilds them through its default autolabel.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 1527d28: set_enabled(False) unregisters and then pokes every node without forcing, so Nuke rebuilds them through its stock autolabel; set_enabled(True) registers and refreshes every label. The dialog's extra refresh_all_labels() call is gone.

Comment thread labelmaker.py Outdated
full_name = nuke.thisNode().fullName()
cached = self._content.get(full_name)
if in_burst and cached is not None and full_name not in self._forced:
# a whole-script pass: nothing about this node changed

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

low — The comment is false for bursts of 9–200 requests: this branch also handles genuine multi-node edits where the node may have changed. It contradicts the later burst-close logic and the stated design.

Suggested fix: Explain that both candidate whole-script passes and multi-node edits are initially served from cache, with classification deferred until the burst closes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reworded in 1527d28: the branch serves from the cache for both whole-script passes and genuine multi-node edits, which are only told apart in _close_burst.

Comment thread tests/test_label_cache.py Outdated
names = ["Grade{}".format(i) for i in range(20)]
for name in names:
request(labeller, clock, name, "old")
whole_script_pass(labeller, clock, names) # 20 nodes changed by a script

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

low — The comment says the 20 nodes changed, but their configured build text remains "old". Consequently test_small_burst_is_a_real_edit_and_gets_refreshed checks only stale-set classification and never verifies that changed content is ultimately rebuilt and shown.

Suggested fix: Change every node's pending text before the burst, fire the refresh, simulate the resulting callbacks, and assert that each new value is returned.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 1527d28: every node's text changes before the burst, the pass is asserted to still show the old text, the burst closes, the refresh fires, and the simulated relabel requests are asserted to return the new text.

Comment thread tests/test_label_cache.py Outdated
knob = labeller.nodes["Grade1"]["dope_sheet"]
clock.now += 1.0
labeller._refresh_timer.fire()
assert knob.value() is False # flipped and flipped back

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

low — Several added comments merely narrate adjacent setup or assertions (for example lines 123, 135, 139, 145, 160, 180, 197, and 213), contrary to the requested comment policy.

Suggested fix: Remove the narrating comments. Where context is genuinely needed, explain a hidden constraint—for example, that the stub cannot emit Nuke's automatic relabel callback and the test must simulate it.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Removed in 1527d28. One comment kept, rewritten to state the non-obvious constraint: the stub cannot emit Nuke's own relabel request, so the tests simulate it with a lone request.

Comment thread .gitignore Outdated
# Intermediate Markdown generated from the README for the User Guide PDF
docs/.build/

# Local profiling harness (see .profiling/README.md)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

low — The new comment directs readers to .profiling/README.md, but that file is absent and the entire directory is ignored. The related source comment also refers to unavailable .profiling results.

Suggested fix: Remove the dangling references or commit the profiling methodology/results in a tracked documentation location.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 1527d28: both references now point at the profiling-harness branch, which holds the harness, raw results and write-up.

…e reuse, disable path

- _frame_dependent tested the label knob after TCL substitution, so a
  [frame] label was cached as frame-independent; use the raw knob value
- nodes without a dope_sheet knob (Viewer, Backdrop) cannot be poked, so a
  held-back string would never be released: rebuild and show them on every
  request instead of caching or holding
- onCreate evicts cache entries under a created node's name, so a name
  freed by a rename or delete cannot hand a new node the old label
- set_enabled(False) pokes every node so Nuke rebuilds them with its own
  autolabel instead of leaving Labelmaker's strings on screen;
  set_enabled(True) refreshes every label
- stronger burst test, narrating test comments removed, dangling
  .profiling references point at the profiling-harness branch
@charlesangus

Copy link
Copy Markdown
Owner Author

Review round closed: all 8 Codex findings addressed in 1527d28 (replies on each thread). Re-verified in Nuke 17.0v3 on the 3 002-node script with stock and this branch interleaved per gesture: slider drag 3.4–4.6 ms/tick vs stock 2.6–4.3, whole-script pass 79 ms vs 204 ms, scrub 4.3–4.9 vs 35–46 ms/tick, node drags equal, no tracebacks, final values correct after every drag. ruff clean, pytest 84 passed.

… bursts

A burst of label requests served from the cache was classified by size
(> 200 = whole-script pass, left as served); a tool setting a knob on 1000
selected nodes produces the same burst, so 784 of them kept the old label
indefinitely. Any classifier has that failure mode (a frame/viewer-change
heuristic was tried and dropped: a bulk edit that also steps the frame is
misclassified the same way).

Now every label answered from the cache during a burst is re-composed in
the background once label traffic is quiet: nuke.runIn() gives the label
code its node context, the compose is read-only (the indicators knob write
from Foundry's set_indicators only happens on a real request), 15 ms slices
on a 0 ms timer back off while traffic resumes, and only labels whose text
differs are poked. Labels that were frame-dependent at their last build
are verified first and released as soon as that phase ends, since a frame
step is what changes them. LABEL_BURST_GENUINE_MAX and the per-frame cache
entries are gone.

Measured with the harness on profiling-harness (5400f88), §11 of
RESULTS-2026-09-13.md: correct on bulk edits of 100/1000/all nodes, with a
frame change or viewer connect in the same callback, with deletes, renames,
undo, set_enabled(False) or scriptClear before the refresh; slider drags
stay at stock latency (16–20 ms/tick at 10k). Cost: ~0.4 s (3k) / 1.6 s
(10k) of background compute after a whole-script pass on the profiling box,
and frame-dependent readouts refresh ~1–2.5 s after a frame step instead of
inside stock's UI freeze.
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.

1 participant