Skip to content

Only allow side-effects within termination.#499

Open
Laimiux wants to merge 1 commit intomasterfrom
laimonas/simplify-terminate
Open

Only allow side-effects within termination.#499
Laimiux wants to merge 1 commit intomasterfrom
laimonas/simplify-terminate

Conversation

@Laimiux
Copy link
Copy Markdown
Collaborator

@Laimiux Laimiux commented Mar 2, 2026

Summary

Updating termination APIs to remove state transitions from it.

Previously:
The API accepted transition with a potential state change

Action.onTerminate().onEvent {
    transition { fireAnalytics() }
}

After:

Action.onTerminate {
    fireAnalytics()
}

@Laimiux Laimiux force-pushed the laimonas/simplify-terminate branch from ea66666 to 281c8e3 Compare March 2, 2026 21:46
@carrotkite
Copy link
Copy Markdown

2 Warnings
⚠️ No coverage data found for com/instacart/formula/Action
⚠️ No coverage data found for com/instacart/formula/ActionBuilder

JaCoCo Code Coverage 93.68% ✅

Class Covered Meta Status
com/instacart/formula/runtime/SnapshotImpl 93% 0%
com/instacart/formula/lifecycle/TerminateLifecycleComponent 50% 0%
com/instacart/formula/Action No coverage data found : -% No coverage data found : -% 🃏
com/instacart/formula/ActionBuilder No coverage data found : -% No coverage data found : -% 🃏
com/instacart/formula/action/ActionComponent 100% 0%
com/instacart/formula/test/TestFormula 100% 0%

Generated by 🚫 Danger

@carrotkite
Copy link
Copy Markdown

carrotkite commented Mar 2, 2026

1 Warning
⚠️ ⚠️ Performance regressions detected in benchmarks

📊 Benchmark Comparison Report

Summary

  • Regressions: 19 ⚠️
  • Improvements: 0
  • Unchanged: 6

⚠️ Performance Regressions

Benchmark Baseline Current Change
com.instacart.formula.benchmarks.TransitionQueueBenchmark.measure1 0.342 ± 0.007 us/op 0.486 ± 0.007 us/op +41.9% 🔴
com.instacart.formula.benchmarks.TransitionQueueBenchmark.measure10000 0.357 ± 0.01 us/op 0.491 ± 0.003 us/op +37.6% 🔴
com.instacart.formula.benchmarks.TransitionQueueBenchmark.measure100 0.357 ± 0.003 us/op 0.489 ± 0.01 us/op +37.0% 🔴
com.instacart.formula.benchmarks.GlobalEffectQueueBenchmark.measure100Effects 8.214 ± 0.131 us/op 10.577 ± 0.116 us/op +28.8% 🔴
com.instacart.formula.benchmarks.ChildrenInitializationBenchmark.initializeNewChildren (childrenCount=1) 0.68 ± 0.055 us/op 0.848 ± 0.09 us/op +24.6% 🔴
com.instacart.formula.benchmarks.ActionInitializationBenchmark.initializeNewActions (actionCount=1) 0.668 ± 0.036 us/op 0.829 ± 0.042 us/op +24.2% 🔴
com.instacart.formula.benchmarks.CallbackOverheadBenchmark.transitions (callbackCount=50) 14.56 ± 0.502 us/op 17.446 ± 0.785 us/op +19.8% 🔴
com.instacart.formula.benchmarks.ActionCountBenchmark.stateChanges (actionCount=100) 13.937 ± 0.073 us/op 16.679 ± 0.154 us/op +19.7% 🔴
com.instacart.formula.benchmarks.CallbackOverheadBenchmark.transitions (callbackCount=10) 13.111 ± 0.055 us/op 15.664 ± 0.2 us/op +19.5% 🔴
com.instacart.formula.benchmarks.ActionCountBenchmark.stateChanges (actionCount=25) 13.066 ± 0.123 us/op 15.58 ± 0.198 us/op +19.2% 🔴
com.instacart.formula.benchmarks.ActionCountBenchmark.stateChanges (actionCount=1) 12.835 ± 0.164 us/op 15.278 ± 0.156 us/op +19.0% 🔴
com.instacart.formula.benchmarks.TransitionBenchmark.transitions (depth=20) 13.3 ± 0.105 us/op 15.758 ± 0.164 us/op +18.5% 🔴
com.instacart.formula.benchmarks.ChildrenCountBenchmark.stateChanges (childrenCount=25) 13.211 ± 0.091 us/op 15.626 ± 0.216 us/op +18.3% 🔴
com.instacart.formula.benchmarks.ChildrenCountBenchmark.stateChanges (childrenCount=1) 12.869 ± 0.218 us/op 15.214 ± 0.149 us/op +18.2% 🔴
com.instacart.formula.benchmarks.TransitionBenchmark.transitions (depth=0) 12.788 ± 0.089 us/op 15.107 ± 0.068 us/op +18.1% 🔴
com.instacart.formula.benchmarks.ChildrenCountBenchmark.stateChanges (childrenCount=100) 14.674 ± 0.203 us/op 17.3 ± 0.149 us/op +17.9% 🔴
com.instacart.formula.benchmarks.TransitionBenchmark.transitions (depth=10) 13.059 ± 0.212 us/op 15.383 ± 0.152 us/op +17.8% 🔴
com.instacart.formula.benchmarks.ActionInitializationBenchmark.initializeNewActions (actionCount=25) 3.073 ± 0.052 us/op 3.564 ± 0.062 us/op +16.0% 🔴
com.instacart.formula.benchmarks.ChildrenInitializationBenchmark.initializeNewChildren (childrenCount=25) 3.43 ± 0.243 us/op 3.841 ± 0.117 us/op +12.0% 🔴
No significant changes (6 benchmarks)
Benchmark Baseline Current Change
com.instacart.formula.benchmarks.ActionInitializationBenchmark.initializeNewActions (actionCount=100) 10.424 ± 0.173 us/op 11.271 ± 0.12 us/op +8.1%
com.instacart.formula.benchmarks.CallbackInitializationBenchmark.initializeNewCallbacks (callbackCount=10) 1.584 ± 0.231 us/op 1.993 ± 0.233 us/op +25.8%
com.instacart.formula.benchmarks.CallbackInitializationBenchmark.initializeNewCallbacks (callbackCount=50) 5.686 ± 0.922 us/op 7.141 ± 2.082 us/op +25.6%
com.instacart.formula.benchmarks.ChildrenInitializationBenchmark.initializeNewChildren (childrenCount=100) 12.192 ± 0.527 us/op 13.068 ± 0.36 us/op +7.2%
com.instacart.formula.benchmarks.GlobalEffectQueueBenchmark.measure10Effects 0.709 ± 0.009 us/op 0.748 ± 0.006 us/op +5.5%
com.instacart.formula.benchmarks.GlobalEffectQueueBenchmark.measure1Effect 0.053 ± 0.002 us/op 0.058 ± 0.0 us/op +9.5%

Regressions: ±10% with non-overlapping confidence intervals. Improvements: ±10% change only.

Generated by 🚫 Danger

Copy link
Copy Markdown

@deesonpatel deesonpatel left a comment

Choose a reason for hiding this comment

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

This would be a breaking change. We should probably note that.

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