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

Attempt to reduce resource usage, by not eagerly fetching all pages, for long/large documents #11263

Merged

Conversation

Snuffleupagus
Copy link
Collaborator

For very long/large documents fetching all pages on load may cause quite bad performance, both memory and CPU wise. In order to at least slightly alleviate this, we can let the viewer treat these kind of documents[1] as if disableAutoFetch were set.


[1] One example of a really bad case is https://bugzilla.mozilla.org/show_bug.cgi?id=1588435, which this patch should at least help somewhat. In general, for these cases, we'd probably need to implement switching between PDFViewer/PDFSinglePageViewer (as already tracked on GitHub) and use the latter for these kind of long documents.

…for long/large documents

For *very* long/large documents fetching all pages on load may cause quite bad performance, both memory and CPU wise. In order to at least slightly alleviate this, we can let the viewer treat these kind of documents[1] as if `disableAutoFetch` were set.

---
[1] One example of a really bad case is https://bugzilla.mozilla.org/show_bug.cgi?id=1588435, which this patch should at least help somewhat. In general, for these cases, we'd probably need to implement switching between `PDFViewer`/`PDFSinglePageViewer` (as already tracked on GitHub) and use the latter for these kind of long documents.
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/d45cea9fb8c89e6/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/d45cea9fb8c89e6/output.txt

Total script time: 1.73 mins

Published

@timvandermeij timvandermeij merged commit d7f651a into mozilla:master Oct 20, 2019
@timvandermeij
Copy link
Contributor

timvandermeij commented Oct 20, 2019

Thank you! This PDF file is horribly large, so I'm not surprised we're having trouble rendering this properly. I do notice that this patch improves the situation a bit; it's still slow, but skipping to some pages at least makes them render better now.

I noticed that PDF.js doesn't render more than 12656 pages. The last canvas is cut off at the top, so that's also "interesting" about this PDF file... In short, it's good that the issue on Bugzilla is still open.

@Snuffleupagus
Copy link
Collaborator Author

Thanks for landing this!

[...] it's still slow, but skipping to some pages at least makes them render better now.

Part of the slowness seems to be related to the file size itself (at 185 MB) and particularly the connection of the server, since having the file available locally also seem to help a bit.

I noticed that PDF.js doesn't render more than 12656 pages.

If I were to guess, that's probably the browser choking on the sheer number of DOM elements, rather than anything else. (With PDFSinglePageViewer there's no problem accessing later pages.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants