942167 - Please add non-unified builds to mozilla-central

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[(no longer active)]

Based on our recent discussion on dev.platform and also discussing this with our sheriffs, we're going to need periodic non-unified builds on mozilla-central:

* These builds need to run once per day.
* They only need to run on mozilla-central, no other branches will be affected.
* They need to use the same mozconfigs as our regular builds, with --disable-unified-compilation added.
* Ideally it would be nice to run our tests on them as well.
* We need both debug and optimized builds, for all of the platforms that we currently do builds for.

Please let me know if you have any questions or if I can provide any information to help.  Thanks!

Comment 1

[Ryan VanderMeulen [:RyanVM]]

If we decide to go this route, please file another bug for adding proper TBPL support for these builds/tests.

Comment 2

[(no longer active)]

(In reply to comment #1)
> If we decide to go this route, please file another bug for adding proper TBPL
> support for these builds/tests.

Sure, will do.

Comment 3

[Chris AtLee [:catlee]]

Should the opt builds be PGO where applicable? Or should we have separate opt and PGO builds?

Comment 4

[(no longer active)]

(In reply to comment #3)
> Should the opt builds be PGO where applicable? Or should we have separate opt
> and PGO builds?

Ideally we would have them as separate builds.  But in practice, I would expect PGO only issues which only show up in non-unified builds to be much less common, so if that is difficult to add then we can perhaps live without it for a while.

Comment 5

[(no longer active)]

It would be really helpful if we had an ETA here...

Comment 6

[Chris AtLee [:catlee]]

Presumably we'd want these on Try as well?

Comment 7

[(no longer active)]

(In reply to comment #6)
> Presumably we'd want these on Try as well?

No, for try you can just edit the appropriate mozconfigs.

Comment 8

[Chris AtLee [:catlee]]

Ok, and from IRC yesterday we want these more frequently than nightly, but not per-checkin. Something like every 4 hours like we do for PGO builds?

Could we do PGO builds non-unified to serve this purpose?

Comment 9

[(no longer active)]

(In reply to comment #8)
> Ok, and from IRC yesterday we want these more frequently than nightly, but not
> per-checkin. Something like every 4 hours like we do for PGO builds?

Sure.

> Could we do PGO builds non-unified to serve this purpose?

We could, except that won't give us coverage over debug only build bustage.  If it's simpler to do PGO builds in non-unified mode and get separate non-unified debug builds, then that sounds fine too.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #0)
> * Ideally it would be nice to run our tests on them as well.

Are we really expecting unified-only test bustage often enough to make this worthwhile? Adding tests to these builds would add a lot of extra load vs. just the builds.

Comment 11

[(no longer active)]

(In reply to comment #10)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #0)
> > * Ideally it would be nice to run our tests on them as well.
> 
> Are we really expecting unified-only test bustage often enough to make this
> worthwhile? Adding tests to these builds would add a lot of extra load vs. just
> the builds.

I still don't have evidence that such test bustages are common, hence the "ideally".

Comment 12

[Mike Hommey [:glandium]]

What is the ETA for this?

Comment 13

[Chris AtLee [:catlee]]

Attachment: support enable_nonunified_build platform config flag

Comment 14

[Chris AtLee [:catlee]]

Attachment: enable non unified builds on linux64 opt and debug

Comment 15

[Chris AtLee [:catlee]]

Comment on attachment 8359907 [details] [diff] [review]
support enable_nonunified_build platform config flag

The builders will be called e.g. "Linux x86-64 b2g-inbound leak test non-unified". How does that sound?

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8359907 [details] [diff] [review]
support enable_nonunified_build platform config flag

Sounds fine to me. We're running just the builds without tests, right?

Comment 17

[Rail Aliiev [:rail]]

Comment on attachment 8359908 [details] [diff] [review]
enable non unified builds on linux64 opt and debug

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

lgtm

Comment 18

[Chris AtLee [:catlee]]

Attachment: tbpl support for non-unified builds

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8359943 [details] [diff] [review]
tbpl support for non-unified builds

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

::: js/Config.js
@@ +353,5 @@
>      "AddressSanitizer Opt Build": "Bo",
>      "AddressSanitizer Debug Build": "Bd",
>      "AddressSanitizer Opt Nightly": "No",
>      "AddressSanitizer Debug Nightly": "Nd",
> +    "Non-Unified Build": "B",

This should be:
"Non-Unified Build": "Bn",

And let's put it under the |"Build" : "B",| line.

Comment 20

[Chris AtLee [:catlee]]

Attachment: tbpl support for non-unified builds

comme ça?

Comment 21

[Mike Hommey [:glandium]]

Why only linux? We have had platform-specific non-unified builds problems in the past.

Comment 22

[(no longer active)]

(In reply to comment #21)
> Why only linux? We have had platform-specific non-unified builds problems in
> the past.

I think Chris just wants to start with Linux.

Comment 23

[[e-mail] (:bhearsum)]

in production

Comment 24

[[e-mail] (:bhearsum)]

These are doing sendchanges to the same branches as the normal opt builds, which causes two of each type of test to happen without a way to tell them apart (unless you dive deep into the log). I'm not sure if that's intended or not, so I wanted to note it here. Example: the "Linux x64 Opt" row here: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=44f359ca30eb

Comment 25

[Chris AtLee [:catlee]]

Attachment: disable upload/sendchange for nonunified builds

Comment 26

[Ed Morley [:emorley]]

TBPL part in production

Comment 27

[[e-mail] (:bhearsum)]

There may be some fallout which I filed as bug 960247. It's not clear if it's a one-off yet though.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

These are running on all branches (aurora, beta, b2g*, esr24, etc). I thought this was trunk-only for this cycle. We certainly don't have the configs on any other branches.

Comment 29

[Mike Hommey [:glandium]]

They are also running on try. We obviously don't need those periodic builds on try.

We don't need those on aurora, beta, etc. because of http://hg.mozilla.org/mozilla-central/file/1f835fe670d7/configure.in#l3645

Comment 30

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #29)
> They are also running on try.

Evidence:
https://tbpl.mozilla.org/?tree=Try&rev=d0fabc50c882

Comment 31

[Mike Hommey [:glandium]]

It also uses a very old mozconfig when building on branches without browser/config/mozconfigs/linux64/nightly-nonunified:
https://tbpl.mozilla.org/php/getParsedLog.php?id=33069978&full=1&branch=birch

Comment 32

[Chris AtLee [:catlee]]

Attachment: disable non-unified builds on gecko < 29 and try

Comment 33

[[e-mail] (:bhearsum)]

Comment on attachment 8361089 [details] [diff] [review]
disable non-unified builds on gecko < 29 and try

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

Don't we want these enabled as an option on try if we want them to stay unhidden?

Comment 34

[Ed Morley [:emorley]]

try-non-default perhaps? :-)

Comment 35

[Chris AtLee [:catlee]]

Comment on attachment 8361089 [details] [diff] [review]
disable non-unified builds on gecko < 29 and try

No, if people want to run them on try, they can edit their mozconfigs.

Comment 36

[[e-mail] (:bhearsum)]

something is in production!

Comment 37

[(no longer active)]

Chris, are we ready to extend this to all of the other platforms now?

Comment 38

[Chris AtLee [:catlee]]

Attachment: enable win32, osx opt and debug nonunified builds

Comment 39

[John Hopkins (:jhopkins)]

catlee: is win64 intentionally not enabled?

Comment 40

[John Hopkins (:jhopkins)]

Comment on attachment 8364348 [details] [diff] [review]
enable win32, osx opt and debug nonunified builds

[09:45am] jhopkins|buildduty: catlee: not enabled on win64 intentionally?
[09:50am] catlee: jhopkins|buildduty: yeah
[09:51am] catlee: very few people watch win64 build bustage

Comment 41

[John Hopkins (:jhopkins)]

in production.

Comment 42

[Jordan Lund (:jlund)]

something is in production!

Comment 43

[(no longer active)]

Chris, can we proceed with the rest of the builds here?

Comment 44

[(no longer active)]

ping?

Comment 45

[Chris AtLee [:catlee]]

Attachment: enable android/android-debug non-unified builds

Comment 46

[(no longer active)]

Chris, can we add b2g non-unified builds too?  Once we have those builds, we can turn on unified builds for b2g developers.

Comment 47

[Chris AtLee [:catlee]]

I don't think we have any way to change which mozconfig is used by b2g builds at the moment. John, how do we do that?

Comment 48

[(no longer active)]

Hmm, there is a bunch of mozconfigs here: http://mxr.mozilla.org/mozilla-central/source/b2g/config/mozconfigs/

Comment 49

[Chris AtLee [:catlee]]

Those are used by the desktop b2g builds, but not the device builds. Which ones do you want to have non-unified builds of?

Comment 50

[(no longer active)]

(In reply to comment #49)
> Those are used by the desktop b2g builds, but not the device builds. Which ones
> do you want to have non-unified builds of?

Both.  My goal is for us to enable unified builds locally for b2g developers, which requires us to have some kind of non-unified build on TBPL to make sure that things do not regress.

Comment 51

[Chris AtLee [:catlee]]

So my question still stands wrt the mozconfig used by b2g device builds.

Comment 52

[(no longer active)]

(In reply to comment #51)
> So my question still stands wrt the mozconfig used by b2g device builds.

Who can answer that question?  I don't know the answer.

Comment 53

[John Ford [:jhford] CET/CEST Berlin Time]

Well, we use the mozconfig here:

https://github.com/mozilla-b2g/gonk-misc/blob/master/default-gecko-config

We should be able to source a local file at the end of that mozconfig so that the local mozconfig can override the checked in configuration settings.

If we want something more specific, we could go with something like:

if [ "${MOZ_DMD:-0}" != 0 ]; then
  ac_add_options --enable-dmd
fi

and the required environment glue.

Comment 54

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: Device builds should know how to disable unifed builds

This works by adding MOZ_NON_UNIFIED=1 to .userconfig, or including it on the ./build.sh command line.  I have a really stale build directory, so I haven't tested it yet, but it should work

Comment 55

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: Device builds should know how to disable unifed builds

Previous patch had stuff it shouldn't.

Comment 56

[Chris AtLee [:catlee]]

lgtm

Comment 57

[(no longer active)]

John, is this patch good enough here?  If yes, can you please get this landed?  Thanks!

Comment 58

[Michael Wu [:mwu]]

Comment on attachment 8389388 [details] [diff] [review]
Device builds should know how to disable unifed builds

At the top of default-gecko-config, we source "b2g/config/mozconfigs/common". In b2g/config/mozconfigs/common, there is "ac_add_options --disable-unified-compilation". Do we really need this? I'm also not sure how this would hook into automation, unless we need to add code which sets MOZ_NON_UNIFIED.

Comment 59

[(no longer active)]

(In reply to Michael Wu [:mwu] from comment #58)
> Comment on attachment 8389388 [details] [diff] [review]
> Device builds should know how to disable unifed builds
> 
> At the top of default-gecko-config, we source
> "b2g/config/mozconfigs/common". In b2g/config/mozconfigs/common, there is
> "ac_add_options --disable-unified-compilation". Do we really need this? I'm
> also not sure how this would hook into automation, unless we need to add
> code which sets MOZ_NON_UNIFIED.

Comment 60

[John Ford [:jhford] CET/CEST Berlin Time]

I'm not sure what the requirements are here, but I can help with the patch regarding setting this value.

I thought the issue here was that we wanted to remove the --disable-unified-compilation in common mozconfig and have seperate builds to test unified disabled.

Is this correct?

Comment 61

[(no longer active)]

(In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC from comment #60)
> I'm not sure what the requirements are here, but I can help with the patch
> regarding setting this value.
> 
> I thought the issue here was that we wanted to remove the
> --disable-unified-compilation in common mozconfig and have seperate builds
> to test unified disabled.
> 
> Is this correct?

Yes.  For that to work, we need to give this as a parameter to the b2g build system and get it to give us --disable-unified-compilation when building Gecko, and once we have that capibility we can make unified builds the default and add additional builds on b2g which perform a non-unified compilation.  Therefore, we need to do something which we will be able to use in our infrastructure.

Gabriele also offered help with this on dev-b2g a while ago, wondering if those efforts got somewhere?

Comment 62

[Gabriele Svelto [:gsvelto]]

(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #61)
> Gabriele also offered help with this on dev-b2g a while ago, wondering if
> those efforts got somewhere?

I did some quick experimentation a couple of weeks ago so my results are of limited value but I'll come back and test again this week in the hope of having unified builds working on B2G. In my testing I noticed that removing the "ac_add_options --disable-unified-compilation" entry from b2g/config/mozconfigs/common was not enough to enable unified builds. I'm not sure why this is happening as my understanding is that unified builds are enabled by default and need to be disabled explicitly, not vice-versa. Anyway I tried explicitly adding "ac_add_options --enable-unified-compilation" to the build and it worked well both with regular device build and a debug one.

I still need to investigate what's needed to enable this correctly on B2G and more importantly if this works properly across the device, emulator and desktop builds.

Comment 63

[Mike Hommey [:glandium]]

Note the real problem is providing builds with --disable-unified-compilation *as well*.

I wish someone would fix the mess that b2g's mozconfig setup creates (that releng-only files are included for all builds).

Comment 64

[(no longer active)]

(In reply to comment #62)
> (In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment
> #61)
> > Gabriele also offered help with this on dev-b2g a while ago, wondering if
> > those efforts got somewhere?
> 
> I did some quick experimentation a couple of weeks ago so my results are of
> limited value but I'll come back and test again this week in the hope of having
> unified builds working on B2G. In my testing I noticed that removing the
> "ac_add_options --disable-unified-compilation" entry from
> b2g/config/mozconfigs/common was not enough to enable unified builds. I'm not
> sure why this is happening as my understanding is that unified builds are
> enabled by default and need to be disabled explicitly, not vice-versa. Anyway I
> tried explicitly adding "ac_add_options --enable-unified-compilation" to the
> build and it worked well both with regular device build and a debug one.
> 
> I still need to investigate what's needed to enable this correctly on B2G and
> more importantly if this works properly across the device, emulator and desktop
> builds.

FWIW the default is set to unified $NIGHTLY_BUILD is set in configure.in, which I would expect to be true for b2g trunk.

Comment 65

[(no longer active)]

So we chatted on IRC about this today, and it seems like we can take attachment 8389388 [details] [diff] [review] and remove the --disable-unified-compilation rule from b2g/config/mozconfigs/common.  Would that be the right fix here?

Comment 66

[Mike Hommey [:glandium]]

Sounds about right.

Comment 67

[Chris AtLee [:catlee]]

More decisions from IRC:

- we're going to target emulator builds (opt+debug) to start with
- going to use a cmdline flag to b2g_build.py to distinguish between unified and non-unified. I'm thinking that if we set --non-unified then MOZ_NON_UNIFIED=1 will be set in the build's environment.

Comment 68

[Chris AtLee [:catlee]]

Attachment: bbcustom support for periodic non-unified mh builds

Comment 69

[Chris AtLee [:catlee]]

Attachment: enable non-unified builds for all emulator types

Comment 70

[Chris AtLee [:catlee]]

Attachment: non-unified support for b2g_build.py

Comment 71

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: Read the environment variable

Mike, once this change is landed and the new build type is active, we'll be removing the --disable-unified options in the in-tree mozconfigs to make this effective.

Comment 72

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

Merged to production and deployed.

Comment 73

[Michael Wu [:mwu]]

Comment on attachment 8431103 [details] [diff] [review]
Read the environment variable

I'll let glandium make the call on this one since he's been wanting to kill this file.

Comment 74

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

mozharness non-unified builds are trying to upload and are failing, causing pulse queue problems on bm75. I'm not sure what's intended here, but based on attachment #8360555 [details] [diff] [review], I've landed the following patch to disable uploads for now:

https://hg.mozilla.org/build/mozharness/rev/be75692664ab

Comment 75

[Ryan VanderMeulen [:RyanVM]]

FWIW, we're also expecting comment 74 to stop tests from starting on nonunified builds. If not, that will need fixing as well.

Comment 76

[Nick Thomas [:nthomas] (UTC+12)]

Looks like we're doing periodic builds on try too. This code needs to allow disabling by branch:
  http://hg.mozilla.org/build/buildbotcustom/file/default/misc.py#l1237
  http://hg.mozilla.org/build/buildbotcustom/file/default/misc.py#l1293

Comment 77

[Nick Thomas [:nthomas] (UTC+12)]

To clarify, the set of builders is
    "b2g_try periodic": {
      "downstream": [
        "b2g_try_emulator-debug_nonunified", 
        "b2g_try_emulator-jb-debug_nonunified", 
        "b2g_try_emulator-jb_nonunified", 
        "b2g_try_emulator-kk-debug_nonunified", 
        "b2g_try_emulator-kk_nonunified", 
        "b2g_try_emulator_nonunified"
      ]
    }, 
(from the schedulers part of http://builddata.pub.build.mozilla.org/reports/allthethings.json.gz)
Ignore the previous comment - we can set enable_nonunified_build to False at the platform level, like attachment 8361089 [details] [diff] [review] attempts. I suspect it's failing for b2g emulator because no value is already set.

When the builds run they're green, but the log fails to upload ('who must be supplied'), and we get a nagios alert about dead entries in the comment queue on the masters. I've just been removing them, so the builds only show up on TBPL while they are running.

Comment 79

[Chris AtLee [:catlee]]

Attachment: disable non-unified b2g builds on non-trunk branches + try

This removes the following builders:
- b2g_mozilla-b2g26_v1_2_emulator-debug_nonunified
- b2g_mozilla-b2g26_v1_2_emulator-jb-debug_nonunified
- b2g_mozilla-b2g26_v1_2_emulator-jb_nonunified
- b2g_mozilla-b2g26_v1_2_emulator_nonunified
- b2g_mozilla-b2g28_v1_3_emulator-debug_nonunified
- b2g_mozilla-b2g28_v1_3_emulator-jb-debug_nonunified
- b2g_mozilla-b2g28_v1_3_emulator-jb_nonunified
- b2g_mozilla-b2g28_v1_3_emulator_nonunified
- b2g_mozilla-b2g28_v1_3t_emulator-debug_nonunified
- b2g_mozilla-b2g28_v1_3t_emulator_nonunified
- b2g_mozilla-b2g30_v1_4_emulator-debug_nonunified
- b2g_mozilla-b2g30_v1_4_emulator-jb-debug_nonunified
- b2g_mozilla-b2g30_v1_4_emulator-jb_nonunified
- b2g_mozilla-b2g30_v1_4_emulator-kk-debug_nonunified
- b2g_mozilla-b2g30_v1_4_emulator-kk_nonunified
- b2g_mozilla-b2g30_v1_4_emulator_nonunified
- b2g_try_emulator-debug_nonunified
- b2g_try_emulator-jb-debug_nonunified
- b2g_try_emulator-jb_nonunified
- b2g_try_emulator-kk-debug_nonunified
- b2g_try_emulator-kk_nonunified
- b2g_try_emulator_nonunified

Comment 80

[Chris AtLee [:catlee]]

What's left to do here?

Comment 81

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 8431103 [details] [diff] [review]
Read the environment variable

gonk-misc master: 27b2c2ef9a50d5dc79b6a771b3a3c775a888d13b

Comment 82

[Chris AtLee [:catlee]]

(In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC from comment #81)
> Comment on attachment 8431103 [details] [diff] [review]
> Read the environment variable
> 
> gonk-misc master: 27b2c2ef9a50d5dc79b6a771b3a3c775a888d13b

Landed https://github.com/mozilla-b2g/gonk-misc/commit/44b04243e31cd16f3baf54fcd4b5fecf9deb8b94 to fix broken variable expansion.

Comment 83

[(no longer active)]

Chris, glandium seems to think we should do unified builds for all of our platforms (see bug 950676 comment 22 and prior comments).  I'll let you guys discuss that here.

Comment 84

[Chris AtLee [:catlee]]

:glandium, do you have any numbers that show if we're a net win by enabling unified / periodic non-unified builds on these platforms?

We end up doing periodic non-unified builds on all trunk branches, including project branches where there could be relatively few checkins. In the worst case each push on a project branch is getting a unified + non-unified build.

Comment 85

[Chris AtLee [:catlee]]

I looked at a few logs on inbound, and we are already doing unified builds for linux32 and android x86.

Comment 86

[(no longer active)]

(In reply to Chris AtLee [:catlee] from comment #85)
> I looked at a few logs on inbound, and we are already doing unified builds
> for linux32 and android x86.

You mean for debug builds?  <http://mxr.mozilla.org/mozilla-central/source/browser/config/mozconfigs/linux32/debug#5> for example doesn't agree!

Comment 87

[Chris AtLee [:catlee]]

I meant for opt builds!

Comment 88

[Mike Hommey [:glandium]]

(In reply to Chris AtLee [:catlee] from comment #84)
> :glandium, do you have any numbers that show if we're a net win by enabling
> unified / periodic non-unified builds on these platforms?

I can't say for sure on incremental builds, but non-unified clobber builds, with or without sccache, are about twice as slow as clobber builds (and here i'm talking about the make -f client.mk step alone)

> We end up doing periodic non-unified builds on all trunk branches, including
> project branches where there could be relatively few checkins. In the worst
> case each push on a project branch is getting a unified + non-unified build.

Arguably, on those branches, we should do periodic builds every n pushes instead of every n hours (actually, I'd argue we should do that everywhere).

(In reply to Chris AtLee [:catlee] from comment #85)
> I looked at a few logs on inbound, and we are already doing unified builds
> for linux32 and android x86.

We aren't doing unified builds on linux debug, android armv6 debug and android x86 debug

Comment 89

[(no longer active)]

Nothing more to be done here.