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

Use sparse array for cell position caches #1312

Merged
merged 2 commits into from
May 3, 2019

Conversation

trxcllnt
Copy link
Contributor

@trxcllnt trxcllnt commented Jan 27, 2019

This PR swaps out the hash based cell position caches and the underlying binary search with a constant-time sparse array linear layout vector implementation.

The vector stores element sizes in a single dimension. Sizes are stored in blocks with power-of-two length. The vector supports constant-time (index) => position lookup, update, insertion, and removal by shifting the index to find the block index, then a mask (mod) to find the cell index.

Reverse lookup (position) => index is currently linear with respect to the number of blocks/size of each block. Flamegraphs indicate that on a table with 1MM rows and with the default block_size of 128, this amounts to about 0.15-1ms (150-1000 microseconds) per call when scrolling at the end of the list. If avoiding linear scans is an issue on principle, it would be possible to cache and invalidate the block sizes' prefix sum, and implement this lookup as a binary search again. The block size can also be intelligently tuned based on total element count for a classic space/time trade-off.

This change means the following methods are now O(1):

getSizeAndPositionOfCell
getSizeAndPositionOfLastMeasuredCell
getUpdatedOffsetForIndex

getVisibleCellRange is O(n), where n=num_blocks

The biggest practical benefit is that cell dimensions are stable across invalidation. After initial measurement the scrollbars won't jump around, as you can see in this screen capture I uploaded to youtube. The first half is rendering a MultiGrid with react-virtualized@9.21.0, and the second half is rendering the same MultiGrid with the current PR. You can find the source for this demo here.

react virtualized scrolling

  • The existing test suites (npm test) all pass
  • For any new features or bug fixes, both positive and negative test cases have been added
  • For any new features, documentation has been added
  • For any documentation changes, the text has been proofread and is clear to both experienced users and beginners.
  • Format your code with prettier (npm run prettier).
  • Run the Flow typechecks (npm run typecheck).

@wuweiweiwu
Copy link
Contributor

@trxcllnt What is the size impact of this dependency?

@trxcllnt
Copy link
Contributor Author

trxcllnt commented Feb 25, 2019

2.2kb minified (w/ closure compiler simple optimizations) + gzipped, though that could be even smaller if cleaned up and converted from ES5 to an ES6 class. The version up on npm is a few years old, and that was converted to ES5 from ActionScript a few years before that.

@omerts
Copy link

omerts commented Mar 6, 2019

@trxcllnt This PR really caught my attention, and is an interesting concept.. How is getSizeAndPositionOfCell O(1) if there is still a for loop iterating over the cells and calculating their size?

Copy link
Contributor

@wuweiweiwu wuweiweiwu left a comment

Choose a reason for hiding this comment

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

Looks good to me!!

Thanks for doing this!

@wuweiweiwu wuweiweiwu merged commit 7be1258 into bvaughn:master May 3, 2019
wuweiweiwu added a commit that referenced this pull request May 22, 2019
wuweiweiwu added a commit that referenced this pull request Jun 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants