Closed Bug 1073594 Opened 6 years ago Closed 5 years ago

Remove the mean and standard deviation from the BloatView table

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(4 files)

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.
dbaron, is it okay with you if the mean and stddev are removed from BloatView?
Flags: needinfo?(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).
Flags: needinfo?(dbaron)
Blocks: 1138616
They are now unused.
Attachment #8572020 - Flags: review?(dbaron)
They are now unused.
Attachment #8572022 - Flags: review?(dbaron)
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.
Attachment #8572019 - Flags: review?(dbaron) → review+
Attachment #8572020 - Flags: review?(dbaron) → review+
Attachment #8572021 - Flags: review?(dbaron) → review+
Attachment #8572022 - Flags: review?(dbaron) → review+
Assignee: nobody → continuation
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
Attachment #8572022 - Flags: approval-mozilla-beta?
Attachment #8572022 - Flags: approval-mozilla-aurora?
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.
Attachment #8572022 - Flags: approval-mozilla-beta?
Attachment #8572022 - Flags: approval-mozilla-aurora?
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
Attachment #8572022 - Flags: approval-mozilla-aurora?
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.
Attachment #8572022 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.