mozilla / pdf.js

PDF Reader in JavaScript
https://mozilla.github.io/pdf.js/
Apache License 2.0
48.77k stars 10.03k forks source link

Memory issue when streaming a PDF #5400

Closed automatedbugreportingfacility closed 10 years ago

automatedbugreportingfacility commented 10 years ago

This is a regression. Loading the following pdf causes the browser to consume large amounts of memory (>2 GB here) until it hits OOM. Please note that:

1) It won't reproduce locally, it must be loaded from the URL. 2) The pdf file must not be in cache, ie. clear it (Ctrl + Shift + Delete) if the problem stops reproducing.

URL: http://www.gergs-blum.de/Mensa%20KCG/Bauen%20mit%20Holz%205%202014.pdf

c3f191a27cb14e23d8129cd10ec58d7709ecc51b is the first bad commit commit c3f191a27cb14e23d8129cd10ec58d7709ecc51b Author: Yury Delendik Date: Fri Sep 5 20:02:54 2014 -0500

Implement streaming using moz-chunk-arraybuffer
automatedbugreportingfacility commented 10 years ago

By the way, this is not a dupe of #5329, at least the patches from PR #5332 don't seem to resolve this issue.

Snuffleupagus commented 10 years ago

Loading the following pdf causes the browser to consume large amounts of memory (>2 GB here) until it hits OOM.

With streaming enabled, I can reproduce the large memory consumption (over 2 GB), and very intermittently I'm also getting OOM errors. However, with streaming disabled, the memory consumption is a lot lower (about 500 MB), so streaming apparently requires a lot more memory.

/cc @yurydelendik Is there anything (simple) we can do to reduce the memory consumption of streaming?

Snuffleupagus commented 10 years ago

The reason for the difference between streaming/non-streaming is probably related to the differences between getData (PdfStreamConverter.jsm#L209) and readData (PdfStreamConverter.jsm#L204). In the first case we delete this.data, which we cannot do in latter case, and this should explain why the memory consumption is so different.

automatedbugreportingfacility commented 10 years ago

Right, adding "delete this.data;" before "return data;" fixes the issue for me.

Snuffleupagus commented 10 years ago

Right, adding "delete this.data;" before "return data;" fixes the issue for me.

As I alluded to above, we cannot do that since it breaks streaming (it would be functionally equivalent to disableStream=true), so this is unfortunately not a solution.