809492 - Rename "Number of Constructors" to "Number of static constructors" since that's what it measures

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Joe Drew (not getting mail)]

Number of Constructors isn't what is being measured AIUI - it's number of static constructors. If this is the case, we should name the talos test to reflect what's actually being measured.

Comment 1

[Phil Ringnalda (:philor)]

If only that were our worst naming problem around it - the fact that the regression script sends email making people think that it's a talos test is a much much bigger problem.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

:froydnj, is this a valid change we want to see?

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Joel Maher (:jmaher) from comment #2)
> :froydnj, is this a valid change we want to see?

Yes.

Comment 4

[William Lachance (:wlach)]

This isn't really a release engineering issue.

It's unfortunate that we didn't think to fix this bug when moving things into perfherder. Oh well.

So we should be able to do this just by altering this one line:

http://searchfox.org/mozilla-central/source/build/util/count_ctors.py#57

And updating the docs here:

https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Automated_Performance_Testing_and_Sheriffing/Build_Metrics#num_constructors

The one disadvantage of this is that old data would still have the old name, i.e. we'd have one old series with the name "num_constructors" and one new one with the name "num_static_constructors". This is probably a worthwhile trade-off for improved clarity. Nathan, thoughts?

Comment 5

[Nathan Froyd [:froydnj]]

(In reply to William Lachance (:wlach) [ away Jan 28-> Feb 4 2017 ] from comment #4)
> The one disadvantage of this is that old data would still have the old name,
> i.e. we'd have one old series with the name "num_constructors" and one new
> one with the name "num_static_constructors". This is probably a worthwhile
> trade-off for improved clarity. Nathan, thoughts?

That sounds fine to me.

Comment 6

[William Lachance (:wlach)]

Attachment: Bug 809492 - Rename num_constructors test to num_static_constructors

Review commit: https://reviewboard.mozilla.org/r/110168/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/110168/

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8834104 [details]
Bug 809492 - Rename num_constructors test to num_static_constructors

https://reviewboard.mozilla.org/r/110168/#review111202

::: build/util/count_ctors.py:59
(Diff revision 1)
>              "suites": [{
>                  "name": "compiler_metrics",
>                  "subtests": [{
> -                    "name": "num_constructors",
> +                    "name": "num_static_constructors",
>                      "value": count_ctors(f),
>                      "alertThreshold": 0.25

can we increase this threshold to a higher number?

Comment 8

[William Lachance (:wlach)]

Comment on attachment 8834104 [details]
Bug 809492 - Rename num_constructors test to num_static_constructors

https://reviewboard.mozilla.org/r/110168/#review111206

::: build/util/count_ctors.py:59
(Diff revision 1)
>              "suites": [{
>                  "name": "compiler_metrics",
>                  "subtests": [{
> -                    "name": "num_constructors",
> +                    "name": "num_static_constructors",
>                      "value": count_ctors(f),
>                      "alertThreshold": 0.25

Hmm, yes, but I think we should talk to :nfroyd about that in the larger context of how we're going to sheriff these tests.

I'm going to drop this issue for now.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a0f8ac320e1
Rename num_constructors test to num_static_constructors r=jmaher

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/1a0f8ac320e1