From 57d3b9e2ca65f591caa45a8fa6424995cfc399ca Mon Sep 17 00:00:00 2001 From: Genki Kondo Date: Mon, 16 May 2022 13:14:35 -0700 Subject: [PATCH] Modify VirtualizeUtils.elementsThatOverlapOffsets to use binary search Summary: elementsThatOverlapOffsets was quite inefficient as it was doing a linear scan over all items without an early out for each input offset. The number of items can be large, so we'd want to use binary search here. Before: O(MN), where M is the # offsets and N is the # items After: O(MlogN) As a utility method, elementsThatOverlapOffsets should not be responsible for checking that the offsets are in increasing order 'binary-search' dep added via https://www.internalfb.com/intern/wiki/React_Native/Building_Product_Experiences/Third_Party_Packages_(npm)/ Changelog: [Internal] - Modify VirtualizeUtils.elementsThatOverlapOffsets to use binary search Reviewed By: javache Differential Revision: D36292326 fbshipit-source-id: 5f22eb5533b69e2ebe5c1cb34e4f82605838f0a7 --- Libraries/Lists/VirtualizeUtils.js | 45 +++++++++++-------- .../Lists/__tests__/VirtualizeUtils-test.js | 27 +++++------ .../VirtualizeUtils-test.js.snap | 3 -- 3 files changed, 41 insertions(+), 34 deletions(-) delete mode 100644 Libraries/Lists/__tests__/__snapshots__/VirtualizeUtils-test.js.snap diff --git a/Libraries/Lists/VirtualizeUtils.js b/Libraries/Lists/VirtualizeUtils.js index 9b92f42bfc2..70e48ff1a20 100644 --- a/Libraries/Lists/VirtualizeUtils.js +++ b/Libraries/Lists/VirtualizeUtils.js @@ -27,27 +27,36 @@ export function elementsThatOverlapOffsets( }, zoomScale: number = 1, ): Array { - const out = []; - let outLength = 0; - for (let ii = 0; ii < itemCount; ii++) { - const frame = getFrameMetrics(ii); - const trailingOffset = (frame.offset + frame.length) * zoomScale; - for (let kk = 0; kk < offsets.length; kk++) { - if (out[kk] == null && trailingOffset >= offsets[kk]) { - out[kk] = ii; - outLength++; - if (kk === offsets.length - 1) { - invariant( - outLength === offsets.length, - 'bad offsets input, should be in increasing order: %s', - JSON.stringify(offsets), - ); - return out; - } + const result = []; + for (let offsetIndex = 0; offsetIndex < offsets.length; offsetIndex++) { + const currentOffset = offsets[offsetIndex]; + let left = 0; + let right = itemCount - 1; + + while (left <= right) { + // eslint-disable-next-line no-bitwise + const mid = left + ((right - left) >>> 1); + const frame = getFrameMetrics(mid); + const scaledOffsetStart = frame.offset * zoomScale; + const scaledOffsetEnd = (frame.offset + frame.length) * zoomScale; + + // We want the first frame that contains the offset, with inclusive bounds. Thus, for the + // first frame the scaledOffsetStart is inclusive, while for other frames it is exclusive. + if ( + (mid === 0 && currentOffset < scaledOffsetStart) || + (mid !== 0 && currentOffset <= scaledOffsetStart) + ) { + right = mid - 1; + } else if (currentOffset > scaledOffsetEnd) { + left = mid + 1; + } else { + result[offsetIndex] = mid; + break; } } } - return out; + + return result; } /** diff --git a/Libraries/Lists/__tests__/VirtualizeUtils-test.js b/Libraries/Lists/__tests__/VirtualizeUtils-test.js index c700d599100..5f4de3e52e1 100644 --- a/Libraries/Lists/__tests__/VirtualizeUtils-test.js +++ b/Libraries/Lists/__tests__/VirtualizeUtils-test.js @@ -65,8 +65,20 @@ describe('elementsThatOverlapOffsets', function () { elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii], 1), ).toEqual([1, 1, 3]); }); + it('handles frame boundaries', function () { + const offsets = [0, 100, 200, 300]; + function getFrameMetrics(index: number) { + return { + length: 100, + offset: 100 * index, + }; + } + expect( + elementsThatOverlapOffsets(offsets, 100, getFrameMetrics, 1), + ).toEqual([0, 0, 1, 2]); + }); it('handles out of bounds', function () { - const offsets = [150, 900]; + const offsets = [-100, 150, 900]; const frames = [ {offset: 0, length: 50}, {offset: 50, length: 150}, @@ -74,17 +86,6 @@ describe('elementsThatOverlapOffsets', function () { ]; expect( elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii], 1), - ).toEqual([1]); - }); - it('errors on non-increasing offsets', function () { - const offsets = [150, 50]; - const frames = [ - {offset: 0, length: 50}, - {offset: 50, length: 150}, - {offset: 250, length: 100}, - ]; - expect(() => { - elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii], 1); - }).toThrowErrorMatchingSnapshot(); + ).toEqual([undefined, 1]); }); }); diff --git a/Libraries/Lists/__tests__/__snapshots__/VirtualizeUtils-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/VirtualizeUtils-test.js.snap deleted file mode 100644 index 6daafe6961f..00000000000 --- a/Libraries/Lists/__tests__/__snapshots__/VirtualizeUtils-test.js.snap +++ /dev/null @@ -1,3 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`elementsThatOverlapOffsets errors on non-increasing offsets 1`] = `"bad offsets input, should be in increasing order: [150,50]"`;