-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Cache JPEG images, just as we do for other image formats, in evaluator.js
(issue 8380)
#8388
Conversation
…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.
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/db82c90fbe08a1f/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/4b3710c80d047a4/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/db82c90fbe08a1f/output.txt Total script time: 16.68 mins
Image differences available at: http://54.67.70.0:8877/db82c90fbe08a1f/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/4b3710c80d047a4/output.txt Total script time: 25.81 mins
|
One thing I forgot to mention in #8388 (comment), is that if you disable the What this patch does is simply to ensure that we also cache JPEG images, as |
/botio makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @yurydelendik received. Current queue size: 0 Live output at: http://54.67.70.0:8877/9e82d0040872c3f/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @yurydelendik received. Current queue size: 1 Live output at: http://54.215.176.217:8877/4d4869fa3fd3560/output.txt |
Thank you for the patch. |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/4d4869fa3fd3560/output.txt Total script time: 24.51 mins
|
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/9e82d0040872c3f/output.txt Total script time: 60.00 mins
|
/botio-linux makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://54.67.70.0:8877/2d422734914b925/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/2d422734914b925/output.txt Total script time: 0.09 mins |
/botio-linux makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://54.67.70.0:8877/21c62528934c340/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/21c62528934c340/output.txt Total script time: 0.08 mins |
/botio-linux makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://54.67.70.0:8877/44014aed63cb13a/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/44014aed63cb13a/output.txt Total script time: 0.09 mins |
Sorry; I've just restored the branch, let's try again. /botio-linux makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/bcd4d9e7d856ef9/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/bcd4d9e7d856ef9/output.txt Total script time: 23.50 mins
|
Cache JPEG images, just as we do for other image formats, in `evaluator.js` (issue 8380)
For some reason, we're putting all kind of images except JPEG into the
imageCache
inevaluator.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.