1304722 - 4.76 - 7.02% compiler_metrics num_constructors (linux32, linux64) regression on push 29f80f1769fc66ca5cc390183f3b49eec160ad4f (Wed Sep 21 2016)

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[William Lachance (:wlach)]

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

[Matt Woodrow (:mattwoodrow)]

Nathan, any ideas on this?

I'm pretty sure I only added StaticRefPtrs, which shouldn't have constructors.

Comment 2

[Michelle Funches - QA]

Hi Guys, can you please place this in a component group, something other than Untriaged, thanks.

Comment 3

[Andrew McCreight [:mccr8]]

I think it makes sense to move it to the same component as the bug that caused the regression.

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: don't define nsCString constants in VideoUtils.h

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.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19c62c33ce97
don't define nsCString constants in VideoUtils.h; r=mattwoodrow

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/19c62c33ce97