Avoid overloading the worker-thread during eager page initialization in the viewer (PR 11263 follow-up) #14359
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…in the viewer (PR 11263 follow-up) This patch is essentially *another* continuation of PR 11263, which tried to improve loading/initialization performance of *very* large/long documents. For most documents, unless they're *very* long, we'll eagerly initialize all of the pages in the viewer. For shorter documents having all pages loaded/initialized early provides overall better performance/UX in the viewer, however there's cases where it can instead *hurt* performance. For documents with a couple of thousand pages[1], the parsing and pre-rendering of the *second* page of the document can be delayed (quite a bit). The reason for this is that we trigger `PDFDocumentProxy.getPage` for *all pages* early during the viewer initialization, which causes the worker-thread to be swamped with handling (potentially) thousands of `getPage`-calls and leaving very little time for other parsing (such as e.g. of operatorLists). To address this situation, this patch thus proposes temporarily "pausing" the eager `PDFDocumentProxy.getPage`-calls once a threshold has been reached, to give the worker-thread a change to handle other requests.[2] Obviously this may *slightly* delay the "pagesloaded" event in longer documents, but considering that it's already the result of asynchronous parsing that'll hopefully not be seen as a blocker for these changes.[3] --- [1] A particularly problematic example is https://github.com/mozilla/pdf.js/files/876321/kjv.pdf (16 MB large), which is a document with 2236 pages and a /Pages-tree that's only *one* level deep. [2] Please note that I initially considered simply chaining the `PDFDocumentProxy.getPage`-calls, however that'd slowed things down for all documents which didn't seem appropriate. [3] This patch will *hopefully* also make it possible to re-visit PR 11312, since it seems that changing `Catalog.getPageDict` to an `async` method wasn't the problem in itself. Rather it appears that it leads to slightly different timings, thus exacerbating the already existing issues with the worker-thread being overloaded by `getPage`-calls. Having recently worked with that method, there's a couple of (very old) issues that I'd also like to address and having `Catalog.getPageDict` be `async` would simplify things a great deal.
Snuffleupagus
force-pushed
the
PAUSE_EAGER_PAGE_INIT
branch
from
0c9a6c2 to
90472e5
Compare
|
/botio integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/3e2c77f0b7067e6/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/1e31515c509e783/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/1e31515c509e783/output.txt Total script time: 2.49 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/3e2c77f0b7067e6/output.txt Total script time: 5.42 mins
|
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/d23d305efb4aef5/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/d23d305efb4aef5/output.txt Total script time: 4.24 mins Published |
timvandermeij
approved these changes
Dec 11, 2021
|
Let's do this; thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch is essentially another continuation of PR #11263, which tried to improve loading/initialization performance of very large/long documents.
For most documents, unless they're very long, we'll eagerly initialize all of the pages in the viewer. For shorter documents having all pages loaded/initialized early provides overall better performance/UX in the viewer, however there's cases where it can instead hurt performance.
For documents with a couple of thousand pages[1], the parsing and pre-rendering of the second page of the document can be delayed (quite a bit). The reason for this is that we trigger
PDFDocumentProxy.getPagefor all pages early during the viewer initialization, which causes the worker-thread to be swamped with handling (potentially) thousands ofgetPage-calls and leaving very little time for other parsing (such as e.g. of operatorLists).To address this situation, this patch thus proposes temporarily "pausing" the eager
PDFDocumentProxy.getPage-calls once a threshold has been reached, to give the worker-thread a change to handle other requests.[2]Obviously this may slightly delay the "pagesloaded" event in longer documents, but considering that it's already the result of asynchronous parsing that'll hopefully not be seen as a blocker for these changes.[3]
[1] A particularly problematic example is https://github.com/mozilla/pdf.js/files/876321/kjv.pdf (16 MB large), which is a document with 2236 pages and a /Pages-tree that's only one level deep.
[2] Please note that I initially considered simply chaining the
PDFDocumentProxy.getPage-calls, however that'd slowed things down for all documents which didn't seem appropriate.[3] This patch will hopefully also make it possible to re-visit PR #11312, since it seems that changing
Catalog.getPageDictto anasyncmethod wasn't the problem in itself. Rather it appears that it leads to slightly different timings, thus exacerbating the already existing issues with the worker-thread being overloaded bygetPage-calls.Having recently worked with that method, there's a couple of (very old) issues that I'd also like to address and having
Catalog.getPageDictbeasyncwould simplify things a great deal.