769099 - deploy new NDK (r8c) for use on production releng systems

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task, P2)

Description

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

per mtg w/blassey:

1) new ndk + gcc + gold linker setup in bug#675572. This has already been deployed to all the production RelEng systems. At the time, to avoid disruption to the FennecNative14.0 release, we decided to NOT enable the new ndk/compiler/linker by default.

2) Now that FN14.0 has shipped, we can go ahead to enable this by default on m-c. This compiler change can then ride the trains to migrate to aurora to beta to release along with everything else.

3) To give the most time possible for shaking out any compiler/linker/ndk woes, lets enable this on m-c as soon as safe after the next migration. If I follow calendars correctly, this means on/soonafter 17july2012.

Comment 1

[Mike Hommey [:glandium]]

Please don't switch armv6 builds, because of bug 772974

Comment 2

[Kartikaya Gupta (email:[e-mail])]

The blocker is a WFM now.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Some notes:

1) There is new NDK (version r8c) pushed out by Google that uses gcc 4.6 and gold, thereby making the mozilla-repackaged NDK redundant (as far as I know there are no other advantages to the mozilla one)

2) Using the mozilla-repackaged NDK causes failures (as per bug 811501) which do not occur with the stock NDK

3) I also did a quick check to see if the new NDK produces smaller binaries/APKs and discovered that it did. I built cset a9d59073400b locally using the same mozconfig as is used on tbpl, except updated the NDK and SDK bits to point to my local r8c/android-17 NDK/SDK (I also had to disable warnings-as-errors since gcc 4.6 produces more warnings and that causes the build to fail otherwise).
I then compared the file sizes from my APK and the one from tbpl:

kats@kgupta-pc new$ (find . -type f | while read file; do SIZE=$(cat $file | wc -c); NFILE=$(echo $file); NSIZE=$(cat ../old/$NFILE | wc -c); if [ $SIZE -ne $NSIZE ]; then echo "$((NSIZE - SIZE)) $file"; fi; done) | sort -rn
cat: ../old/./META-INF/CERT.RSA: No such file or directory
cat: ../old/./META-INF/CERT.SF: No such file or directory
248930 ./libxul.so
57076 ./classes.dex
22608 ./libnss3.so
12027 ./lib/armeabi-v7a/libplugin-container.so
10797 ./libmozalloc.so
10110 ./libmozsqlite3.so
7428 ./lib/armeabi-v7a/libmozglue.so
5601 ./libsmime3.so
5084 ./libnssutil3.so
4606 ./libsoftokn3.so
4167 ./libfreebl3.so
3250 ./libssl3.so
2165 ./libnspr4.so
129 ./omni.ja
80 ./platform.ini
80 ./application.ini
19 ./META-INF/MANIFEST.MF
-340 ./libplc4.so
-509 ./libnssckbi.so
-776 ./META-INF/CERT.RSA
-1758 ./lib/armeabi-v7a/libomxplugin.so
-3561 ./libplds4.so
-9817 ./libxpcom.so
-55031 ./META-INF/CERT.SF

That is, the new NDK/SDK produce .so and .dex files that are a total of 320788 and 57076 bytes smaller, respectively. The packaged APK actually goes down in size by over 2 MiB (from 23522739 to 21048011) but I'm a little more suspicious of that number.


So, in conclusion: I think that we should
(1) abandon the mozilla-repackaged NDKs and therefore mark bug 811501 as a wontfix
(2) fix up the code so that the new warnings produced by gcc-4.6 are handled and the warnings-as-errors build actually completes, and
(3) update the buildbot to use the r8c NDK.

Since doing this should in theory have some memory usage benefits because of smaller .so files, I'm marking this as a blocker of bug 792131 and also adding the MemShrink tag. Assuming B2G is also being built with an older NDK/SDK this might be useful there as well. It's not a huge gain but it's better than nothing, and using a newer toolchain is probably better anyway.

Comment 4

[Mike Hommey [:glandium]]

Moreover, NDK r8c finally fixed http://code.google.com/p/android/issues/detail?id=23203 (for which we added an ugly workaround in bug 720621).

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Bug 813918 is another bug that we have worked around because of badness in older NDKs but is fixed in r8c.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kartikaya Gupta (:kats) from comment #3)
> So, in conclusion: I think that we should
> (1) abandon the mozilla-repackaged NDKs and therefore mark bug 811501 as a
> wontfix

I have updated the wiki at https://wiki.mozilla.org/Mobile/Fennec/Android to no longer mention the mozilla-repackaged NDKs, and have marked bug 811501 as wontfix.

> (2) fix up the code so that the new warnings produced by gcc-4.6 are handled
> and the warnings-as-errors build actually completes, and

Patches are in bug 814418, pending review.

> (3) update the buildbot to use the r8c NDK.

Need somebody from releng to deploy r8c (and preferably also SDK version 17 - see bug 812227) to the slaves before I can update mozconfigs. :jodiunn, I know you guys are busy as hell but this should help both B2G and Android so is there any chance of it happening?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

:glandium tells me that B2G doesn't use the NDK to build so maybe this won't help them at all. But I'd still like it for Fennec :)

Comment 8

[Justin Lebar (not reading bugmail)]

(In reply to Kartikaya Gupta (:kats) from comment #7)
> :glandium tells me that B2G doesn't use the NDK to build so maybe this won't
> help them at all. But I'd still like it for Fennec :)

We use the NDK binaries in the B2G/prebuilt directory, I think.  We ought to be able to change things so as to use the new NDK binaries, I believe.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Justin Lebar [:jlebar] from comment #8)
> (In reply to Kartikaya Gupta (:kats) from comment #7)
> > :glandium tells me that B2G doesn't use the NDK to build so maybe this won't
> > help them at all. But I'd still like it for Fennec :)
> 
> We use the NDK binaries in the B2G/prebuilt directory, I think.  We ought to
> be able to change things so as to use the new NDK binaries, I believe.

AFAIK, it's not the same toolchain as the NDK. (Android has two toolchains)

Comment 10

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

Could you update the summary with the action you'd like take now ?

Comment 11

[Kartikaya Gupta (email:[e-mail])]

If I understand the process correctly, I believe the only action needed by releng is to deploy the stock r8c NDK to the buildbot/production systems. I can do the rest of the legwork to verify everything builds on m-c and update the in-tree mozconfigs to use the new NDK. Also see bug 812227 - it might be convenient to upgrade the android SDKs with v17 at the same time, and I can update our mozconfigs to use that as well.

Comment 12

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

From irc w/kats (and mfinkle, ted):

1) kats will file bug to get a loaner machine and verify this NDK n8c can deploy and build just fine on production machines.
2) This NDK n8c change will (possibly) fix some android-x86 build issues also (per ted)
3) This NDK n8c change should be done after the current B2G rush, and sometime before the next train migration (end of dec?). 
4) scope of work here is 
* puppet changes to deploy NDK n8c to all machines (per comment#0, RelEng has already done this at least once for NDK n7b)
* mozconfig changes to change builds-per-checkin and nightly builds to use the new NDK - this change is to be visible on mozilla-central, project branches and try.
* these changes would ride the trains, and need to be tracked as part of each branch migration work
5) kats has already updated https://wiki.mozilla.org/Mobile/Fennec/Android#Install_Android_SDK


Its hard to pay attention to this before mid-week next week (deadline for lot of b2g items). kats/joduinn to reconnect end of next week (13-14dec).

Comment 13

[Mike Hommey [:glandium]]

Would it be possible to use tooltool for the ndk while at it?

Comment 14

[Kartikaya Gupta (email:[e-mail])]

(In reply to John O'Duinn [:joduinn] from comment #12)
> 1) kats will file bug to get a loaner machine and verify this NDK n8c can
> deploy and build just fine on production machines.

filed bug 818676 for this.

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Attachment: NDK r8c rpm spec file

I verified that NDK r8c can be used to build ARMv7 fennec builds just fine (see https://bugzilla.mozilla.org/show_bug.cgi?id=818676#c2). In order to do this I manually installed NDK r8c onto the machine I was testing with, however I believe the attached spec file can be used to generate the necessary RPM. It's basically the same as the r5c spec file from the rpm-sources repo with a few bits flipped to point to r8c.

I believe the work left in this bug involves:
1) building the rpm
2) deploying the RPM to the releng-centos6-i386 repo
3) updating the fennec mock environment to include the r8c NDK alongside the r5c one (we will need both for the time being)

I can update the in-tree ARMv7 mozconfigs as part of a separate bug once the NDK is deployed, because there may be changes that sneak in between now and then that may cause the build to fail with r8c.

Comment 16

[Kartikaya Gupta (email:[e-mail])]

I mentioned this in my comment on bug 818676, but putting it here as well: ARMv7 builds work fine with NDK r8c, but ARMv6 builds crash with bug 803601. I am looking into that issue and hope to have a fix or workaround for it. We also have bug 772974 and bug 790624 which are armv6-related and will need to be verified on r8c before we can switch over ARMv6 builds.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

I also talked with :kmoir and we agreed that this shouldn't affect the x86 builds; they will continue to be built with r7b until we've tested them with the new NDK and are ready to switch them over.

Comment 18

[Rail Aliiev [:rail]]

Comment on attachment 692799 [details]
NDK r8c rpm spec file

Checked in with 2 minor changes (s/\t// and removed % from comments which was causing some problems)
http://hg.mozilla.org/build/rpm-sources/rev/c1ecef0d4be2

Comment 19

[Rail Aliiev [:rail]]

FTR, build and deployed as following:

# on a mock slave
mock_mozilla -v -r mozilla-centos6-i386 --init
wget http://dl.google.com/android/ndk/android-ndk-r8c-linux-x86.tar.bz2
wget -O android-ndk8.spec https://bug769099.bugzilla.mozilla.org/attachment.cgi?id=692799
mock_mozilla -r mozilla-centos6-i386 --copyin ./android-ndk-r8c-linux-x86.tar.bz2 /builds/rpmbuild/SOURCES/
mock_mozilla -r mozilla-centos6-i386 --copyin ./android-ndk8.spec /builds/rpmbuild/SPECS/

mock_mozilla -r mozilla-centos6-i386 --shell "rpmbuild -ba /builds/rpmbuild/SPECS/./android-ndk8.spec"
cp /builds/mock_mozilla/mozilla-centos6-i386/root/builds/rpmbuild/RPMS/i686/android-ndk8-r8c-0moz1.i686.rpm ./


# on releng-puppet1.srv.releng.scl3
chmod 644 *.rpm
sudo chown puppetagainsync:puppetagainsync *.rpm
sudo mv -vi *.rpm /data/repos/yum/releng/public/CentOS/6/i386/
sudo -u puppetagainsync createrepo --update /data/repos/yum/releng/public/CentOS/6/i386

Comment 20

[Rail Aliiev [:rail]]

Attachment: configs

To be tested

Comment 21

[Rail Aliiev [:rail]]

Thanks to Callek, I moved the spec to centos6 directory http://hg.mozilla.org/build/rpm-sources/rev/963b152d7186

Comment 22

[Rail Aliiev [:rail]]

Comment on attachment 695047 [details] [diff] [review]
configs

It worked fine in staging:
* it installs 2 versions of ndk side by side
* android build works with old mozconfigs
* After I pushed a mozconfig changes to my user repo the build picked up the new ndk:
...
/usr/bin/ccache /tools/android-ndk-r8c/toolchains/arm-linux-androideabi-4.4.3/prebuilt/linux-x86/bin/arm-linux-androideabi-g++ ...
...

Once this patch is reviewed, landed and the build masters are reconfiged we can push a couple of build to try to test the binaries and make sure that the new ndk works as expected.

I'll file a bug to remove the old ndk when this one lands on mozilla-release.

Comment 23

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

per email thread, we expect to have this new NDK deployed in production next week, well in advance of the next migration. 


(In reply to Kartikaya Gupta (email:[e-mail]) from comment #16)
> I mentioned this in my comment on bug 818676, but putting it here as well:
> ARMv7 builds work fine with NDK r8c, but ARMv6 builds crash with bug 803601.
> I am looking into that issue and hope to have a fix or workaround for it. We
> also have bug 772974 and bug 790624 which are armv6-related and will need to
> be verified on r8c before we can switch over ARMv6 builds.

Anything here still to be verified? I'd like to make sure upgrading NDK this close to migration causes no surprises.

Comment 24

[Kartikaya Gupta (email:[e-mail])]

Thanks rail, this all looks beautiful. Just one comment:

(In reply to Rail Aliiev [:rail] from comment #22)
> /usr/bin/ccache
> /tools/android-ndk-r8c/toolchains/arm-linux-androideabi-4.4.3/prebuilt/linux-
> x86/bin/arm-linux-androideabi-g++ ...
> ...

When we switch over ARMv7 builds to the new NDK we will be using the arm-linux-androideabi-4.6 toolchain rather than the -4.4.3 toolchain shown above. I'll be adding an --with-android-toolchain thing to the mozconfigs to make this happen. I verified that this works fine on the loaner slave; I am also doing this in my local builds. Feel free to verify it if you like; otherwise we can verify it on try once the new config changes are rolled out.

(In reply to John O'Duinn [:joduinn] from comment #23)
> Anything here still to be verified? I'd like to make sure upgrading NDK this
> close to migration causes no surprises.

There are still issues that exist when building ARMv6 with the new NDK, and I'm working on resolving them. However as long as we don't change the ARMv6 mozconfigs yet it shouldn't have any effect on anything, as we will continue building those with old NDK.

Comment 25

[Rail Aliiev [:rail]]

Comment on attachment 695047 [details] [diff] [review]
configs

http://hg.mozilla.org/build/buildbot-configs/rev/8a808b2d62d9

Comment 26

[Rail Aliiev [:rail]]

In production

DEBUG: Installed:
DEBUG:   android-ndk5.i686 0:r5c-0moz3                                                 
DEBUG:   android-ndk8.i686 0:r8c-0moz1

@kats: can you push to try to verify how does it work? Feel free to reopen if something goes wrong.

Comment 27

[Kartikaya Gupta (email:[e-mail])]

Thanks, rail! It seems to be working great; I filed bug 825151 for switching over the ARMv7 mozconfigs since the try run I pushed at https://tbpl.mozilla.org/?tree=Try&rev=71cbe0026b7a seems to be good.

Comment 28

[Rail Aliiev [:rail]]

sweet \o/

Comment 29

[Hal Wine [:hwine] use NI!]

in production