491464 - Shark builds should support partial/full upgrades

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P4)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

+++ This bug was initially created as a clone of Bug #448727 +++

With the work on bug 448727 we have Shark builds on trunk and 1.9.1 now. Those builds are very helpful for testing. Sadly they don't support partial/full upgrades. Personally I would like to use those builds on a daily basis but I don't want to download and install those builds each day. I believe the lack of this feature could also stop other people.

For QA it would be very helpful to have upgrades for Shark builds so we can rely on a larger user base which use those builds daily and could report performance issues more easily.

Comment 1

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

AUS can't tell the fully-optimized nightlies and shark builds apart, so we cannot enable nightly updates for those at present. Related is bug 469161, where tracemonkey builds update to mozilla-central.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

I think there's two decisions to be made here:

1. Do we want nightly builds to always be shark builds?

Pros: Shark builds for all OSX nightly testers!
Cons: Larger binaries, potentially not as close to actual-shipping-code?

2. Do we want to create an AUS channel for Shark builds?

Pros: Shark builds for some OSX nightly testers!
Cons: Another nightly process to track, more build resources required?

I think I'm fine with either of these options, with a slight preference for option 1 if we don't think Shark builds are materially different from non-Shark builds.

Comment 3

[Benjamin Smedberg]

In bug 482462 I wanted to enable dtrace for shark builds, which would give us additional profiling and statistics capabilities, but that makes the builds 10.5+ only and removes PPC support.

I'd prefer to use a separate update channel (nightly-shark) for these builds if that's practical (it should just be --enable-update-channel=nightly-shark or something like that).

Comment 4

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

Not quite so easy as setting nightly-shark in the mozconfig I'm afraid, as the partial update generator and AUS where built with only one nightly channel in mind.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

It would be really useful to have this. Being able to profile my regular browser would be tremendously handy.

Comment 6

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

Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

I wonder if perhaps we should just add shark support to our regular builds, but preff'ed off. Still the releng problems preventing this from being easy should be fixed, especially if we want to change the builds in any other way.

Comment 8

[(not currently active) Ted Mielczarek]

Our regular nightlies have been shipping unstripped for a while, so you can Shark them, you just don't get the nice startShark / stopShark APIs for fine-grained control of profiling JS.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #8)
> Our regular nightlies have been shipping unstripped for a while, so you can
> Shark them, you just don't get the nice startShark / stopShark APIs for
> fine-grained control of profiling JS.

Sorry, I meant adding startShark()/stopShark() support to our regular builds.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Now that we omit the frame pointer in our mac builds this has become significantly more important again. When can we expect to see this fixed?

Comment 11

[Jeff Muizelaar [:jrmuizel]]

ping?

Comment 12

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

We don't have any working shark builds at the moment, need to fix that first (bug 600434). Supposing we do they'll be 32bit builds, per https://developer.mozilla.org/en/Profiling_JavaScript_with_Shark, and still require 10.6. Given we now default to running 64bit on 10.6 are 32bit shark builds useful for profiling ?

Comment 13

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

Shark builds were revived in bug 634908.

Devs: is this still useful/wanted?

Nick: what would need to happen on the AUS side WRT setting separate channels for these builds?

Comment 14

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

Coop: we'd still need some way of distinguishing shark and non-shark nightlies, which otherwise have the same version and channel. Using channel='nightly-shark' would be OK, unless people want shark on long-lived project branches like tracemonkey too, then the likes of
 http://mxr.mozilla.org/mozilla/source/webtools/aus/xml/inc/config-dist.php#144
gets pretty gross.

Comment 15

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #13)
> Shark builds were revived in bug 634908.
> 
> Devs: is this still useful/wanted?

It would be definitely be useful to me.

Comment 16

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

(In reply to comment #14)
> Coop: we'd still need some way of distinguishing shark and non-shark nightlies,
> which otherwise have the same version and channel. Using
> channel='nightly-shark' would be OK, unless people want shark on long-lived
> project branches like tracemonkey too, then the likes of
>  http://mxr.mozilla.org/mozilla/source/webtools/aus/xml/inc/config-dist.php#144
> gets pretty gross.

I suspect they might want such a thing, i.e. shark updates on project branches. 

I'm not too concerned about it getting ugly(er), so long as our data structure is manageable.

Comment 17

[Chris AtLee [:catlee]]

So, for m-c, does this mean:

* Add --enable-update-channel=nightly-shark in mozconfig
* Figure out how to get snippets uploaded to a new directory (mozilla-central-shark)
* Add 'nightly-shark' to aus' config-dist.php, and point it at mozilla-central-shark

?

Other projects could use nightly-<project>-shark channel.

Comment 18

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

Taking bug 491464 off nthomas' plate, but giving him this bug in return. It's all about balance. ;)

Comment 19

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

(In reply to comment #18)
> Taking bug 491464 off nthomas' plate, but giving him this bug in return. It's
> all about balance. ;)

Er, make that bug 507544.

Comment 20

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

(In reply to comment #3)
> In bug 482462 I wanted to enable dtrace for shark builds, which would give
> us additional profiling and statistics capabilities, but that makes the
> builds 10.5+ only and removes PPC support.
> 
> I'd prefer to use a separate update channel (nightly-shark) for these builds
> if that's practical (it should just be --enable-update-channel=nightly-shark
> or something like that).

bsmedberg, can we revisit this now that m-c & peers are 10.5 and above ? We currently create a 35MB opt binary and a 37M opt shark binary - which are both universal builds. The effective differences between the mozconfigs are:

-ac_add_options --enable-update-channel=nightly
-ac_add_options --enable-update-packaging
-ac_add_options --enable-tests
-ac_add_options --enable-codesighs
+ac_add_options --disable-tests
 ac_add_options --disable-install-strip
 
+# shark specific options
+ac_add_options --enable-shark
+ac_add_options --enable-dtrace
+ac_add_options --enable-debugger-info-modules
+
+# Need this to prevent name conflicts with the normal nightly build packages
+export MOZ_PKG_SPECIAL="shark"
-----

I suspect we could just add the 3 'shark specific options' to the nightly mozconfig, and deprecate a bunch of (nasty) shark specific buildbot code and CPU time.

Comment 21

[(not currently active) Ted Mielczarek]

FWIW,

(In reply to comment #20)
> +# shark specific options
> +ac_add_options --enable-shark

This adds content-exposed JS APIs, which is the main reason we've been shipping it as a separate build, I think. It also disables a specific compiler optimization (-fomit-frame-pointer), since that hinders profiling, which will have a slight perf impact.

> +ac_add_options --enable-debugger-info-modules

This option is obsolete. (Replaced by --enable-debug-symbols, which is on by default now.)

Comment 22

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

Urgh, guess we'll have to continue with separate builds then.

We are currently creating shark builds for these branches:
  mozilla-1.9.1
  mozilla-1.9.2
  mozilla-2.0
  mozilla-aurora
  mozilla-central
  tracemonkey

Which ones do we want to provide nightly updates for ?

Comment 23

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

Attachment: Append -shark to BUILDTARGET for --enable-shark

Rob, what do you think of this ? It should get us AUS queries from Shark builds containing build targets like this:
  Darwin_x86_64-gcc3-u-i386-x86_64-shark
  Darwin_x86-gcc3-u-i386-x86_64-shark
Since it's only build time it shouldn't be any risk to release builds, and would be a much cleaner solution than setting channels like nightly-shark, nightly-tracemonkey-shark etc

Comment 24

[Robert Strong (they/them - no direct email)]

Comment on attachment 538431 [details] [diff] [review]
Append -shark to BUILDTARGET for --enable-shark

I'm fine with this though the following test needs to be updated for this as well
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/test/unit/test_0040_general.js#143

might want to change the following as well
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/nsBlocklistService.js#117

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpcshell/test_bug430120.js#108

Comment 25

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

Attachment: [m-c] partial test support (WIP)

Rob, here's a first cut at adding tests. I would like to change the blocklist as well as update queries but I'm hitting a problem with the test infra there, in particular knowing that a build is shark at test time. There's no preprocessing there in the style of toolkit/mozapps/update/test/unit/head_update.js.in.

Before I start adding makefiles and preprocessing I just wanted 
to point out that the shark nightlies aren't used in any unit tests, and to confirm it's useful to add test support for shark.

Comment 26

[Robert Strong (they/them - no direct email)]

(In reply to comment #25)
> Created attachment 539034 [details] [diff] [review] [review]
> [m-c] partial test support (WIP)
> 
> Rob, here's a first cut at adding tests. I would like to change the
> blocklist as well as update queries but I'm hitting a problem with the test
> infra there, in particular knowing that a build is shark at test time.
> There's no preprocessing there in the style of
> toolkit/mozapps/update/test/unit/head_update.js.in.
> 
> Before I start adding makefiles and preprocessing I just wanted 
> to point out that the shark nightlies aren't used in any unit tests, and to
> confirm it's useful to add test support for shark.
Since dev's compiling shark builds can run the tests I think it is useful.

Comment 27

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

Ah yes, we don't want tests behaving differently for them and for tbpl.

Comment 28

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

Attachment: Append -shark to BUILDTARGET for --enable-shark (with test support) [inbound]

Rob, this is the test support you asked for. I've confirmed we get build targets like this
  Darwin_x86_64-gcc3-u-i386-x86_64-shark
in AUS queries when shark is enabled. The xpcshell tests are green on try with shark disabled or enabled, opt and debug (except mac64 debug with shark, which blew up in the compile of content/base/test/TestNativeXMLHttpRequest.cpp, so no result there).

I've spun blocklist changes out to bug 668156, because the build system mods required to add preprocessing of the tests are beyond the scope of what I need to do here.

Comment 29

[Robert Strong (they/them - no direct email)]

Comment on attachment 542740 [details] [diff] [review]
Append -shark to BUILDTARGET for --enable-shark (with test support) [inbound]

>diff --git a/toolkit/mozapps/update/test/unit/test_0040_general.js b/toolkit/mozapps/update/test/unit/test_0040_general.js
>--- a/toolkit/mozapps/update/test/unit/test_0040_general.js
>+++ b/toolkit/mozapps/update/test/unit/test_0040_general.js
>@@ -136,16 +136,22 @@ function check_test_pt4() {
>     // Mac universal build should report a different ABI than either macppc
>     // or mactel. This is necessary since nsUpdateService.js will set the ABI to
>     // Universal-gcc3 for Mac universal builds.
>     var macutils = AUS_Cc["@mozilla.org/xpcom/mac-utils;1"].
>                    getService(AUS_Ci.nsIMacUtils);
> 
>     if (macutils.isUniversalBinary)
>       abi += "-u-" + macutils.architecturesInBinary;
>+
>+    if (IS_SHARK) {
>+      // Disambiguate optimised and shark nightlies
>+      abi += "-shark"
>+    }
>+
nit: please remove the blank extra line

>   }
> 
>   do_check_eq(getResult(gRequestURL), gAppInfo.OS + "_" + abi);
>   run_test_pt5();
> }
> 
> // url constructed with %LOCALE%
> // Bug 488936 added the update.locale file that stores the update locale

Comment 30

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

Comment on attachment 542740 [details] [diff] [review]
Append -shark to BUILDTARGET for --enable-shark (with test support) [inbound]

With the nit fixed:
http://hg.mozilla.org/integration/mozilla-inbound/rev/95f6f9fcb15d

Comment 31

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

Please do not resolve this bug FIXED when that change merges to mozilla-central.

Comment 32

[Marco Bonardo [:mak]]

As you wish, Sir.
http://hg.mozilla.org/mozilla-central/rev/95f6f9fcb15d

Comment 33

[Jeff Muizelaar [:jrmuizel]]

It seems like ehsan's profiling branch will meet my need sufficiently that I have no need for this anymore.

Comment 34

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #33)
> It seems like ehsan's profiling branch will meet my need sufficiently that I
> have no need for this anymore.

What does that mean? Is there any code which will be merged to mozilla-central? If not it's still a valid request. At least for me as reporter of the bug. I don't want to use a different branch to be able to get updates for shark builds.

Comment 35

[Chris AtLee [:catlee]]

macosx64 builds on the profiling branch are now built with shark enabled. If you need shark builds that update, please use those.