Conversation
Optimize `_sortNodeUp/Down` methods and `_moveNode` method (11x improvement)
Optimize `_sortNodeUp/Down` methods and `_moveNode` method
…formance improvements and contributions
WalkthroughVersion bumped to 2.7.0. Internal refactors in Heap and HeapAsync adjust heapify start index, parent index computation, swapping, and sift up/down implementations. Documentation HTML “Defined in” links updated to new commit references and changelog entries added. Tests gain ESLint overrides to allow any. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Heap as Heap (sync)
participant Cmp as Comparator
rect rgb(245,250,255)
note over Heap: Initialization / heapify (start from parent(last))
Client->>Heap: new Heap(items, comparator)
Heap->>Heap: for i = parent(last) downto 0
Heap->>Heap: _sortNodeDown(i) -- uses cached value + final assignment
end
rect rgb(245,255,245)
note over Heap: Push
Client->>Heap: push(value)
Heap->>Heap: insert at end
Heap->>Heap: _sortNodeUp(idx) -- cached value + final assignment
Heap->>Cmp: compare during ascent
end
rect rgb(255,248,240)
note over Heap: Pop
Client->>Heap: pop()
Heap->>Heap: move last to root (temp swap)
Heap->>Heap: _sortNodeDown(0)
Heap->>Cmp: compare during descent
end
sequenceDiagram
autonumber
actor Client
participant AHeap as HeapAsync
participant ACmp as AsyncComparator
note over AHeap: Same control flow as sync, with awaited comparisons
Client->>AHeap: push/pop/top...
AHeap->>AHeap: _sortNodeUp/_sortNodeDown (iterative, cached value)
AHeap->>ACmp: await compare(...)
ACmp-->>AHeap: Promise<number>
AHeap-->>Client: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/types/IsEqual.html (1)
1-1: Repository URL mismatch in generated documentation.The "Defined in" link references
github.com/BeatsuDev/heap-jsinstead ofgithub.com/ignlg/heap-js(where this PR resides). This is consistent with the same issue in other documentation files and indicates the TypeDoc output was generated from incorrect repository settings.docs/types/AsyncComparator.html (1)
1-1: Repository URL mismatch in generated documentation.The "Defined in" link references
github.com/BeatsuDev/heap-jsinstead of the correctgithub.com/ignlg/heap-jsrepository. This matches the issue found in the other documentation files.
🧹 Nitpick comments (1)
src/HeapAsync.ts (1)
430-438: Optimize heapify loop start indexIn
src/HeapAsync.ts’sinit, adjust the loop to begin at the parent of the last element rather than atMath.floor(this.heapArray.length / 2), which invokes_sortNodeDownonce on a leaf. For example:- for (let i = Math.floor(this.heapArray.length / 2); i >= 0; --i) { + for (let i = Math.floor(this.heapArray.length / 2) - 1; i >= 0; --i) { await this._sortNodeDown(i); }This is equivalent to using
getParentIndexOf(this.heapArray.length - 1)and removes an unnecessary leaf‐node call without affecting correctness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
dist/heap-js.es5.jsis excluded by!**/dist/**dist/heap-js.umd.jsis excluded by!**/dist/**
📒 Files selected for processing (12)
README.md(2 hunks)docs/functions/toInt.html(1 hunks)docs/index.html(2 hunks)docs/types/AsyncComparator.html(1 hunks)docs/types/AsyncIsEqual.html(1 hunks)docs/types/Comparator.html(1 hunks)docs/types/IsEqual.html(1 hunks)package.json(1 hunks)src/Heap.ts(5 hunks)src/HeapAsync.ts(3 hunks)tests/heap-async/heap-async-private-methods.test.ts(1 hunks)tests/heap/heap-private-methods.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/HeapAsync.ts (1)
src/Heap.ts (1)
length(511-513)
src/Heap.ts (1)
src/HeapAsync.ts (1)
length(463-465)
🔇 Additional comments (16)
package.json (1)
3-3: LGTM! Version bump aligns with PR objectives.The version increment from 2.6.0 to 2.7.0 is appropriate for a minor release that includes performance improvements and internal refactors.
tests/heap/heap-private-methods.test.ts (1)
1-1: LGTM! ESLint override is appropriate for private method tests.The
no-explicit-anyoverride is reasonable here since this test file explicitly tests heap operations with various types includingany. This aligns with the parallel change in the async heap test file.docs/functions/toInt.html (1)
1-1: Documentation-only change: Repository reference updated.The "Defined in" link has been updated from ignlg/heap-js to BeatsuDev/heap-js. This is a generated documentation file with no functional impact.
README.md (2)
46-50: LGTM: Version 2.7 changelog added.The new changelog entry appropriately documents the performance improvements and acknowledges the contributor.
77-77: LGTM: Markdown link formatting corrected.The closing bracket has been correctly moved to fix the URL formatting for the Java PriorityQueue reference.
docs/types/Comparator.html (1)
1-1: Documentation-only change: Repository reference updated.The "Defined in" link has been updated from ignlg/heap-js to BeatsuDev/heap-js. This is a generated documentation file with no functional impact.
docs/index.html (1)
14-18: LGTM: Version 2.6 changelog section added to HTML documentation.The changelog correctly documents the performance improvements and test/documentation enhancements for version 2.6, maintaining consistency with the README.
tests/heap-async/heap-async-private-methods.test.ts (1)
1-1: LGTM: ESLint directive added for test flexibility.The
no-explicit-anyrule is appropriately disabled for this test file, which tests generic heap behavior across multiple type configurations. This is a reasonable accommodation for test code.src/Heap.ts (5)
64-69: LGTM! Bit-shift optimization for parent index calculation.The change from
Math.floor((idx - 1) / 2)to(idx - 1) >> 1is a standard performance optimization for binary heap parent index calculation. The bit-shift operation is mathematically equivalent and more efficient.
423-431: LGTM! Optimal heapify starting index.Starting the heapify loop from
Heap.getParentIndexOf(this.length - 1)is the correct optimization. This index represents the last non-leaf node in the heap, and all nodes after it are leaves that don't require sifting down. This is the standard Floyd's algorithm for O(n) heap construction.
805-809: LGTM! Swap optimization using temporary variable.Replacing array destructuring with a temporary variable is a micro-optimization that avoids temporary array allocation. This can improve performance in the hot path of heap operations.
815-833: LGTM! Optimized sift-down implementation.The refactored
_sortNodeDownuses a standard optimization pattern:
- Caches the original value to reduce array accesses
- Computes the best child in a single step
- Moves children up during traversal
- Performs a single final assignment of the original value
This reduces the number of array writes and is a well-established heap performance optimization.
839-852: LGTM! Optimized sift-up implementation.The refactored
_sortNodeUpmirrors the_sortNodeDownoptimization:
- Caches the original value to reduce array accesses
- Moves parents down during upward traversal
- Performs a single final assignment at the correct position
This optimization reduces array writes and is consistent with the sift-down improvements.
src/HeapAsync.ts (3)
729-733: LGTM! Swap optimization using temporary variable.Consistent with the optimization in
src/Heap.ts, this replaces array destructuring with a temporary variable to avoid array allocation overhead.
739-759: LGTM! Async version of optimized sift-down.The refactored
_sortNodeDowncorrectly mirrors the optimization insrc/Heap.tswith proper async/await handling for comparisons. The value caching and single-write pattern is preserved.
765-778: LGTM! Async version of optimized sift-up.The refactored
_sortNodeUpcorrectly mirrors the optimization insrc/Heap.tswith proper async/await handling. The implementation is consistent and correct.
Summary by CodeRabbit