1422734 - move --enable-small-chunk-size to moz.configure

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: move --enable-small-chunk-size to moz.configure

The setting claims that it was for B2G.  I know we've been removing B2G-related
code, but this setting actually seems pretty generic, and therefore would be
valuable to retain, rather than removing it outright.  Maybe the JS GC people
would feel differently, asking Jon for feedback.

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8934152 [details] [diff] [review]
move --enable-small-chunk-size to moz.configure

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

This gets set in mobile/android/confvars.sh. That needs to be reflected in mobile/android/moz.configure so it's not lost with this change.

Comment 3

[Jon Coppeard (:jonco)]

Comment on attachment 8934152 [details] [diff] [review]
move --enable-small-chunk-size to moz.configure

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

We've talked about reducing the chunk size for other reasons (nursery per zone group) so I think this is worth retaining.

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to Chris Manchester (:chmanchester) from comment #2)
> This gets set in mobile/android/confvars.sh. That needs to be reflected in
> mobile/android/moz.configure so it's not lost with this change.

Good catch.  I think I'd prefer to set it in js/moz.configure, with some sort of checking for Android when choosing the default; that feels a little more inline with how we handle other configure options.  WDYT?

Comment 5

[Chris Manchester (limited bugmail, email directly)]

(In reply to Nathan Froyd [:froydnj] (doing reviews while in Austin) from comment #4)
> (In reply to Chris Manchester (:chmanchester) from comment #2)
> > This gets set in mobile/android/confvars.sh. That needs to be reflected in
> > mobile/android/moz.configure so it's not lost with this change.
> 
> Good catch.  I think I'd prefer to set it in js/moz.configure, with some
> sort of checking for Android when choosing the default; that feels a little
> more inline with how we handle other configure options.  WDYT?

I would leave this patch as is, but simply add `imply_option('--enable-small-chunk-size', True)` to `android/moz.configure`. This is how we've done confvars.sh things so far, it preserves the behavior, because things set in confvars.sh override any attempts to set the option. 

Really though, allowing people to set/unset this in mozconfigs probably wouldn't do any real harm, I'll leave it up to you.

Comment 6

[Nathan Froyd [:froydnj]]

Attachment: move --enable-small-chunk-size to moz.configure

As suggested.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b21a98026b7
move --enable-small-chunk-size to moz.configure; r=chmanchester

Comment 8

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/3b21a98026b7

Comment 9

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/3b21a98026b7