721628 - A slew of JS memory reporter clean-ups

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch 1: remove stringstats()

This patch removes the shell's stringstats() built-in, which hasn't worked for ages.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: patch 2: remove slop descriptions

The JS engine memory reporters used to be notable because they measured slop bytes.  But now that's commonplace, so there's no point bloating the descriptions.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: patch 3: type inference renaming, etc

This patch:

- Renames a bunch of things to use more (now) standard names.  This includes
  changing TypeSet::dynamicSize() and TypeObject::dynamicSize() both to 
  computedSizeOfExcludingThis()... I wasn't sure if the "ExcludingThis" part
  was appropriate, I still don't entirely grok the accounting of the 
  temporary space.

- Moves some declarations that no longer need to be in MemoryMetrics.h 
  elsewhere.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: patch 4: rename IterateData and others

This patch renames a bunch of things:

- |IterateData data| --> |RuntimeStats rtStats|

- |JS::CollectCompartmentStatsForRuntime| --> |JS::CollectRuntimeStats|

- |CompartmentStats &curr|  --> |CompartmentStats &cStats|

- |CompartmentStats &stats| --> |CompartmentStats &cStats|

And a few other things like that.  I came up with the original names and 
have been confusing myself with them ever since (especially |IterateData|, 
geez).  The new ones are much better!

Ms2ger, I don't think you're a JS peer but I don't care, you're the best 
person to review this, having mucked around with this code a lot recently 
yourself.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: patch 5: use size_t instead of int64_t in memory reporter structs

Luke, we talked about exactly this in another bug recently.  Here you go!

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 592046 [details] [diff] [review]
patch 4: rename IterateData and others

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

Sure.

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 592048 [details] [diff] [review]
patch 5: use size_t instead of int64_t in memory reporter structs

Sweet

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c6aad97a8288
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7cc6d203ced
https://hg.mozilla.org/integration/mozilla-inbound/rev/19b63ce08d27
https://hg.mozilla.org/integration/mozilla-inbound/rev/829732925bfa
https://hg.mozilla.org/integration/mozilla-inbound/rev/27f749dda6fd

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c6aad97a8288
https://hg.mozilla.org/mozilla-central/rev/c7cc6d203ced
https://hg.mozilla.org/mozilla-central/rev/19b63ce08d27
https://hg.mozilla.org/mozilla-central/rev/829732925bfa
https://hg.mozilla.org/mozilla-central/rev/27f749dda6fd