performance improvements 1 #7
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Level calculation (Signals__Scheduler.res:150-199) - Replaced array allocation + reduce with mutable ref for max tracking - Before: let computedLevels = [] → Array.push → Array.reduce - After: let maxLevel = ref(0) → inline comparison 2. Flush function (Signals__Scheduler.res:234-261) - Eliminated O(n log n) Map lookups during sort - Pre-allocated pendingComputeds and pendingEffects arrays - Added efficient clearArray using arr.length = 0 - Separate sorting for computeds vs effects (simpler comparator) 3. addDep function (Signals__Scheduler.res:116-129) - Removed redundant isCurrentObserver check - Moved SignalObservers.ensure inside the "new dep" branch - Reduced operations on the hot path
- Add backingSubs field to observer for direct dirty propagation (no Map lookup) - Remove computedSubs Map entirely - Skip computeLevel on subsequent runs when level is already set - 87% improvement on signal updates benchmark
Add needsSort check to avoid O(n log n) sort overhead when all pending observers have the same level, which is common for flat dependency structures.
- Skip clearDeps on fresh observer (no deps yet) - Defer computeLevel to first retrack (lazy) - Remove try/catch from creation hot path - Add makeForComputed for optimized backing signal
brnrdog
force-pushed
the
perf-improvements-1
branch
from
35884de to
89c8b2d
Compare
|
🎉 This PR is included in version 1.3.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
I run some benchmarks with rescript-signals using the js-reactivity-benchmark project. It didn't go well in many categories, so I decided to spend some time investing in improving.
This resulted in some small changes and other deep structural changes, especially in our internal data structures.
The API and expected behaviours remain the same.
Performance Improvements
The following are results from a custom benchmark script, also included in this pull request:
The most dramatic improvements are in creation paths (20-27x faster) due to eliminating the separate observer object
allocation for computeds. The signal update path is also significantly faster (5.8x) from avoiding Map lookups and
pointer indirection during dirty propagation.