848764 - Enable on-demand decompression

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Enable on-demand decompression

This alone would regress apk size by more than 500KB, but bug 848770 brings this back to a few KB regression (except, probably, for x86, but 848773 should set things straight). We'll have to track startup times and possible problems with weird phones after this lands.

Comment 2

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bfc7718d631a

Comment 3

[Mike Hommey [:glandium]]

If this needs to be disabled for some reason, please only change mobile/android/confvars.sh. The configure.in change should stay.

Comment 4

[Mike Hommey [:glandium]]

And already disabled :(
https://hg.mozilla.org/integration/mozilla-inbound/rev/37bbd241f79a

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bfc7718d631a
https://hg.mozilla.org/mozilla-central/rev/37bbd241f79a

Comment 6

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

I was looking at the graphs on areweslimyet.com/mobile, and if you zoom in to the data around april 18 there's a spike in memory usage on the inbound build for bfc7718d631a that disappears on the next build when it was disabled. The about:memory dumps show, among other things, an increase of ~34 MB in resident memory usage in the size/shared-libraries/shared-libraries-mozilla/libxul.so memory reporter (which I believe is taken straight from the smaps file). I can provide the full memory dumps and a diff if that's useful in any way.

Comment 7

[Mike Hommey [:glandium]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> I was looking at the graphs on areweslimyet.com/mobile, and if you zoom in
> to the data around april 18 there's a spike in memory usage on the inbound
> build for bfc7718d631a that disappears on the next build when it was
> disabled. The about:memory dumps show, among other things, an increase of
> ~34 MB in resident memory usage in the
> size/shared-libraries/shared-libraries-mozilla/libxul.so memory reporter
> (which I believe is taken straight from the smaps file). I can provide the
> full memory dumps and a diff if that's useful in any way.

The smaps data would be useful, but i doubt it's pointing to anything real. It probably just points deficiencies in how we could memory usage.

Comment 8

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

Attachment: about:memory dump before landing on-demand decompression

Note that the smaps data isn't actually in this file; the about:memory page reads some of the smaps data and just reports some of it. The attached patch has the dump from the build prior to on-demand decompression landing. The next patch has the dump from the build that includes on-demand decompression turned on.

Comment 9

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

Attachment: about:memory dump before landing on-demand decompression

Comment 10

[Mike Hommey [:glandium]]

Yeah, so this is exactly what i thought. The fact that we're counting RSS instead of PSS is hurting, because the extra 30+MB of libxul are a writable view of the same things that are mapped elsewhere.

Comment 11

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef5b7b1039ac

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

As noted in comment 6, this patch does cause a RSS regression:

Regression: Mozilla-Inbound - Tp4 Mobile NoChrome (Main RSS) - Android 2.2 (Native) - 37% increase
---------------------------------------------------------------------------------------------
    Previous: avg 90637775.000 stddev 593265.832 of 12 runs up to revision be1d72a1f228
    New     : avg 124135500.000 stddev 572542.734 of 12 runs since revision ef5b7b1039ac
    Change  : +33497725.000 (37% / z=56.463)
    Graph   : http://mzl.la/16FEZ3w

Comment 14

[Nick Alexander :nalexander [he/him]]

Seems like there's a typo in

http://mxr.mozilla.org/mozilla-central/source/testing/testsuite-targets.mk#460

The latter should be host/bin.

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Nick Alexander :nalexander from comment #14)
> Seems like there's a typo in
> 
> http://mxr.mozilla.org/mozilla-central/source/testing/testsuite-targets.
> mk#460
> 
> The latter should be host/bin.

This is breaking l10n nightly builds:
https://tbpl.mozilla.org/php/getParsedLog.php?id=23220111&tree=Mozilla-Central

Comment 16

[Mike Hommey [:glandium]]

(In reply to Nick Alexander :nalexander from comment #14)
> Seems like there's a typo in
> 
> http://mxr.mozilla.org/mozilla-central/source/testing/testsuite-targets.
> mk#460
> 
> The latter should be host/bin.

Nope, that's purposeful, as indicated in the comment.

Comment 17

[Mike Hommey [:glandium]]

(In reply to Mark Finkle (:mfinkle) from comment #15)
> (In reply to Nick Alexander :nalexander from comment #14)
> > Seems like there's a typo in
> > 
> > http://mxr.mozilla.org/mozilla-central/source/testing/testsuite-targets.
> > mk#460
> > 
> > The latter should be host/bin.
> 
> This is breaking l10n nightly builds:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=23220111&tree=Mozilla-
> Central

I don't believe that particular failure is due to this bug. However https://tbpl.mozilla.org/php/getParsedLog.php?id=23200287&tree=Mozilla-Central#error10 is.

Comment 18

[Aki Sasaki (not active)]

All mobile nightly repacks are broken; I suspect this change.

http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-central-l10n/mozilla-central-android-l10n_1-unknown-bm63-build1-build8.txt.gz

06:26:02     INFO -  /bin/sh: line 1: host/bin/szip: No such file or directory
06:26:02    ERROR -  make[1]: *** [repackage-zip] Error 127
06:26:02     INFO -  make[1]: Leaving directory `/builds/slave/m-cen-and-l10n_1-0000000000000/build/mozilla-central/obj-l10n/mobile/android/locales'
06:26:02     INFO -  make: *** [repackage-zip-ar] Error 2
06:26:02    ERROR - Return code: 2

Comment 19

[Mike Hommey [:glandium]]

(In reply to Aki Sasaki [:aki] from comment #18)
> All mobile nightly repacks are broken; I suspect this change.
> 
> http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-
> central-l10n/mozilla-central-android-l10n_1-unknown-bm63-build1-build8.txt.gz
> 
> 06:26:02     INFO -  /bin/sh: line 1: host/bin/szip: No such file or
> directory
> 06:26:02    ERROR -  make[1]: *** [repackage-zip] Error 127
> 06:26:02     INFO -  make[1]: Leaving directory
> `/builds/slave/m-cen-and-l10n_1-0000000000000/build/mozilla-central/obj-l10n/
> mobile/android/locales'
> 06:26:02     INFO -  make: *** [repackage-zip-ar] Error 2
> 06:26:02    ERROR - Return code: 2

That's bug 874783

Comment 20

[Mike Hommey [:glandium]]

Backed out from beta, see bug 907957 comment 23.
https://hg.mozilla.org/releases/mozilla-beta/rev/34edd639e3d9

Comment 21

[Mike Hommey [:glandium]]

Backed out because bug 873569 broke the non MOZ_ENABLE_SZIP case. Filing a separate bug for that issue.
https://hg.mozilla.org/releases/mozilla-beta/rev/59d74640f7bd

Comment 22

[Mike Hommey [:glandium]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0b42c4652100

Comment 23

[Kevin Brosnan [Ex-Mozilla]]

Just for bookkeeping the bug referenced in comment 21 is bug 912377

Comment 24

[Mike Hommey [:glandium]]

Ryan, would you mind backing this out from beta? (per bug 886736 comments 66 to 68)

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/68bea7ee703d