1180123 - Fix minor problems with the JS memory reporter

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

I noticed a couple of minor problems with the JS memory reporter.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Report the size of the saved stacks sets

Bug 972045 added measurement of the saved stacks set, but failed to actually
report those measurements so that they show up in about:memory.

This patch remedies that. Better suggestions for the description sentence are
welcome.

BTW, I get 322,048 bytes for this measurement (all compartments combined) in
the main runtime just after start-up with just one tab open. That seems kind of
high; I expected this would be a devtools only kind of thing. Is this expected?

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Minor nursery report fixes

Comment 3

[Terrence Cole [:terrence]]

Comment on attachment 8629254 [details] [diff] [review]
(part 2) - Minor nursery report fixes

Review of attachment 8629254 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8629249 [details] [diff] [review]
(part 1) - Report the size of the saved stacks sets

Review of attachment 8629249 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Nicholas Nethercote [:njn] from comment #1)
> Created attachment 8629249 [details] [diff] [review]
> (part 1) - Report the size of the saved stacks sets
> 
> Bug 972045 added measurement of the saved stacks set, but failed to actually
> report those measurements so that they show up in about:memory.

Thanks!

> BTW, I get 322,048 bytes for this measurement (all compartments combined) in
> the main runtime just after start-up with just one tab open. That seems kind
> of
> high; I expected this would be a devtools only kind of thing. Is this
> expected?

The SavedStacks machinery is used in almost every case where we want to capture a JS stack these days. That includes DOMExceptions and JS Error objects which are not devtools related. Overall, this should be a net win for reducing memory consumption (even though stacks' memory consumption is more "visible" than before when they were naive JS strings) because SavedStacks does tail sharing of older frames. See bug 1125259 for one example.

What tab do you have open? That number sounds a bit high if it was about:blank, but if it is cnn.com, maybe not so much.

Comment 5

[Nicholas Nethercote [inactive]]

> The SavedStacks machinery is used in almost every case where we want to
> capture a JS stack these days. That includes DOMExceptions and JS Error
> objects which are not devtools related. Overall, this should be a net win
> for reducing memory consumption (even though stacks' memory consumption is
> more "visible" than before when they were naive JS strings) because
> SavedStacks does tail sharing of older frames. See bug 1125259 for one
> example.

Makes sense. Thank you for the explanation.
 
> What tab do you have open? That number sounds a bit high if it was
> about:blank, but if it is cnn.com, maybe not so much.

Good guess! :)  I had cnn.com open, plus about:memory to do the actual measurement.

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Ha! cnn.com is a great test site :)

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/543e06741a9c
https://hg.mozilla.org/integration/mozilla-inbound/rev/50e8abbb4a2c

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/543e06741a9c
https://hg.mozilla.org/mozilla-central/rev/50e8abbb4a2c