Simplify singularValueDecompose2dScale in order to make it using less memory #19721
Conversation
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/6c0e683d8e5a4a6/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/6c0e683d8e5a4a6/output.txt Total script time: 30.55 mins
Image differences available at: http://54.241.84.105:8877/6c0e683d8e5a4a6/reftest-analyzer.html#web=eq.log |
calixteman
force-pushed
the
simplif_singular_decomposition
branch
from
011c05f to
73a62ee
Compare
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/a920653da594e64/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/dbd0ded44b09b27/output.txt |
calixteman
force-pushed
the
simplif_singular_decomposition
branch
from
73a62ee to
87b8ba5
Compare
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/a920653da594e64/output.txt Total script time: 29.81 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/dbd0ded44b09b27/output.txt Total script time: 60.00 mins
|
|
Given that all invocations of this method now use the same TypedArray, it's quite easy to accidentally introduce subtle bugs because of it (as seen in even the first version of this patch). Obviously the memory reduction is important here, but from a developer and reviewer perspective it seems that we'll now need to be very careful to not accidentally "mess up" when using this. Obviously it's not used in lots of places, but it does feel like a somewhat brittle situation and I wonder if there's anything that can be done to reduce the risk of future bugs? |
I'm obliged to agree with you mainly because I had to fix such a bug while writing this patch. |
Is However, for the cases in |
calixteman
force-pushed
the
simplif_singular_decomposition
branch
from
87b8ba5 to
f70bd4c
Compare
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/5a1dbc72ca933b6/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/7927c61f61ca00d/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/5a1dbc72ca933b6/output.txt Total script time: 30.54 mins
Image differences available at: http://54.241.84.105:8877/5a1dbc72ca933b6/reftest-analyzer.html#web=eq.log |
calixteman
force-pushed
the
simplif_singular_decomposition
branch
from
f70bd4c to
a1b1ed0
Compare
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/7927c61f61ca00d/output.txt Total script time: 60.00 mins |
… memory In using the Firefox profiler (with JS allocations tracking) and wuppertal.pdf, I noticed we were using a bit too much memory for a function which is supposed to just compute 2 numbers. The memory used by itself isn't so important but having a too much objects lead to waste some time to gc them. So this patch aims to simplify it a bit.
calixteman
force-pushed
the
simplif_singular_decomposition
branch
from
a1b1ed0 to
6e9fbd9
Compare
|
/botio browsertest |
From: Bot.io (Windows)ReceivedCommand cmd_browsertest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/ea6962279be1780/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_browsertest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/1e9f57d7b805d1a/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/1e9f57d7b805d1a/output.txt Total script time: 16.13 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/ea6962279be1780/output.txt Total script time: 29.74 mins
|
In using the Firefox profiler (with JS allocations tracking) and wuppertal.pdf, I noticed we were using a bit too much memory for a function which is supposed to just compute 2 numbers. The memory used by itself isn't so important but having a too much objects lead to waste some time to gc them.
So this patch aims to simplify it a bit.