about:memory report formatting is slow
Categories
(Toolkit :: about:memory, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: heycam, Assigned: heycam)
Details
Attachments
(1 file)
Loading a large memory report in about:memory is slow. For example when I load a memory report file with around 12,000 reports in it, the main thread is blocked for around 2 s. Profiling reveals that more than half of the time is spent in Number.toLocaleString. That's unfortunate, since it's a convenient API, but reimplementing just what we need helps a lot. With the forthcoming patch, the time spent under formatNum on this file I was testing with drops from 1200 ms to 10 ms. (Making report formatting faster is important for bug 1517175, where my patch will reformat upon typing in a new filter string.)
Assignee | ||
Comment 1•4 years ago
|
||
Assignee | ||
Comment 2•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c4106e777ab7bf0dcf7d7eff0590c9b4e8e106b
![]() |
||
Comment 3•4 years ago
|
||
Aw geez, I just got rid of the old hand-rolled formatting code in bug 1499906 :( The fact that the JS code in the patch, which doesn't look particularly fast, is nonetheless vastly faster than toLocaleString() suggests that the implementation of toLocaleString() has perf problems. Jason, do you have any thoughts about that?
Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a52b66841df9 Improve about:memory performance by not using toLocaleString r=njn
Comment 5•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a52b66841df9
Comment 6•4 years ago
|
||
Thanks for the needinfo on this. By coincidence, I recently filed a bug about this. Bug 1514851.
Comment 7•4 years ago
|
||
toLocaleString will recompute/recreate an ICU formatting object every time you call it. The intended way users are expected to adapt to that, is to create a formatting object once, then use that repeatedly. We could perhaps add caching or something for commonly-specified sets of options, or a one-element LRU cache, or similar, but that would slow down every user for the few that don't realize they should be creating a formatter object.
But it looks near-trivial to make this use formatter objects instead of toLocaleString, so we should just do that. I'll file a bug to do that, with an untested patch I've just churned out locally that hopefully someone else can push over the line.
Comment 8•4 years ago
|
||
Filed bug 1518263 to use formatter objects for this. The way formatNum
is used here is exactly amenable to using cached formatter objects.
Description
•