Closed Bug 721628 Opened 8 years ago Closed 8 years ago

A slew of JS memory reporter clean-ups

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: njn, Assigned: njn)

Details

Attachments

(5 files)

This patch removes the shell's stringstats() built-in, which hasn't worked for ages.
Attachment #592038 - Flags: review?(luke)
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.
Attachment #592040 - Flags: review?(luke)
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.
Attachment #592042 - Flags: review?(bhackett1024)
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.
Attachment #592046 - Flags: review?(Ms2ger)
Luke, we talked about exactly this in another bug recently.  Here you go!
Attachment #592048 - Flags: review?(luke)
Comment on attachment 592046 [details] [diff] [review]
patch 4: rename IterateData and others

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

Sure.
Attachment #592046 - Flags: review?(Ms2ger) → review+
Attachment #592042 - Flags: review?(bhackett1024) → review+
Attachment #592038 - Flags: review?(luke) → review+
Attachment #592040 - Flags: review?(luke) → review+
Comment on attachment 592048 [details] [diff] [review]
patch 5: use size_t instead of int64_t in memory reporter structs

Sweet
Attachment #592048 - Flags: review?(luke) → review+
You need to log in before you can comment on or make changes to this bug.