Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[perf] Improve SchedulerMinHeap implementation #21147

Merged
merged 2 commits into from
Apr 6, 2021
Merged

[perf] Improve SchedulerMinHeap implementation #21147

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fb47c2
Improve MinHeap implementation
Tom910 Mar 30, 2021
49d3249
Revert unrelated changes
acdlite Apr 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 16 additions & 17 deletions packages/scheduler/src/SchedulerMinHeap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,28 @@ export function push(heap: Heap, node: Node): void {
}

export function peek(heap: Heap): Node | null {
const first = heap[0];
return first === undefined ? null : first;
return heap.length === 0 ? null : heap[0];
}

export function pop(heap: Heap): Node | null {
const first = heap[0];
if (first !== undefined) {
const last = heap.pop();
if (last !== first) {
heap[0] = last;
siftDown(heap, last, 0);
}
return first;
} else {
if (heap.length === 0) {
return null;
}
const first = heap[0];
const last = heap.pop();
if (last !== first) {
heap[0] = last;
siftDown(heap, last, 0);
}
return first;
}

function siftUp(heap, node, i) {
let index = i;
while (true) {
while (index > 0) {
const parentIndex = (index - 1) >>> 1;
const parent = heap[parentIndex];
if (parent !== undefined && compare(parent, node) > 0) {
if (compare(parent, node) > 0) {
// The parent is larger. Swap positions.
heap[parentIndex] = node;
heap[index] = parent;
Expand All @@ -58,15 +56,16 @@ function siftUp(heap, node, i) {
function siftDown(heap, node, i) {
let index = i;
const length = heap.length;
while (index < length) {
const halfLength = length >>> 1;
while (index < halfLength) {
const leftIndex = (index + 1) * 2 - 1;
const left = heap[leftIndex];
const rightIndex = leftIndex + 1;
const right = heap[rightIndex];
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Tom910 Isn't the algorithm going beyond the array length here potentially? Otherwise we wouldn't need to check if rightIndex < length later, right?

Or does v8 only actually go through the prototype chain once we actually use the value e.g. when doing right !== undefined?

Or is de-opting here fine? You mentioned benchmarks in your PR description but I couldn't find where they are located. Could you share them for (future) testing?


// If the left or right node is smaller, swap with the smaller of those.
if (left !== undefined && compare(left, node) < 0) {
if (right !== undefined && compare(right, left) < 0) {
if (compare(left, node) < 0) {
if (rightIndex < length && compare(right, left) < 0) {
heap[index] = right;
heap[rightIndex] = node;
index = rightIndex;
Expand All @@ -75,7 +74,7 @@ function siftDown(heap, node, i) {
heap[leftIndex] = node;
index = leftIndex;
}
} else if (right !== undefined && compare(right, node) < 0) {
} else if (rightIndex < length && compare(right, node) < 0) {
heap[index] = right;
heap[rightIndex] = node;
index = rightIndex;
Expand Down