Closed Bug 920852 Opened 6 years ago Closed 6 years ago

Rename class fields to match memory reporter paths.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(4 files, 2 obsolete files)

When measuring various things in the JS engine, we try to keep the field names
the same as the paths used in the memory reporters.  Some time recently the 
paths got rearranged without the field names being changed.  This bug will fix
that.
As well as renaming the fields, I fixed a couple of minor errors in paths.
Attachment #810283 - Flags: review?(till)
This patch does the following.

- Renames RuntimeStats::gcHeapGcThings.

- Removes RuntimeStats::gcHeapUnusedGcThings -- it's dead.

- Moves the computation of rtStats->gcHeapGCThings and totalArenaSize --
  instead of incrementing them for each compartment and zone, we just compute
  them from the compartment and zone totals.

- Removes a bogus decrement of rtStats->gcHeapUnusedArenas.  This is dead code
  because it's assigned to immediately afterwards.
Attachment #810290 - Flags: review?(till)
No functional changes here.  I just put various things in a more sensible
order.
Attachment #810304 - Flags: review?(till)
This version fixes a problem -- sizeOfLiveGCThings() was wrong for ZoneStats,
because it didn't count strings that had been marked as notable.  This didn't
show up as a problem before because we only ever called sizeOfLiveGCThings()
before calling FindNotableStrings().

But now that has changed, it needs fixing, which is pretty simple.  Thank
heaven for assertions.
Attachment #810346 - Flags: review?(till)
Attachment #810290 - Attachment is obsolete: true
Attachment #810290 - Flags: review?(till)
Comment on attachment 810269 [details] [diff] [review]
(part 1) Rename fields of ObjectsExtraSizes and CompartmentStats to match memory reporter paths.

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

The new naming scheme makes a lot of sense to me.

(Also, Splinter is entirely useless for this kind of review. Proper diff tools to the rescue!)
Attachment #810269 - Flags: review?(till) → review+
Comment on attachment 810283 [details] [diff] [review]
(part 2) - Rename fields of ZoneStatsPod and StringInfo to match memory reporter paths.

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

I approve
Attachment #810283 - Flags: review?(till) → review+
Comment on attachment 810304 [details] [diff] [review]
(part 4) - Reorder a bunch of stuff.

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

nice
Attachment #810304 - Flags: review?(till) → review+
Comment on attachment 810346 [details] [diff] [review]
(part 3) - Tweak various things about RuntimeStats.

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

yep

(In this case, though: my kingdom for a content-aware diff tool!)
Attachment #810346 - Flags: review?(till) → review+
--HG--
extra : rebase_source : 0f55dd20e69d3410d0cb811529f53570f588cc5e
Attachment #8364706 - Flags: feedback?(n.nethercote)
Whiteboard: [B2G26 land after 919889]
Attachment #8364706 - Flags: feedback?(n.nethercote) → feedback+
Whiteboard: [B2G26 land after 919889]
Attachment #8364706 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.