1087100 - mozharness mach nightly builds are not running with pgo on twig branches

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Comment 1

[Jordan Lund (:jlund)]

Attachment: 141023_twigs_not_running_pgo_for_nightlies.patch

essentially the logic (incorrectly) was
    if platform is a pgo-able platform *and* branch does actual pgo builds
        compile with pgo for nightlies

where is should just be
    if platform is a pgo able platform:
        compile with pgo for nightlies

So this code wasn't doing anything anyways since we only have branches that are enabled that either do pgo builds or nightlies, not both. 

I'm leaving it out because this led me to realize that once we get to release branches, the logic gets more complicated since we will have this 'per-checkin' strategy for opt builds to deal with too.

Instead, I'm going configure things more in mozharness. This is because:

1) I'd prefer buildbot just tell mozharness when the build is an actual PGO build (there is PGO in the buildername). Then, in mozharness, configure the edge cases of when we still want to compile with PGO even when it is not a PGO classified build.

2) the way things are now, mozharness already has to figure out if we passed --pgo-build because it's a classified PGO or because it is a nightly.

accompanying mozharness patch incoming

Comment 2

[Jordan Lund (:jlund)]

sent a push with mozharness patch here: https://tbpl.mozilla.org/?tree=Ash&rev=c1de7c0ede14

will wait for results before publishing patch for review. above buildbotcustom patch still is fine to review since it is a no-op anyway.

Comment 3

[[e-mail] (:bhearsum)]

Comment on attachment 8510801 [details] [diff] [review]
141023_twigs_not_running_pgo_for_nightlies.patch

Review of attachment 8510801 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Jordan Lund (:jlund) from comment #1)
> Created attachment 8510801 [details] [diff] [review]
> 141023_twigs_not_running_pgo_for_nightlies.patch
> 
> essentially the logic (incorrectly) was
>     if platform is a pgo-able platform *and* branch does actual pgo builds
>         compile with pgo for nightlies
> 
> where is should just be
>     if platform is a pgo able platform:
>         compile with pgo for nightlies
> 
> So this code wasn't doing anything anyways since we only have branches that
> are enabled that either do pgo builds or nightlies, not both. 
> 
> I'm leaving it out because this led me to realize that once we get to
> release branches, the logic gets more complicated since we will have this
> 'per-checkin' strategy for opt builds to deal with too.
> 
> Instead, I'm going configure things more in mozharness. This is because:
> 
> 1) I'd prefer buildbot just tell mozharness when the build is an actual PGO
> build (there is PGO in the buildername). Then, in mozharness, configure the
> edge cases of when we still want to compile with PGO even when it is not a
> PGO classified build.

Am I reading this right that we're detecting whether or not a build should be PGO on mozharness by parsing the builder name? If so, I don't think this is a good strategy.

Comment 4

[Jordan Lund (:jlund)]

> > 1) I'd prefer buildbot just tell mozharness when the build is an actual PGO
> > build (there is PGO in the buildername). Then, in mozharness, configure the
> > edge cases of when we still want to compile with PGO even when it is not a
> > PGO classified build.
> 
> Am I reading this right that we're detecting whether or not a build should
> be PGO on mozharness by parsing the builder name? If so, I don't think this
> is a good strategy.

no sorry, the params '(there is PGO in the buildername)' was added to describe in this bug what we consider an actual PGO build to be vs when we just do the compile/sendchange step against pgo (but not change the upload dir or stage_platform). mozharness does not parse the buildername.

the strategy is:

if --pgo-build is passed to mozharness: mozharness will think this is an actual pgo build

else: mozharness will apply pgo on compile+sendchange steps if (the build is an opt pgo-able platform on a per-checkin strat branch) or (the build is a pgo-able platform and it's a nightly)

this is the same logic we do in misc.py[1][2][3] but I am offloading the responsibility away from misc.py and making it specific to the mozharn script.

[1] http://mxr.mozilla.org/build/source/buildbotcustom/misc.py?rev=451d7d27ca46#1705 <- opt
[2] http://mxr.mozilla.org/build/source/buildbotcustom/misc.py?rev=451d7d27ca46#1999 <- nightly
[3] http://mxr.mozilla.org/build/source/buildbotcustom/misc.py?rev=451d7d27ca46#1802 <- actual pgo build

Comment 5

[Jordan Lund (:jlund)]

also the above ash push failed for different reasons but I have this push: https://tbpl.mozilla.org/?tree=Ash&rev=2bee4e7591f9

against mozharness changes that are now green. the assoctiated pgo fixes for mozharness can be seen here: https://bug1055918.bugzilla.mozilla.org/attachment.cgi?id=8512158

basically I am adding a _compile_against_pgo() in mozharness to represent whether or not we still compile against pgo even if it's not a pgo build. *sigh* so many edge cases...

Comment 6

[Jordan Lund (:jlund)]

Comment on attachment 8510801 [details] [diff] [review]
141023_twigs_not_running_pgo_for_nightlies.patch

on default: http://hg.mozilla.org/build/buildbotcustom/rev/28cdef2c1df3

Comment 7

[Chris Cooper [:coop] (he/him)]

In production: https://hg.mozilla.org/build/buildbotcustom/rev/28cdef2c1df3

Comment 8

[Jordan Lund (:jlund)]

https://bugzilla.mozilla.org/attachment.cgi?id=8512807 resolves this issue from the mozharness side