Avoid unnecessary array allocations in EvaluatorPreprocessor_read(). #5168
Conversation
|
looking at it, this feels a bit too magic for me. maybe this could be made a bit more explicit? |
|
I added nine lines of new comments explaining it. I'm not sure what else I can do to make it less magic. |
| // - |fn| is an outparam. | ||
| // - |args| is an inoutparam. This allows the caller to reuse a single | ||
| // array for all calls to this function (and there are many) if | ||
| // appropriate. | ||
| read: function EvaluatorPreprocessor_read(operation) { | ||
| // We use an array to represent args, except we use |null| to represent |
There was a problem hiding this comment.
does this comment still make sense?
|
I added a few inline comments which hopefully make it more clear why i think the behaviour is magic for me. |
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/2d05ab5e86b5803/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/6a9c4f06e25c2e4/output.txt |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/2d05ab5e86b5803/output.txt Total script time: 2.83 mins
Image differences available at: http://107.22.172.223:8877/2d05ab5e86b5803/reftest-analyzer.html#web=eq.log |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/6a9c4f06e25c2e4/output.txt Total script time: 22.27 mins
|
|
/botio-windows test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/92fd7e5389f2dca/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/92fd7e5389f2dca/output.txt Total script time: 19.78 mins
|
EvaluatorPreprocessor_read() is called in two cases. For the normal layer, the args array it produces is used beyond the bounds of the loop in which EvaluatorPreprocessor_read() is called. But for the text layer, the args array is used in a very short-term fashion. This change reworks things so that a single array is repeatedly used for the text layer. This reduces total JS allocations for the Spoorkaart map by 11%, and has similar effects on many other PDFs.
|
I've expanded the comments. I think the comment above EvaluatorPreprocessor_read() is now the largest single comment in the entire codebase. |
|
I like it better now |
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/a44f92019604e41/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/a44f92019604e41/output.txt Total script time: 0.73 mins Published
|
|
/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/d922c1fb619374a/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/12c0f2e2d22f5ab/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/12c0f2e2d22f5ab/output.txt Total script time: 19.95 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/d922c1fb619374a/output.txt Total script time: 22.35 mins
|
Avoid unnecessary array allocations in EvaluatorPreprocessor_read().
|
Thanks for the patch! |
EvaluatorPreprocessor_read() is called in two cases. For the normal
layer, the args array it produces is used beyond the bounds of the loop
in which EvaluatorPreprocessor_read() is called.
But for the text layer, the args array is used in a very short-term
fashion. This change reworks things so that a single array is repeatedly
used for the text layer. This reduces total JS allocations for the
Spoorkaart map by 11%, and has similar effects on many other PDFs.