Skip to content

Don't call onPropsChanged on a child's initial render with COMPOSE_RUNTIME_SKIPPING [CLF-658] - #1592

Merged
zach-klippenstein merged 2 commits into
mainfrom
zachklipp/skipping-initial-onpropschanged
Oct 29, 2026
Merged

Don't call onPropsChanged on a child's initial render with COMPOSE_RUNTIME_SKIPPING [CLF-658]#1592
zach-klippenstein merged 2 commits into
mainfrom
zachklipp/skipping-initial-onpropschanged

Conversation

@zach-klippenstein

@zach-klippenstein zach-klippenstein commented Oct 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

With RuntimeConfigOptions.COMPOSE_RUNTIME_SKIPPING, a child StatefulWorkflow's onPropsChanged is called on its very first render with old == new. The legacy runtime and COMPOSE_RUNTIME_NON_SKIPPING never do this.

renderWorkflowRestartableImpl computes its dirty bits with composer.changed(props), which always returns true on first composition. That reaches WorkflowSnapshotState.updateAndGetState as didPropsChange = true, even though ComposeRenderContext was just created with those same props.

Found while writing tests for CLF-654 (#1591).

  • First commit: a failing test in RenderWorkflowInTest. It fails only on COMPOSE_RUNTIME_SKIPPING.
  • Second commit: the fix. On first composition (composer.inserting), report props as unchanged and onOutput as changed. WorkflowSnapshotState starts with onOutput = null, and the spurious props-changed write was the thing that stored the real handler. Simply falling back to equals broke output delivery for the root workflow and added an extra props equals call.

🤖 Generated with Claude Code

zach-klippenstein and others added 2 commits October 28, 2026 15:56
@zach-klippenstein @claude
Add failing test for onPropsChanged on child's initial render
bb35db5 
With COMPOSE_RUNTIME_SKIPPING, a child's onPropsChanged is called on its
first render with old == new. The legacy runtime never does this.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zach-klippenstein @claude
Don't call onPropsChanged on initial render with COMPOSE_RUNTIME_SKIP…
163d783 
…PING

composer.changed() always reports a change on first composition, so the
restartable render path told WorkflowSnapshotState the props had changed
even though ComposeRenderContext was just created with them. On insert,
report props as unchanged and onOutput as changed instead: the snapshot
state starts without an output handler, and the spurious props-changed
write was what stored it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zach-klippenstein
zach-klippenstein marked this pull request as ready for review October 28, 2026 23:08
@zach-klippenstein
zach-klippenstein requested a review from a team as a code owner October 28, 2026 23:08
@zach-klippenstein

zach-klippenstein commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Compose runtime microbenchmarks: this PR (163d783a8) vs. main at the branch point (0cf056963)

:benchmarks:runtime-microbenchmark:connectedReleaseAndroidTest, limited to the four compose classes: {ShallowBushyTree, SquareishTree} × {ComposeNoSkip, ComposeSkipping}. Both runs used a Galaxy A25 (SM-A256U1, Android 16) on AC power at thermal status 0, back to back, baseline first. Numbers are medians.

Result: no measurable change.

  • Allocations are identical in all 56 benchmarks (within ±1). That's expected: the benchmark leaves use the default onPropsChanged, so the extra call this PR removes cost no allocations.
  • Time deltas are noise. ComposeNoSkip is a control group, since this PR only changes the skipping path, and it moved just as much as the skipping configs.
Variant Mean Δ time Range
ShallowBushyTree_ComposeNoSkip (control) +1.5% −6.8% … +9.1%
ShallowBushyTree_ComposeSkipping +3.6% −6.4% … +10.4%
SquareishTree_ComposeNoSkip (control) +0.8% −4.7% … +9.0%
SquareishTree_ComposeSkipping −1.6% −6.4% … +6.5%
Full results

ShallowBushyTree_ComposeNoSkip

Benchmark Time (main) Time (fix) Δ time Allocs (main) Allocs (fix) Δ allocs
initialRenderAllChildren 2.38 ms 2.40 ms +1.0% 10,863 10,863 -0.0%
initialRenderNewSibling 728.4 µs 726.8 µs -0.2% 2,582 2,582 -0.0%
rememberManyEntries_changingInputs 491.9 µs 499.0 µs +1.5% 2,865 2,865 -0.0%
rememberManyEntries_mixedInputTypes 548.8 µs 555.1 µs +1.1% 3,241 3,241 -0.0%
rememberManyEntries_sameInputs 462.2 µs 441.2 µs -4.5% 2,733 2,733 -0.0%
rerenderAllChildrenByPropsChange 637.0 µs 593.6 µs -6.8% 1,931 1,931 +0.0%
rerenderAllChildrenViaStateChange 545.2 µs 569.3 µs +4.4% 1,828 1,828 +0.0%
rerenderSingleSiblingByPropsChange 504.4 µs 529.9 µs +5.1% 1,914 1,914 -0.0%
rerenderSingleSiblingViaStateChange 493.2 µs 515.2 µs +4.5% 1,820 1,820 +0.0%
stableHandlers_manyCallbacks_propChange 421.7 µs 439.3 µs +4.2% 2,733 2,733 -0.0%
tearDownAllChildren 1.58 ms 1.73 ms +9.1% 5,036 5,036 -0.0%
tearDownSingleSibling 1.31 ms 1.32 ms +0.3% 2,674 2,674 +0.0%
wideSiblingKeys_initialRenderAllChildren 3.23 ms 3.33 ms +3.0% 15,854 15,855 +0.0%
wideSiblingKeys_rerenderSingleSiblingByPropsChange 600.6 µs 590.3 µs -1.7% 1,533 1,533 +0.0%

ShallowBushyTree_ComposeSkipping

Benchmark Time (main) Time (fix) Δ time Allocs (main) Allocs (fix) Δ allocs
initialRenderAllChildren 2.57 ms 2.68 ms +4.6% 11,619 11,619 +0.0%
initialRenderNewSibling 558.6 µs 523.1 µs -6.4% 1,190 1,190 -0.0%
rememberManyEntries_changingInputs 523.9 µs 573.6 µs +9.5% 2,867 2,867 -0.0%
rememberManyEntries_mixedInputTypes 568.0 µs 603.5 µs +6.2% 3,243 3,243 +0.0%
rememberManyEntries_sameInputs 467.1 µs 485.2 µs +3.9% 2,743 2,743 +0.0%
rerenderAllChildrenByPropsChange 683.9 µs 734.1 µs +7.4% 2,085 2,085 -0.0%
rerenderAllChildrenViaStateChange 693.6 µs 699.3 µs +0.8% 2,063 2,063 +0.0%
rerenderSingleSiblingByPropsChange 293.3 µs 303.9 µs +3.6% 514 514 +0.0%
rerenderSingleSiblingViaStateChange 278.9 µs 276.0 µs -1.0% 423 423 +0.0%
stableHandlers_manyCallbacks_propChange 452.2 µs 474.8 µs +5.0% 2,743 2,743 +0.0%
tearDownAllChildren 1.83 ms 2.02 ms +10.4% 5,041 5,041 -0.0%
tearDownSingleSibling 1.23 ms 1.28 ms +4.1% 1,275 1,275 +0.0%
wideSiblingKeys_initialRenderAllChildren 4.84 ms 4.99 ms +3.1% 18,359 18,359 +0.0%
wideSiblingKeys_rerenderSingleSiblingByPropsChange 699.7 µs 694.3 µs -0.8% 1,296 1,296 +0.0%

SquareishTree_ComposeNoSkip

Benchmark Time (main) Time (fix) Δ time Allocs (main) Allocs (fix) Δ allocs
initialRenderAllChildren 11.47 ms 11.55 ms +0.7% 40,446 40,445 -0.0%
initialRenderNewSibling 2.61 ms 2.64 ms +1.1% 8,586 8,587 +0.0%
rememberManyEntries_changingInputs 531.8 µs 506.7 µs -4.7% 2,865 2,865 +0.0%
rememberManyEntries_mixedInputTypes 584.1 µs 600.7 µs +2.8% 3,241 3,241 -0.0%
rememberManyEntries_sameInputs 458.8 µs 448.3 µs -2.3% 2,733 2,733 -0.0%
rerenderAllChildrenByPropsChange 2.69 ms 2.67 ms -0.9% 8,453 8,453 -0.0%
rerenderAllChildrenViaStateChange 2.36 ms 2.42 ms +2.9% 8,344 8,344 +0.0%
rerenderSingleSiblingByPropsChange 2.48 ms 2.49 ms +0.6% 8,446 8,446 -0.0%
rerenderSingleSiblingViaStateChange 2.18 ms 2.20 ms +1.0% 8,331 8,331 +0.0%
stableHandlers_manyCallbacks_propChange 447.1 µs 440.3 µs -1.5% 2,733 2,733 -0.0%
tearDownAllChildren 7.34 ms 7.38 ms +0.5% 25,350 25,351 +0.0%
tearDownSingleSibling 2.87 ms 2.86 ms -0.3% 8,535 8,535 -0.0%
wideSiblingKeys_initialRenderAllChildren 3.37 ms 3.44 ms +2.2% 15,855 15,855 +0.0%
wideSiblingKeys_rerenderSingleSiblingByPropsChange 627.8 µs 684.0 µs +9.0% 1,533 1,533 -0.0%

SquareishTree_ComposeSkipping

Benchmark Time (main) Time (fix) Δ time Allocs (main) Allocs (fix) Δ allocs
initialRenderAllChildren 12.94 ms 12.21 ms -5.6% 43,122 43,122 -0.0%
initialRenderNewSibling 2.08 ms 2.05 ms -1.6% 4,243 4,243 -0.0%
rememberManyEntries_changingInputs 547.7 µs 531.3 µs -3.0% 2,867 2,867 +0.0%
rememberManyEntries_mixedInputTypes 614.9 µs 575.4 µs -6.4% 3,243 3,243 -0.0%
rememberManyEntries_sameInputs 493.3 µs 480.4 µs -2.6% 2,743 2,743 -0.0%
rerenderAllChildrenByPropsChange 3.11 ms 3.31 ms +6.5% 9,185 9,184 -0.0%
rerenderAllChildrenViaStateChange 3.12 ms 3.01 ms -3.7% 9,444 9,444 +0.0%
rerenderSingleSiblingByPropsChange 1.82 ms 1.72 ms -5.8% 4,095 4,095 -0.0%
rerenderSingleSiblingViaStateChange 265.2 µs 261.5 µs -1.4% 336 336 +0.0%
stableHandlers_manyCallbacks_propChange 455.5 µs 459.6 µs +0.9% 2,743 2,743 -0.0%
tearDownAllChildren 7.81 ms 7.95 ms +1.7% 26,325 26,326 +0.0%
tearDownSingleSibling 2.20 ms 2.19 ms -0.5% 4,185 4,185 -0.0%
wideSiblingKeys_initialRenderAllChildren 5.07 ms 4.92 ms -3.0% 18,358 18,358 -0.0%
wideSiblingKeys_rerenderSingleSiblingByPropsChange 694.1 µs 709.7 µs +2.2% 1,296 1,296 -0.0%

@zach-klippenstein
zach-klippenstein merged commit f9f0c4e into main Oct 29, 2026
77 checks passed
@zach-klippenstein
zach-klippenstein deleted the zachklipp/skipping-initial-onpropschanged branch October 29, 2026 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tcmulcahy tcmulcahy tcmulcahy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zach-klippenstein @tcmulcahy