Closed
Bug 721628
Opened 12 years ago
Closed 12 years ago
A slew of JS memory reporter clean-ups
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla12
People
(Reporter: n.nethercote, Assigned: n.nethercote)
Details
Attachments
(5 files)
1.39 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
6.09 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
14.73 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
48.76 KB,
patch
|
Ms2ger
:
review+
|
Details | Diff | Splinter Review |
7.84 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
This patch removes the shell's stringstats() built-in, which hasn't worked for ages.
Attachment #592038 -
Flags: review?(luke)
Assignee | ||
Comment 1•12 years ago
|
||
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)
Assignee | ||
Comment 2•12 years ago
|
||
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)
Assignee | ||
Comment 3•12 years ago
|
||
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)
Assignee | ||
Comment 4•12 years ago
|
||
Luke, we talked about exactly this in another bug recently. Here you go!
Attachment #592048 -
Flags: review?(luke)
Comment 5•12 years ago
|
||
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+
Updated•12 years ago
|
Attachment #592042 -
Flags: review?(bhackett1024) → review+
Updated•12 years ago
|
Attachment #592038 -
Flags: review?(luke) → review+
Updated•12 years ago
|
Attachment #592040 -
Flags: review?(luke) → review+
Comment 6•12 years ago
|
||
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+
Assignee | ||
Comment 7•12 years ago
|
||
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•12 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in
before you can comment on or make changes to this bug.
Description
•