1073594 - Remove the mean and standard deviation from the BloatView table

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

Are these really useful for anything?  Right now it just clutters up the table as far as I can tell.

The leak test harness code in automationutils.py may need to be fixed.  There's an include of some kind of stddev header in XPCOMInit.cpp that may be removable along with this.

Comment 1

[Andrew McCreight [:mccr8]]

dbaron, is it okay with you if the mean and stddev are removed from BloatView?

Comment 2

[David Baron :dbaron:]

Absolutely.  They're meaningless.

Just need to make sure there aren't other pieces of code that depend on them being there for gathering statistics (e.g., the code that summarizes leak stats for a test run that you point out, which we already know uses some regex's that aren't completely right, bug 1045350).

Comment 3

[Nicholas Nethercote [inactive]]

+1

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: part 1 - Remove mean and stddev for BloatView addrefs and releases.

nfroyd is out this week, so maybe you can review, dbaron.

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=40493ce7e99b
https://treeherder.mozilla.org/#/jobs?repo=try&revision=62cb20f5fb98

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: part 2 - Remove nsTraceRefcntStats::mRefsOutstanding{Total, Squared}.

They are now unused.

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: part 3 - Remove the mean and stddev for BloatLog object counts.

Comment 7

[Andrew McCreight [:mccr8]]

Attachment: part 4 - Remove nsTraceRefcntStats::mObjsOutstanding{Total, Squared}.

They are now unused.

Comment 8

[Andrew McCreight [:mccr8]]

I checked that the BloatView parser works, plus I looked at the code, which is totally agnostic to all this stuff at the end.

       |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
                                                Per-Inst   Leaked    Total      Rem
     0 TOTAL                                          40     4304    43692       33
     7 AsyncTransactionTrackersHolder                 72       72        2        1
    38 CompositorChild                               808      808        1        1
    39 CondVar                                        40       80       10        2
   142 MessagePump                                    16       16        9        1
   144 Mutex                                          32       96       88        3

Here's what the output looks like.  Well, also with bug 1138616 applied.  But I did check that the intermediate states look reasonable.

Comment 9

[Andrew McCreight [:mccr8]]

Thanks for the reviews.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/9f6d6c54a10d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/25e5a9346a6a
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ac6e3fccc155
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/505dc97789f0

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9f6d6c54a10d
https://hg.mozilla.org/mozilla-central/rev/25e5a9346a6a
https://hg.mozilla.org/mozilla-central/rev/ac6e3fccc155
https://hg.mozilla.org/mozilla-central/rev/505dc97789f0

Comment 11

[Andrew McCreight [:mccr8]]

Comment on attachment 8572022 [details] [diff] [review]
part 4 - Remove nsTraceRefcntStats::mObjsOutstanding{Total, Squared}.

This approval request is for all four of the patches in this bug.

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: This is needed for bug 1138616.
[Describe test coverage new/current, TreeHerder]: This code is heavily tested on TreeHerder in debug builds.
[Risks and why]: Low. This only affects debug builds, and has been on trunk for four days.
[String/UUID change made/needed]: none

Comment 12

[Andrew McCreight [:mccr8]]

Comment on attachment 8572022 [details] [diff] [review]
part 4 - Remove nsTraceRefcntStats::mObjsOutstanding{Total, Squared}.

Cancelling for now due to regression in the bug this blocks.

Comment 13

[Andrew McCreight [:mccr8]]

Comment on attachment 8572022 [details] [diff] [review]
part 4 - Remove nsTraceRefcntStats::mObjsOutstanding{Total, Squared}.

This approval request is for all of the patches in this bug.

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: Needed for 1138616
[Describe test coverage new/current, TreeHerder]: This code runs very frequently in debug builds.
[Risks and why]: Low. This code only runs in debug builds, and there are no known issues.
[String/UUID change made/needed]: none

Comment 14

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8572022 [details] [diff] [review]
part 4 - Remove nsTraceRefcntStats::mObjsOutstanding{Total, Squared}.

Approving for uplift to aurora since this looks low risk, it's not too late in the aurora cycle, and this is for debug build tests.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1abea956f06f
https://hg.mozilla.org/releases/mozilla-aurora/rev/ae154f933e65
https://hg.mozilla.org/releases/mozilla-aurora/rev/d50b1d697c75
https://hg.mozilla.org/releases/mozilla-aurora/rev/c2ab66bc0fdf