Skip to content

Commit

Permalink
Remove expensive uses of Array.slice
Browse files Browse the repository at this point in the history
When I wrote visjs#1281 I was under the impression that calling `Array.slice` and then using the resulting array in a read-only capacity would be optimised by JS engines to use an offset/crop into the same memory as the original array, similar to how Node's Buffer.subarray works.

I have since discovered that this is not the case: `Array.slice` is an expensive function that physically shallow-copies the array. Avoiding the use of `slice` significantly improves stacking performance with large amounts of items.

(On a dataset of 6,000 items, this reduces stacking time by ~27%!)
  • Loading branch information
Yoshi Walsh committed Dec 27, 2024
1 parent 5394cf3 commit 41ae67d
Showing 1 changed file with 45 additions and 10 deletions.
55 changes: 45 additions & 10 deletions lib/timeline/Stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,12 @@ function performStacking(items, margins, compareTimes, shouldStack, shouldOthers
insertionIndex = 0;
previousEnd = null;
}
if(previousStart === null || itemStart > previousStart + EPSILON) {
// Take advantage of the sorted itemsAlreadyPositioned array to narrow down the search
horizontalOverlapStartIndex = findIndexFrom(itemsAlreadyPositioned, i => itemStart < getItemEnd(i) - EPSILON, horizontalOverlapStartIndex);
}
previousStart = itemStart;

// Take advantage of the sorted itemsAlreadyPositioned array to narrow down the search
horizontalOverlapStartIndex = findIndexFrom(itemsAlreadyPositioned, i => itemStart < getItemEnd(i) - EPSILON, horizontalOverlapStartIndex);
// Since items aren't sorted by end time, it might increase or decrease from one item to the next. In order to keep an efficient search area, we will seek forwards/backwards accordingly.
if(previousEnd === null || previousEnd < itemEnd - EPSILON) {
horizontalOverlapEndIndex = findIndexFrom(itemsAlreadyPositioned, i => itemEnd < getItemStart(i) - EPSILON, Math.max(horizontalOverlapStartIndex, horizontalOverlapEndIndex));
Expand All @@ -308,9 +310,12 @@ function performStacking(items, margins, compareTimes, shouldStack, shouldOthers
}

// Sort by vertical position so we don't have to reconsider past items if we move an item
const horizontallyCollidingItems = itemsAlreadyPositioned
.slice(horizontalOverlapStartIndex, horizontalOverlapEndIndex)
.filter(i => itemStart < getItemEnd(i) - EPSILON && itemEnd - EPSILON > getItemStart(i))
const horizontallyCollidingItems = filterBetween(
itemsAlreadyPositioned,
i => itemStart < getItemEnd(i) - EPSILON && itemEnd - EPSILON > getItemStart(i),
horizontalOverlapStartIndex,
horizontalOverlapEndIndex
)
.sort((a, b) => a.top - b.top);

// Keep moving the item down until it stops colliding with any other items
Expand Down Expand Up @@ -374,11 +379,12 @@ function findIndexFrom(arr, predicate, startIndex) {
if(!startIndex) {
startIndex = 0;
}
const matchIndex = arr.slice(startIndex).findIndex(predicate);
if(matchIndex === -1) {
return arr.length;
for(var i = startIndex; i < arr.length; i++) {
if(predicate(arr[i])) {
return i;
}
}
return matchIndex + startIndex;
return arr.length;
}

/**
Expand All @@ -399,10 +405,39 @@ function findLastIndexBetween(arr, predicate, startIndex, endIndex) {
if(!endIndex) {
endIndex = arr.length;
}
for(i = endIndex - 1; i >= startIndex; i--) {
for(var i = endIndex - 1; i >= startIndex; i--) {
if(predicate(arr[i])) {
return i;
}
}
return startIndex - 1;
}

/**
* Takes an array and returns an array containing only items which meet a predicate within a given range.
*
* @param {any[]} arr The array
* @param {(item) => boolean} predicate A function that should return true for items which should be included within the result
* @param {number|undefined} startIndex The earliest index to include (inclusive). Optional, if not provided will continue until the start of the array.
* @param {number|undefined} endIndex The end of the range to filter (exclusive). Optional, defaults to the end of array.
*
* @return {number}
*/
function filterBetween(arr, predicate, startIndex, endIndex) {
if(!startIndex) {
startIndex = 0;
}
if(endIndex) {
endIndex = Math.min(endIndex, arr.length);
} else {
endIndex = arr.length;
}

var result = [];
for(var i = startIndex; i < endIndex; i++) {
if(predicate(arr[i])) {
result.push(arr[i]);
}
}
return result;
}

0 comments on commit 41ae67d

Please sign in to comment.