1304998 - enable taskcluster windows opt pgo builds (tier 2)

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Rob Thijssen [:grenade (EET/UTC+0300)]]

somehow we neglected to add pgo builds when enabling debug and opt on taskcluster windows. add support for pgo.

Comment 1

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Attachment: Bug 1304998 - enable tc win pgo;

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

Comment 2

[Rob Thijssen [:grenade (EET/UTC+0300)]]

note: in order to test the changes above on try (where we haven't got syntax for pgo), i just added the "enable-pgo" option to the opt builds which are identical to pgo in how they are triggered with the exception of that command line switch being added to the mozharness call. the commit in the review only differs (from the one on try) by not having the enable-pgo options set for the win32-opt and win64-opt builds, only win32-pgo and win64-pgo.
the try test is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b055e05fa53205e842989bd0da3317d0565a9d5

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8794130 [details]
Bug 1304998 - enable tc win pgo;

https://reviewboard.mozilla.org/r/80714/#review79424

::: taskcluster/ci/build/windows.yml:41
(Diff revision 1)
>          using: mozharness
>          script: mozharness/scripts/fx_desktop_build.py
>          config:
>              - builds/taskcluster_firefox_win32_opt.py
>  
> +win32/pgo:

We fix this issue for Linux builds with

https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/build_attrs.py#22
        if build_type == 'pgo':
            build_platform = build_platform + '-pgo'
            build_type = 'opt'

        attributes = job.setdefault('attributes', {})
        attributes.update({
            'build_platform': build_platform,
            'build_type': build_type,
        })
        
The try syntax is matching on the build_platform and build_type (which only allows "d"ebug and "o"pt).  But the treeherder platform is linux64/pgo.  It's confusing, that's why there's bug 1286086

::: taskcluster/taskgraph/transforms/gecko_v2_whitelist.py:69
(Diff revision 1)
>      'win32-debug',
>      'win32-opt',
> +    'win32-pgo',
>      'win64-debug',
>      'win64-opt',
> +    'win64-pgo',

These match the index routes used in Buildbot?  I'm contractually obligated to ask ;)

::: taskcluster/taskgraph/transforms/job/mozharness.py:190
(Diff revision 1)
> +    for option in run.get('options', []):
> +        mh_command.append('--' + option)

++

Comment 4

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Comment on attachment 8794130 [details]
Bug 1304998 - enable tc win pgo;

https://reviewboard.mozilla.org/r/80714/#review79424

> We fix this issue for Linux builds with
> 
> https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/build_attrs.py#22
>         if build_type == 'pgo':
>             build_platform = build_platform + '-pgo'
>             build_type = 'opt'
> 
>         attributes = job.setdefault('attributes', {})
>         attributes.update({
>             'build_platform': build_platform,
>             'build_type': build_type,
>         })
>         
> The try syntax is matching on the build_platform and build_type (which only allows "d"ebug and "o"pt).  But the treeherder platform is linux64/pgo.  It's confusing, that's why there's bug 1286086

I believe the same transform is working for the Windows builds (build_platform should just be win32, win64)

> These match the index routes used in Buildbot?  I'm contractually obligated to ask ;)

I think we're safe:
https://tools.taskcluster.net/index/#gecko.v2.mozilla-central.latest.firefox/gecko.v2.mozilla-central.latest.firefox.win32-pgo
https://tools.taskcluster.net/index/#gecko.v2.mozilla-central.latest.firefox/gecko.v2.mozilla-central.latest.firefox.win64-pgo

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

> I believe the same transform is working for the Windows builds (build_platform should just be win32, win64)

Silly me, you're right of course :)

Comment 6

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Comment on attachment 8794130 [details]
Bug 1304998 - enable tc win pgo;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/80714/diff/1-2/

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8794130 [details]
Bug 1304998 - enable tc win pgo;

https://reviewboard.mozilla.org/r/80714/#review80638

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0aae2c2bdd0
enable tc win pgo; r=dustin

Comment 9

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Attachment: correction to TH platform name

I managed to miss an important platform distinction in my earlier patch. This corrects that error.

Comment 10

[Rob Thijssen [:grenade (EET/UTC+0300)]]

Attachment: ... with metadata

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f0aae2c2bdd0

Comment 12

[Rob Thijssen [:grenade (EET/UTC+0300)]]

please also apply the second patch in this bug, which adds a correction to the first patch
https://bug1304998.bmoattachments.org/attachment.cgi?id=8796434

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c95282b2888
correct TH pgo platform name; r=dustin

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/4c95282b2888