Skip to content

Commit

Permalink
Cache JPEG images, just as we do for other image formats, in `evaluat…
Browse files Browse the repository at this point in the history
…or.js` (issue 8380)

For some reason, we're putting all kind of images *except* JPEG into the `imageCache` in `evaluator.js`.[1]
This means that in the PDF file in issue 8380, we'll keep sending the *same* two small images[2] to the main-thread and decoding them over and over. This is obviously hugely inefficient!

As can be seen from the discussion in the issue, the performance becomes *extremely* bad if the user has the addon "Adblock Plus" installed. However, even in a clean Firefox profile, the performance isn't that great.

This patch not only addresses the performance implications of the "Adblock Plus" addon together with that particular PDF file, but it *also* improves the rendering times considerably for *all* users.
Locally, with a clean profile, the rendering times are reduced from `~2000 ms` to `~500 ms` for my setup!

Obviously, the general structure of the PDF file and its operator sequence is still hugely inefficient, however I'd say that the performance with this patch is good enough to consider the issue (as it stands) resolved.[3]

Fixes 8380.

---
[1] Not technically true, since inline images are cached from `parser.js`, but whatever :-)

[2] The two JPEG images have dimensions 1x2, respectively 4x2.

[3] To make this even more efficient, a new state would have to be added to the `QueueOptimizer`. Given that PDF files this stupid fortunately aren't too common, I'm not convinced that it's worth doing.
  • Loading branch information
Snuffleupagus committed May 7, 2017
1 parent 50d026f commit 0c2ebda
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 3 deletions.
11 changes: 8 additions & 3 deletions src/core/evaluator.js
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,14 @@ var PartialEvaluator = (function PartialEvaluatorClosure() {
NativeImageDecoder.isSupported(image, this.xref, resources)) {
// These JPEGs don't need any more processing so we can just send it.
operatorList.addOp(OPS.paintJpegXObject, args);
this.handler.send('obj',
[objId, this.pageIndex, 'JpegStream',
image.getIR(this.options.forceDataSchema)]);
this.handler.send('obj', [objId, this.pageIndex, 'JpegStream',
image.getIR(this.options.forceDataSchema)]);
if (cacheKey) {
imageCache[cacheKey] = {
fn: OPS.paintJpegXObject,
args,
};
}
return;
}

Expand Down
1 change: 1 addition & 0 deletions test/pdfs/issue8380.pdf.link
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
https://web.archive.org/web/20170507102908/https://www.mbank.pl/download/firma/Dyspozycja-zmiany-typu-rachunku-biecego.pdf?noredir
7 changes: 7 additions & 0 deletions test/test_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,13 @@
"rounds": 1,
"type": "eq"
},
{ "id": "issue8380",
"file": "pdfs/issue8380.pdf",
"md5": "2782af6a4d0540fcea3897560f842094",
"rounds": 1,
"link": true,
"type": "eq"
},
{ "id": "type4psfunc",
"file": "pdfs/type4psfunc.pdf",
"md5": "7e6027a02ff78577f74dccdf84e37189",
Expand Down

0 comments on commit 0c2ebda

Please sign in to comment.