Closed Bug 1465659 Opened Last year Closed Last year

Static initializer counts on windows opt builds are bi-modal

Categories

(Firefox Build System :: General, enhancement)

3 Branch
enhancement
Not set

Tracking

(firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(3 files)

That's because both opt and opt static analysis builds are sending to the same bucket.
It's kind of scary that there's a >5x difference between the two.
And the missing piece is perfherder_extra_options from the mozharness config...
The check at https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/build_lints.py#17 is supposed to detect this. Obviously there's a gap in the check :/

It would be great if the check were updated to catch this scenario so it doesn't happen again.
(In reply to David Major [:dmajor] from comment #2)
> It's kind of scary that there's a >5x difference between the two.

Not surprising, since the recent realization is that MSVC is *really* bad at it. Like, it was > 1300 a few days ago.
(In reply to Gregory Szorc [:gps] from comment #4)
> The check at
> https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/
> transforms/build_lints.py#17 is supposed to detect this. Obviously there's a
> gap in the check :/
> 
> It would be great if the check were updated to catch this scenario so it
> doesn't happen again.

There seems to be two problems:
- some perfherder extra options are set from mozharness only. For instance, no taskcluster config is setting PERFHERDER_EXTRA_OPTIONS to contain "static-analysis", yet, it's used in e.g. https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-inbound,1531342,1,2&selected=mozilla-inbound,1531342,342545,478466569
- the code added in bug 1464522 doesn't use PERFHERDER_EXTRA_OPTIONS and the lint can't test that.
That being said, it kind of seems silly that each PERFHERDER_DATA output in build logs need to set the extra options on their own. Why isn't there one output setting the extra options for all the perfherder data in the log?
I found why the build lint didn't care about PERFHERDER_EXTRA_OPTIONS not being set, and there are two reasons for this:
- not all "build" types have the build_lints transformation setup in their kind.yml
- even if they did, the build lint itself is called for each kind, but doesn't check unicity across all kinds, only the currently processed kind.
Comment on attachment 8982111 [details]
Bug 1465659 - Check perfherder options across all build kinds.

https://reviewboard.mozilla.org/r/248140/#review254430

This looks good.  As a note, there's also a validation framework that could do something similar.  However, it runs on fully-developed task definitions, so it would need to do a bunch of heuristics to find things like primary_config.  So this solution is good :)
Attachment #8982111 - Flags: review?(dustin) → review+
Comment on attachment 8982110 [details]
Bug 1465659 - Take PERFHERDER_EXTRA_OPTIONS into account for static initializer count report.

https://reviewboard.mozilla.org/r/248138/#review254446

Yes please!
Attachment #8982110 - Flags: review+
Comment on attachment 8982112 [details]
Bug 1465659 - Move perfherder extra options from mozharness to taskcluster.

https://reviewboard.mozilla.org/r/248142/#review254448

I didn't fact-check this but it looks like what I expected.
Attachment #8982112 - Flags: review+
Attachment #8982110 - Flags: review?(core-build-config-reviews)
Attachment #8982112 - Flags: review?(core-build-config-reviews)
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/e2087bd3daee
Take PERFHERDER_EXTRA_OPTIONS into account for static initializer count report. r=nalexander
https://hg.mozilla.org/integration/autoland/rev/5fc13d0a68ad
Check perfherder options across all build kinds. r=dustin
https://hg.mozilla.org/integration/autoland/rev/dfb4c974e3b0
Move perfherder extra options from mozharness to taskcluster. r=nalexander
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.