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.

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

feat: Further improve performance of stack function #1847

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
65 changes: 55 additions & 10 deletions lib/timeline/Stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,22 +295,28 @@ 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));
}
if(previousEnd !== null && previousEnd - EPSILON > itemEnd) {
horizontalOverlapEndIndex = findLastIndexBetween(itemsAlreadyPositioned, i => itemEnd + EPSILON >= getItemStart(i), horizontalOverlapStartIndex, horizontalOverlapEndIndex) + 1;
}
previousEnd = itemEnd;

// 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,
horizontalOverlapStartIndex,
horizontalOverlapEndIndex
)
.sort((a, b) => a.top - b.top);

// Keep moving the item down until it stops colliding with any other items
Expand All @@ -329,6 +335,15 @@ function performStacking(items, margins, compareTimes, shouldStack, shouldOthers
insertionIndex = findIndexFrom(itemsAlreadyPositioned, i => getItemStart(i) - EPSILON > itemStart, insertionIndex);

itemsAlreadyPositioned.splice(insertionIndex, 0, item);

if(insertionIndex < horizontalOverlapStartIndex) {
horizontalOverlapStartIndex++;
}

if(insertionIndex <= horizontalOverlapEndIndex) {
horizontalOverlapEndIndex++;
}

insertionIndex++;
}

Expand Down Expand Up @@ -374,11 +389,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 +415,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;
}
21 changes: 20 additions & 1 deletion lib/timeline/component/Group.js
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,8 @@ class Group {
this._traceVisible(initialPosByEnd, orderedItems.byEnd, visibleItems, visibleItemsLookup, item => item.data.end < lowerBound || item.data.start > upperBound);
}

this._sortVisibleItems(orderedItems.byStart, visibleItems, visibleItemsLookup);

const redrawQueue = {};
let redrawQueueLength = 0;

Expand Down Expand Up @@ -1011,7 +1013,7 @@ class Group {
if (!(item.isCluster && !item.hasItems()) && !item.cluster) {
if (visibleItemsLookup[item.id] === undefined) {
visibleItemsLookup[item.id] = true;
visibleItems.push(item);
visibleItems.unshift(item);
}
}
}
Expand All @@ -1034,6 +1036,23 @@ class Group {
}
}

/**
* by-ref reordering of visibleItems array to match
* the specified item superset order
* @param {array} orderedItems
* @param {aray} visibleItems
* @param {object} visibleItemsLookup
*/
_sortVisibleItems(orderedItems, visibleItems, visibleItemsLookup) {
visibleItems.length = 0; // Clear visibleItems array in-place
for(let i = 0; i < orderedItems.length; i++) {
let item = orderedItems[i];
if(visibleItemsLookup[item.id]) {
visibleItems.push(item);
}
}
}

/**
* this function is very similar to the _checkIfInvisible() but it does not
* return booleans, hides the item if it should not be seen and always adds to
Expand Down