Use binary search in getVisibleElements() #5595

fkaelberer · 2014-12-29T18:22:50Z

The method getVisibleElements() uses a linear search to find the first page and thumbnail in the visible range. It is called on every scroll event, so it can take a noticeable amount of time for the bottom pages of a very large document.
This PR replaces the linear search with a binary one (which needs O(log n) time).

A good test:
[3] pdf (5000+ pages)

A benchmark:
Scrolling through pages 5500 to 5515 of [3], using the ↓ key, Linux, Firefox Nightly 37.0a1, textLayer=off
Before:

After:

(Admittedly, the document has to be very large to notice a difference, but it should be more pronounced on slow devices).

yurydelendik · 2014-12-29T18:39:23Z

Really like binary search idea :) I think you can perform binary search only once: to find first div that lays on (or about to touch) top viewport boundary. Then we can scan down to find bottom one (I think using binary search for second part might be slower since it will touch more of clientTop and other similar DOM properties)

On different note: did you look into using https://developer.mozilla.org/en-US/docs/Web/API/element.getBoundingClientRect ?

fkaelberer · 2014-12-29T19:09:41Z

I think you can perform binary search only once:

that might be true, but since the second binary search is very cheap (because it starts where the first search ends), it wouldn't matter much.

(I think using binary search for second part might be slower since it will touch more of clientTop and other similar DOM properties)

I don't think that matters too, because once the layout is done (after the first call of top, clientTop, or similar), further calls to these properties are quite fast.

On different note: did you look into using https://developer.mozilla.org/en-US/docs/Web/API/element.getBoundingClientRect?

I'll have a look.

fkaelberer · 2014-12-29T19:16:48Z

More numbers: getVisibleElements() takes ~12ms to scroll through 5500 pages, 7ms for the thumbnails. Together this is more time than we are allowed to spend within an 60fps animation frame, but it does not matter if we check 5 or 55 pages, because I wouldn't care if it takes 12µs or 120µs.

CodingFabian · 2014-12-29T19:32:50Z

to be fair, using the key down is not even realistic. this method was hot before i added the rAF debouncing. its still hot on mac which likes to fire millions of scroll events when using the touchpad. everything helps here. even 12µs.

fkaelberer · 2014-12-29T20:02:50Z

its still hot on mac which likes to fire millions of scroll events when using the touchpad.

Since we have rAF now, the method is not be called too often. The method still seems to be very heavy as shown in the profiler, but it isn't: Most of the time is spent in the first line var top = scrollEl.scrollTop, which invokes the layout of the DOM tree (which you can see in the Chrome browser's timeline). It is the layout that is really slow, not the getVisibleElements() method.

fkaelberer · 2014-12-31T15:04:37Z

I think using binary search for second part might be slower...

Ok, after thinking about it I agree that linear search might even be faster for the second part, because binary search doesn't profit from the fact that the item to find is likely very close to the start of the search.
I reverted the stop condition of the visibility search to a break statement as it was before, which also makes things a little simpler.

fkaelberer · 2014-12-31T15:06:24Z

On different note: did you look into using https://developer.mozilla.org/en-US/docs/Web/API/element.getBoundingClientRect ?

@yurydelendik I had a look, but honestly I don't know how this could be helpful.

yurydelendik · 2015-02-27T12:42:25Z

web/ui_utils.js

-    if ((currentHeight + viewHeight) < top) {
-      continue;
-    }
+    elem = view.el;


We removed el from the page/thumb. Could you replace this with div?

nit: elem -> element

fkaelberer · 2015-03-01T16:43:09Z

@yurydelendik I addressed your comments.

timvandermeij · 2015-03-01T17:20:35Z

/botio-linux preview

pdfjsbot · 2015-03-01T17:20:35Z

From: Bot.io (Linux)

Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/271bf1a4bf2caf0/output.txt

pdfjsbot · 2015-03-01T17:21:35Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/271bf1a4bf2caf0/output.txt

Total script time: 1.00 mins

Published

yurydelendik · 2015-03-10T04:09:16Z

/botio unittest

pdfjsbot · 2015-03-10T04:09:16Z

From: Bot.io (Linux)

Received

Command cmd_unittest from @yurydelendik received. Current queue size: 0

Live output at: http://107.21.233.14:8877/e384c9cbfd80aa1/output.txt

pdfjsbot · 2015-03-10T04:09:59Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/e384c9cbfd80aa1/output.txt

Total script time: 0.71 mins

Unit Tests: Passed

pdfjsbot · 2015-03-10T04:11:01Z

From: Bot.io (Windows)

Received

Command cmd_unittest from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/f3c665227f34dc8/output.txt

pdfjsbot · 2015-03-10T04:11:43Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/f3c665227f34dc8/output.txt

Total script time: 0.70 mins

Unit Tests: Passed

…Elements Use binary search in getVisibleElements()

yurydelendik · 2015-03-10T04:12:06Z

Really good. Thank you for the patch.

fkaelberer force-pushed the useBinarySearchToFindVisibleElements branch from cebc851 to 6a72760 Compare December 29, 2014 18:23

fkaelberer changed the title ~~Update thumbnail style only when visible~~ Use binary search in getVisibleElements() Dec 29, 2014

timvandermeij added viewer performance labels Dec 29, 2014

fkaelberer force-pushed the useBinarySearchToFindVisibleElements branch 2 times, most recently from 0ba69b0 to 351909a Compare December 31, 2014 15:04

fkaelberer force-pushed the useBinarySearchToFindVisibleElements branch from 351909a to 347cdde Compare December 31, 2014 15:24

yurydelendik reviewed Feb 27, 2015
View reviewed changes

fkaelberer force-pushed the useBinarySearchToFindVisibleElements branch 3 times, most recently from 8fe8820 to f47dcb1 Compare March 1, 2015 16:37

Use binary search in getVisibleElements()

a78bb6b

fkaelberer force-pushed the useBinarySearchToFindVisibleElements branch from f47dcb1 to a78bb6b Compare March 1, 2015 16:41

timvandermeij assigned yurydelendik Mar 6, 2015

yurydelendik added a commit that referenced this pull request Mar 10, 2015

Merge pull request #5595 from fkaelberer/useBinarySearchToFindVisible…

f4dbd69

…Elements Use binary search in getVisibleElements()

yurydelendik merged commit f4dbd69 into mozilla:master Mar 10, 2015

fkaelberer mentioned this pull request Mar 15, 2015

Don't layout sidebar when hidden #5617

Closed

fkaelberer deleted the useBinarySearchToFindVisibleElements branch March 15, 2015 15:45

timvandermeij mentioned this pull request Mar 15, 2015

Poor performance on a pdf document #4626

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use binary search in getVisibleElements() #5595

Use binary search in getVisibleElements() #5595

fkaelberer commented Dec 29, 2014

yurydelendik commented Dec 29, 2014

fkaelberer commented Dec 29, 2014

fkaelberer commented Dec 29, 2014

CodingFabian commented Dec 29, 2014

fkaelberer commented Dec 29, 2014

fkaelberer commented Dec 31, 2014

fkaelberer commented Dec 31, 2014

yurydelendik Feb 27, 2015

yurydelendik Feb 27, 2015

fkaelberer commented Mar 1, 2015

timvandermeij commented Mar 1, 2015

pdfjsbot commented Mar 1, 2015

pdfjsbot commented Mar 1, 2015

yurydelendik commented Mar 10, 2015

pdfjsbot commented Mar 10, 2015

pdfjsbot commented Mar 10, 2015

pdfjsbot commented Mar 10, 2015

pdfjsbot commented Mar 10, 2015

yurydelendik commented Mar 10, 2015

Use binary search in getVisibleElements() #5595

Use binary search in getVisibleElements() #5595

Conversation

fkaelberer commented Dec 29, 2014

yurydelendik commented Dec 29, 2014

fkaelberer commented Dec 29, 2014

fkaelberer commented Dec 29, 2014

CodingFabian commented Dec 29, 2014

fkaelberer commented Dec 29, 2014

fkaelberer commented Dec 31, 2014

fkaelberer commented Dec 31, 2014

yurydelendik Feb 27, 2015

Choose a reason for hiding this comment

yurydelendik Feb 27, 2015

Choose a reason for hiding this comment

fkaelberer commented Mar 1, 2015

timvandermeij commented Mar 1, 2015

pdfjsbot commented Mar 1, 2015

From: Bot.io (Linux)

Received

pdfjsbot commented Mar 1, 2015

From: Bot.io (Linux)

Success

Published

yurydelendik commented Mar 10, 2015

pdfjsbot commented Mar 10, 2015

From: Bot.io (Linux)

Received

pdfjsbot commented Mar 10, 2015

From: Bot.io (Linux)

Success

pdfjsbot commented Mar 10, 2015

From: Bot.io (Windows)

Received

pdfjsbot commented Mar 10, 2015

From: Bot.io (Windows)

Success

yurydelendik commented Mar 10, 2015