Closed
Bug 1304722
Opened 8 years ago
Closed 8 years ago
4.76 - 7.02% compiler_metrics num_constructors (linux32, linux64) regression on push 29f80f1769fc66ca5cc390183f3b49eec160ad4f (Wed Sep 21 2016)
Categories
(Core :: Audio/Video: Playback, defect, P1)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: wlach, Assigned: froydnj)
References
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
patch
|
jwwang
:
review+
|
Details | Diff | Splinter Review |
Hi Matt, I am filing this bug because it seems like a large increase in the # of constructors. I don't know much about this metric so please consult :nfroyd if you have any questions.
--
We have detected a build metrics regression from push 29f80f1769fc66ca5cc390183f3b49eec160ad4f. As author of one of the patches included in that push, we need your help to address this regression.
Regressions:
7% compiler_metrics num_constructors linux64 pgo 171 -> 183
7% compiler_metrics num_constructors linux32 opt 171 -> 183
5% compiler_metrics num_constructors linux32 debug 231 -> 242
5% compiler_metrics num_constructors linux64 debug 231 -> 242
You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=3327
On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.
To learn more about the regressing test(s), please see: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Automated_Performance_Testing_and_Sheriffing/Build_Metrics
Comment 1•8 years ago
|
||
Nathan, any ideas on this?
I'm pretty sure I only added StaticRefPtrs, which shouldn't have constructors.
Comment 2•8 years ago
|
||
Hi Guys, can you please place this in a component group, something other than Untriaged, thanks.
Comment 3•8 years ago
|
||
I think it makes sense to move it to the same component as the bug that caused the regression.
Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core
Updated•8 years ago
|
Priority: -- → P1
Assignee | ||
Comment 4•8 years ago
|
||
Turns out you weren't responsible, though I think sImageMap might have been
part of a static constructor:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b6401bcb115#l15.49
You were just including VideoUtils.h in more places, and since that helpfully
defines string constants in every compilation unit that includes it, you were
injecting static constructors in more places.
Let's stop doing that.
Attachment #8797285 -
Flags: review?(matt.woodrow)
Comment 5•8 years ago
|
||
Comment on attachment 8797285 [details] [diff] [review]
don't define nsCString constants in VideoUtils.h
Review of attachment 8797285 [details] [diff] [review]:
-----------------------------------------------------------------
I think it makes sense for JW to review this.
Attachment #8797285 -
Flags: review?(matt.woodrow) → review?(jwwang)
Updated•8 years ago
|
Attachment #8797285 -
Flags: review?(jwwang) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19c62c33ce97
don't define nsCString constants in VideoUtils.h; r=mattwoodrow
Comment 7•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•8 years ago
|
Assignee: nobody → nfroyd
Version: unspecified → Trunk
You need to log in
before you can comment on or make changes to this bug.
Description
•