Small optimizations 1 #4895
Small optimizations 1 #4895
Conversation
| @@ -635,7 +635,7 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { | |||
| assert(isName(type), | |||
| 'XObject should have a Name subtype'); | |||
|
|
|||
| if (type.name === 'Form') { | |||
| if ('Form' === type.name) { | |||
There was a problem hiding this comment.
What's the reason for this (and the next) change? I don't see how that would be more efficient, but if it is shouldn't this line be changed too: https://github.com/mozilla/pdf.js/blob/master/src/core/evaluator.js#L652?
There was a problem hiding this comment.
No reason. This is a unhappy merge conflict resolution. I will revert the order of the equality check.
|
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/d0aac24e67c4938/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/3b1f17287d9b310/output.txt |
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/d0aac24e67c4938/output.txt Total script time: 3.74 mins
Image differences available at: http://107.21.233.14:8877/d0aac24e67c4938/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/3b1f17287d9b310/output.txt Total script time: 3.82 mins
Image differences available at: http://107.22.172.223:8877/3b1f17287d9b310/reftest-analyzer.html#web=eq.log |
|
Running the tests locally, this PR timeouts at the same point as it does on the bots. |
| @@ -947,7 +947,7 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { | |||
| } | |||
|
|
|||
| var glyphUnicode = glyph.unicode; | |||
| if (glyphUnicode in NormalizedUnicodes) { | |||
| if (NormalizedUnicodes[glyphUnicode] === undefined) { | |||
There was a problem hiding this comment.
Probable typo: shouldn't it be !== instead?
|
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/16099fc3ef1cb41/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/4c9c364c6e7640c/output.txt |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/4c9c364c6e7640c/output.txt Total script time: 23.07 mins
Image differences available at: http://107.22.172.223:8877/4c9c364c6e7640c/reftest-analyzer.html#web=eq.log |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/16099fc3ef1cb41/output.txt Total script time: 26.09 mins
|
| @@ -394,6 +394,8 @@ var CanvasGraphics = (function CanvasGraphicsClosure() { | |||
| // Defines the time the executeOperatorList is going to be executing | |||
| // before it stops and shedules a continue of execution. | |||
| var EXECUTION_TIME = 15; | |||
| // Defines the number of steps before checking the execution time | |||
| var EXECUTION_STEPS = 100; | |||
There was a problem hiding this comment.
We shall not batch long running operations such as paint image, shading pattern or smask compositing (on restore) -- scrolling will suffer and "long running script" dialog will pop-up. Please revert this change.
There was a problem hiding this comment.
This is essentially what the EXECUTION_TIME constant does.
But you're right.
I checked how many operations were processed within the EXECUTION_TIME across several PDFs and saw that the very minimum number was around 4-500 on my machine, and up to 30.000+. That's a lot of calls to Date.now() for nothing ... except I overlooked that my setup is pretty decent and this number of operations could well go as low as 40-50 on another machine.
How do you feel about using var EXECUTION_STEPS = 10; ?
I will run some tests to see the "cost" of the operations when batched by 10 to see whether the possibility of a "long running script" dialog is serious or that small batches contain at most one expensive operation and all the other ones are cheap.
There was a problem hiding this comment.
How do you feel about using var EXECUTION_STEPS = 10; ?
Yep, we can do that. There will not be enough operators to e.g. create and then apply smask composition multiple times.
Declaring the composition and backgdrop functions outside of genericComposeSMask is more efficient.
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 3 Live output at: http://107.22.172.223:8877/080fe71c23dfbc6/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 3 Live output at: http://107.21.233.14:8877/7d32796f6023c2b/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/080fe71c23dfbc6/output.txt Total script time: 21.10 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/7d32796f6023c2b/output.txt Total script time: 26.22 mins
|
|
Thank you for the pull request |
A couple of small optimizations.