Closed
Bug 1039902
Opened 10 years ago
Closed 10 years ago
Minify the Fennec JS code
Categories
(Firefox Build System :: Android Studio and Gradle Integration, defect)
Firefox Build System
Android Studio and Gradle Integration
All
Android
Tracking
(firefox35 fixed, firefox36 fixed, fennecNightly+)
RESOLVED
FIXED
mozilla36
People
(Reporter: ckitching, Assigned: nalexander)
References
(Depends on 2 open bugs, Blocks 1 open bug)
Details
(Keywords: feature, perf)
Attachments
(1 file, 1 obsolete file)
5.91 KB,
patch
|
glandium
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
I noticed that the JS shipped in Fennec isn't minified.
When I mentioned this on IRC, nalexander suggested:
<nalexander> ckitching: http://mxr.mozilla.org/mozilla-central/source/mobile/android/installer/Makefile.in#31
<nalexander> ckitching: add MOZ_PACKAGER_MINIFY_JS=1 there and see what happens :)
So I did. The apk got 6MB smaller.
Wat.
The even better news is that the minification used seems not to be modifying variable names (ever), merely removing whitespace and comments and so forth.
As such, we could have even greater benefit by configuring it to shrink variable names (carefully excluding those which are exposed to the addons API and so forth).
Try push:
https://tbpl.mozilla.org/?tree=Try&rev=9b86a7242f2d
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → chriskitching
OS: Linux → Android
Hardware: x86_64 → All
Reporter | ||
Comment 1•10 years ago
|
||
It's like christmas.
Attachment #8457738 -
Flags: review?(nalexander)
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8457738 [details] [diff] [review]
Turn on minification
Review of attachment 8457738 [details] [diff] [review]:
-----------------------------------------------------------------
There is a ton of discussion about this in https://bugzilla.mozilla.org/show_bug.cgi?id=903149.
Reading that, the following things seem most important:
* an early note suggested that Fennec's APK shrank "only" 400kb; we should verify what the current reduction looks like, and where it is coming from
* at the time of landing, it was unclear how verify that the pre and post minified ASTs agree during Android builds.
glandium, gps: what do you think about flipping the switch for Fennec?
Attachment #8457738 -
Flags: review?(nalexander)
Attachment #8457738 -
Flags: review?(mh+mozilla)
Attachment #8457738 -
Flags: review?(gps)
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #2)
> Comment on attachment 8457738 [details] [diff] [review]
> Turn on minification
>
> Review of attachment 8457738 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> There is a ton of discussion about this in
> https://bugzilla.mozilla.org/show_bug.cgi?id=903149.
>
> Reading that, the following things seem most important:
>
> * an early note suggested that Fennec's APK shrank "only" 400kb; we should
> verify what the current reduction looks like, and where it is coming from
>
> * at the time of landing, it was unclear how verify that the pre and post
> minified ASTs agree during Android builds.
I forgot that JS stacks get mangled as well. We'd want to work out our debugging story here, possibly with jchen.
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #2)
> Comment on attachment 8457738 [details] [diff] [review]
> Turn on minification
>
> Review of attachment 8457738 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> There is a ton of discussion about this in
> https://bugzilla.mozilla.org/show_bug.cgi?id=903149.
>
> Reading that, the following things seem most important:
>
> * an early note suggested that Fennec's APK shrank "only" 400kb; we should
> verify what the current reduction looks like, and where it is coming from
The linked try build's APK (that's a single-locale en-US, non-debug build) is
32096922 bytes
The built APK at http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2014-07-16-03-02-02-mozilla-central-android/en-US/ is
33455758 bytes
That's a tidy (!) Christmas of 33455758 - 32096922 = 1358836 bytes.
Comment 5•10 years ago
|
||
Pros:
* Reduced APK size
* Possibly increased performance due to smaller JS size
Cons:
* Stacks mangled
* Local source doesn't match debugged source
* Increased developer confusion and difficulty
* Minification increases package generation time
* Processes that rely on stack examination (anything in crash/metrics land?) will suffer
Perhaps we could put MOZ_PACKAGER_MINIFY_JS behind MOZ_AUTOMATION so local development isn't impacted. After all, minification *should* have no impact besides rewriting (we verify the AST before and after is identical - whitespace and comments aren't captured in the AST).
To me, this really boils down to a discussion in Fennec land between Fennec developers (who will likely lose) and whoever is fighting for the Fennec users (who will likely benefit).
Comment 6•10 years ago
|
||
The trick will be making tests continue to pass, judging by Try.
https://tbpl.mozilla.org/?tree=Try&rev=9b86a7242f2d
includes a lot of nonsense like
tests/ecma/ExecutionContexts/10.2.1.js:40: NS_ERROR_FACTORY_NOT_REGISTERED: item 2
(In reply to Gregory Szorc [:gps] from comment #5)
> * Reduced APK size
> * Possibly increased performance due to smaller JS size
Plus the joyful memory consumption and speed improvements. Bug 903149 Comment 0 implies a 10% reduction in resident set. Even half that would be a big deal on mobile.
> To me, this really boils down to a discussion in Fennec land between Fennec
> developers (who will likely lose) and whoever is fighting for the Fennec
> users (who will likely benefit).
Honestly, for a multi-MB package saving and any kind of non-trivial RAM improvement -- especially if it doesn't affect developer builds -- I can't see us saying no.
Assignee: chriskitching → nobody
Blocks: fennec-mem
Component: General → Build Config & IDE Support
Keywords: perf
Reporter | ||
Comment 7•10 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #4)
> That's a tidy (!) Christmas of 33455758 - 32096922 = 1358836 bytes.
The apk I get building locally is 28027333 bytes. 34865727 without the patch.
Look:
Small: https://www.dropbox.com/s/jay0qbj37m0930m/small.apk
Large: https://www.dropbox.com/s/rwbazd9jo02suby/fennec-33.0a1.multi.android-arm.apk
I'm not going crazy. Honest!
Why is there such a difference between my local build and automation? That may prove a useful thing to know, since it's making a big difference... whatever it is.
(In reply to Gregory Szorc [:gps] from comment #5)
> Perhaps we could put MOZ_PACKAGER_MINIFY_JS behind MOZ_AUTOMATION so local
> development isn't impacted. After all, minification *should* have no impact
> besides rewriting (we verify the AST before and after is identical -
> whitespace and comments aren't captured in the AST).
Looks like we found an edgecase: It's all going orange :/
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #6)
> The trick will be making tests continue to pass, judging by Try.
>
> https://tbpl.mozilla.org/?tree=Try&rev=9b86a7242f2d
>
> includes a lot of nonsense like
>
> tests/ecma/ExecutionContexts/10.2.1.js:40: NS_ERROR_FACTORY_NOT_REGISTERED:
> item 2
Yeah, I saw this too. I'd sacrifice some time to understand what's going on here.
> (In reply to Gregory Szorc [:gps] from comment #5)
>
> > * Reduced APK size
> > * Possibly increased performance due to smaller JS size
>
> Plus the joyful memory consumption and speed improvements. Bug 903149
> Comment 0 implies a 10% reduction in resident set. Even half that would be a
> big deal on mobile.
We'd need to measure. My APK comparison and ckitching's already disagreed; resident set is a good deal harder to interpret :)
> > To me, this really boils down to a discussion in Fennec land between Fennec
> > developers (who will likely lose) and whoever is fighting for the Fennec
> > users (who will likely benefit).
I'm encouraged that you (gps) have no strong build system or structural complaints; just trade-offs in developer ergonomics.
> Honestly, for a multi-MB package saving and any kind of non-trivial RAM
> improvement -- especially if it doesn't affect developer builds -- I can't
> see us saying no.
At the moment, I'm with rnewman. 1+ MB reduction in our package size is a huge win. I'd want to broaden this discussion, however -- especially drawing in our crash and JS stack handling groups. Would this impact our hang reporting? Would this impact the profiler? How about remote debugging?
Assignee | ||
Updated•10 years ago
|
Summary: Minification of JS can reduce APK size by 6MB → Minify the Fennec JS code
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Chris Kitching [:ckitching] from comment #7)
> (In reply to Nick Alexander :nalexander from comment #4)
> > That's a tidy (!) Christmas of 33455758 - 32096922 = 1358836 bytes.
>
> The apk I get building locally is 28027333 bytes. 34865727 without the patch.
> Look:
>
> Small: https://www.dropbox.com/s/jay0qbj37m0930m/small.apk
> Large:
> https://www.dropbox.com/s/rwbazd9jo02suby/fennec-33.0a1.multi.android-arm.apk
The small build is a single-locale (en-US) developer build; that's a good deal of the difference. And the omni.ja will be packed with multi-locale translations as well, so both figures will be out of whack.
Still, 1+ Mb of savings is very promising! Thanks for starting this discussion!
Comment 10•10 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #5)
> Perhaps we could put MOZ_PACKAGER_MINIFY_JS behind MOZ_AUTOMATION so local
> development isn't impacted. After all, minification *should* have no impact
> besides rewriting (we verify the AST before and after is identical -
> whitespace and comments aren't captured in the AST).
Except that's only done when there's an executable js shell, which, for android, we don't have.
Comment 11•10 years ago
|
||
Some sizes from the APK of the try build and the nightly:
Total uncompressed size of *.js/*.jsm:
try: 7285042
nightly: 12971156
Biggest wins:
chrome/shumway/content/shumway.js: 1656505 -> 1082778
chrome/chrome/content/browser.js: 296185 -> 181977
modules/addons/XPIProvider.jsm: 260370 -> 137774
chrome/shumway/content/shumway-worker.js: 184352 -> 131572
Comment 12•10 years ago
|
||
Comment on attachment 8457738 [details] [diff] [review]
Turn on minification
Review of attachment 8457738 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/installer/Makefile.in
@@ +28,5 @@
> MOZ_PKG_MANIFEST = package-manifest
> endif
>
> MOZ_PACKAGER_MINIFY=1
> +MOZ_PACKAGER_MINIFY_JS=1
As said in comment 10, you need a JS_BINARY.
Attachment #8457738 -
Flags: review?(mh+mozilla)
Attachment #8457738 -
Flags: review?(gps)
Attachment #8457738 -
Flags: review-
Comment 13•10 years ago
|
||
BTW, I see in device build logs that b2g builds are not using a js binary either. I don't know how they end up with something working.
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #13)
> BTW, I see in device build logs that b2g builds are not using a js binary
> either. I don't know how they end up with something working.
Yeah, this was the second bullet of my https://bugzilla.mozilla.org/show_bug.cgi?id=1039902#c2. We don't have to be perfect here:
* we could fetch a known js binary from somewhere on buildbots during the build
* test jobs already install a host-appropriate xpcshell, which we could leverage. We could package both a regular and a minified omni.ja into the test archive, and verify as part of one of the test jobs.
Comment 15•10 years ago
|
||
We should test remote debugging, but I assume that it's working fine if b2g is minifying. Also, the remote debugging tools have "prettifying" source code options too.
I'd be OK with landing something that did not minify JS for local developer builds, making logcat output easier to match up to source code. That said, it's not perfect now since we preprocess JS files.
Comment 16•10 years ago
|
||
Comment 17•10 years ago
|
||
This will probably make profiling harder, but I don't know how many people actually profile chrome JS.
For hang monitoring, it won't break it, but the results will be harder to decipher. Hang monitoring is enabled on aurora and nightly, so I think leaving nightly alone and minifying on aurora and above would be a good compromise.
Comment 18•10 years ago
|
||
(In reply to Jim Chen [:jchen :nchen] from comment #17)
> For hang monitoring, it won't break it, but the results will be harder to
> decipher. Hang monitoring is enabled on aurora and nightly, so I think
> leaving nightly alone and minifying on aurora and above would be a good
> compromise.
I can agree with this logic. We already add debug symbols to Nightly to make it easier to profile.
Comment 19•10 years ago
|
||
Nick and I just discussed in brief. He might comment further, but here are my notes.
I think there's been some confusion in this bug between:
* Minifying JS, which requires only our Python build environment, and
* Verifying the minified JS, which requires a JS interpreter to load and compare the ASTs of two chunks of JS.
Nick and I would like to suggest making MOZ_PACKAGER_MINIFY_JS mean what it says -- just minify JS -- and then to define a testing phase that verifies the minified JS.
We know we have a JS interpreter available during testing (and if not it should be easy enough to add), so this might be an easy-ish way out.
Assignee | ||
Comment 20•10 years ago
|
||
I'd like to move on this for the 36 cycle. To start, I'm going to
refresh some context.
The minification, using jsmin, is already in the tree, but using it
safely requires verifying that the minification has not changed the JS
AST. I see 4 options for verification:
1) do not verify;
2) build a js binary as part of the mobile/android build;
3) download a pre-built js binary;
4) verify, but not at build time.
1) was ruled out by jorendorff at https://bugzilla.mozilla.org/show_bug.cgi?id=903149#c58.
2) is really hard given our current build system.
3) gps already explained why a pre-built js binary is insufficient at https://bugzilla.mozilla.org/show_bug.cgi?id=903149#c59
That leaves 4). What I propose is that we make the verification step
part of the Fennec test jobs. We can either add a new job or find a way
to do what is necessary as part of an existing job.
glandium, gps: do you see some problem with option 4?
Assignee: nobody → nalexander
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(gps)
Assignee | ||
Comment 21•10 years ago
|
||
Assuming that we're okay with verifying during a test job, I have a plan
for how to build that. For other reasons, we already want to upload a
second debug APK that does not have Proguard run against it. I propose
to start building and uploading such an APK, and for that debug APK to
have un-minified JS code in its omni.ja. So we build and upload:
release APK (Proguarded classes.dex, minified omni.ja)
debug APK (un-Proguarded classes.dex, un-minified omni.ja)
Our test job then uses the already-built Android jsshell and the two
uploaded APKs to iterate the JS code and make sure it verifies on
device.
Assignee | ||
Comment 22•10 years ago
|
||
Before moving on any of this, I will re-run the minifying experiments locally to see what the current size reduction is. It might not be worth it at this point.
Comment 23•10 years ago
|
||
What happens with the minification verification is that when one fails, the corresponding file is *not* minified, as opposed to fail the build entirely. There are many files that end up in that state. Moving that out of the build process is not possible, short of having a list of files not to minify, and I'd rather avoid having to maintain such a list, especially when there are alternatives.
That is, contrary to what you claim, 2) is not "really hard given our current build system". In fact, we're already doing it for some of the analysis builds (iirc it's either hazard or root analysis builds) That "just" relies on a different build script, probably using mozharness. There are other ways our build system can cope with that without mozharness too, like adding support for --enable-application=js and using MOZ_BUILD_PROJECTS to build js and mobile/android, but that also requires cooperation from the buildbot-side build script.
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 24•10 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #23)
> What happens with the minification verification is that when one fails, the
> corresponding file is *not* minified, as opposed to fail the build entirely.
> There are many files that end up in that state. Moving that out of the build
> process is not possible, short of having a list of files not to minify, and
> I'd rather avoid having to maintain such a list, especially when there are
> alternatives.
I started to dig into this locally, and I'm seeing roughly 10 of 900 JS/JSM files failing the existing AST comparison on my device. At least one of those files is shumway, which we don't care about (yet) on Fennec.
I propose that we try to understand how our minification is failing on these files; with a little luck, we can always minify at build time (but fail the tests when minification fails) and avoid such a list, which I agree is not good for business.
> That is, contrary to what you claim, 2) is not "really hard given our
> current build system". In fact, we're already doing it for some of the
> analysis builds (iirc it's either hazard or root analysis builds) That
> "just" relies on a different build script, probably using mozharness. There
> are other ways our build system can cope with that without mozharness too,
> like adding support for --enable-application=js and using MOZ_BUILD_PROJECTS
> to build js and mobile/android, but that also requires cooperation from the
> buildbot-side build script.
I should have been more precise: it's really hard for *me* to drive building a hosted jsshell as part of our cross compiled Fennec builds. But even if this was trivial, it would not address the fact that the hosted jsshell and the native jsshell might not agree (which gps and I both raised in point 3).
So, I propose to spend some time understanding why jsmin fails on the few files that it does (for example, I think our in-tree copy is behind upstream). Once we understand better the number and complexity of the differences between the minified and un-minified ASTs we can make a better informed choice between our options. Sound reasonable?
Assignee | ||
Comment 25•10 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #24)
> (In reply to Mike Hommey [:glandium] from comment #23)
> > What happens with the minification verification is that when one fails, the
> > corresponding file is *not* minified, as opposed to fail the build entirely.
> > There are many files that end up in that state. Moving that out of the build
> > process is not possible, short of having a list of files not to minify, and
> > I'd rather avoid having to maintain such a list, especially when there are
> > alternatives.
>
> I started to dig into this locally, and I'm seeing roughly 10 of 900 JS/JSM
> files failing the existing AST comparison on my device. At least one of
> those files is shumway, which we don't care about (yet) on Fennec.
With the latest jsmin (https://bitbucket.org/dcs/jsmin/commits/ee072c47cb30), I have 5/906 files failing on my device:
Summary
=======
Ran 906 tests
Expected results: 901
Unexpected results: 5 (FAIL: 5)
Unexpected Results
==================
FAIL chrome/toolkit/content/global/aboutwebrtc/aboutWebrtc.js
FAIL components/nsContentPrefService.js
FAIL modules/ContentPrefService2.jsm
FAIL modules/commonjs/sdk/deprecated/unit-test.js
FAIL modules/commonjs/sdk/test/httpd.js
Comment 26•10 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #24)
> I should have been more precise: it's really hard for *me* to drive building
> a hosted jsshell as part of our cross compiled Fennec builds. But even if
> this was trivial, it would not address the fact that the hosted jsshell and
> the native jsshell might not agree (which gps and I both raised in point 3).
I don't see why a host and a native js shell built from the same source would not agree on js syntax.
Assignee | ||
Comment 27•10 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #26)
> (In reply to Nick Alexander :nalexander from comment #24)
> > I should have been more precise: it's really hard for *me* to drive building
> > a hosted jsshell as part of our cross compiled Fennec builds. But even if
> > this was trivial, it would not address the fact that the hosted jsshell and
> > the native jsshell might not agree (which gps and I both raised in point 3).
>
> I don't see why a host and a native js shell built from the same source
> would not agree on js syntax.
I agree that it's unlikely, but it's still a possibility. As I referenced earlier, gps had the same opinion. Perhaps a SM dev would have a strong opinion one way or the other?
Comment 28•10 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #27)
> I agree that it's unlikely, but it's still a possibility. As I referenced
> earlier, gps had the same opinion. Perhaps a SM dev would have a strong
> opinion one way or the other?
As I read it, gps's concern with a downloaded version, which would not necessarily match the local version. Obviously, if you build both from the same source, this problem doesn't exist.
I'll add that if we *do* have differences in expectation in js syntax between the same source as built for arm or built for x86, then we have a *serious* problem in the js engine.
Comment 29•10 years ago
|
||
And even if we don't have such differences, if that's still a possibility, then there's a *serious* problem.
I doubt that's possible.
Comment 30•10 years ago
|
||
A lot has changed since https://bugzilla.mozilla.org/show_bug.cgi?id=903149#c59!
What hasn't changed is that the quality of jsmin pales in comparison to available JS minifiers. JS minifiers such as Esprima construct a full AST. jsmin's parsing is far from robust, which is why it emits invalid JS. I have doubts jsmin is going to scale and evolve with the JS language. So using JavaScript to minify JavaScript should be our long-term goal.
With all the work that Release Engineering is apparently doing around Docker containers, etc, I wonder if we should revisit the assumption that obtaining a host SM binary will be difficult. If we can instantiate multiple toolchain architectures from the build, we should be able to produce a SM binary easily enough. One of my concerns around doing this was build times: dual SM builds slows us down. Now that we have s3 ccache, the cost for building a redundant SM should be low.
Yet, standing this all up would likely be a major time sink. I don't think it is practical for the 36 timeline.
I thought jorendorff said that the churn in new AST elements would likely be low and that the risk for an outdated SM binary encountering a new AST element would therefore be low. This mostly assuaged my concerns of using a pre-built SM binary for performing minification.
I'd support defining a host SM binary to perform minification via the tooltool manifest. We will eventually get a build or minification failure due to some new AST element being introduced. That will be extremely annoying as it will prevent anything using that AST element from landing until a new SM binary is deployed. So we need to be sure the procedure for doing that is fast and simple. (I think it should be - find a package from a recent m-c build, upload to tooltool, and update the tooltool manifests - or am I missing a critical step?)
Flags: needinfo?(gps) → needinfo?(jorendorff)
Comment 31•10 years ago
|
||
I'm not sure I understand the question. Why do we need a pre-built SM binary? Isn't building a new standalone SM binary already a really early step in the build process?
Uploading a new build to tooltool doesn't sound hard, but it seems like we shouldn't have the possibility of those two things getting out of sync to begin with.
Flags: needinfo?(jorendorff)
Comment 32•10 years ago
|
||
Ideally, yes, we'd build a host-native SM binary every build. But this is likely a tough ask given the current capabilities of our build infrastructure.
As a stop gap until our automation improves, we'll upload an already built SM and make it configurable via tooltool so it can easily be changed.
The question is: how often will an old binary encounter an unreadable AST in Firefox code? i.e. how often are new AST nodes being introduced or refactored?
Flags: needinfo?(jorendorff)
Assignee | ||
Comment 33•10 years ago
|
||
With the PR at https://bitbucket.org/dcs/jsmin/pull-request/7/fix-for-issue-14-handle-ecmascript-harmony, all the Fennec JS minimified sources have identical ASTs to their un-minified originals:
Summary
=======
Ran 902 tests
Expected results: 902
Unexpected results: 0
OK
So we could make regress minification a build-time error (rather than silently ignoring the minified source).
Assignee | ||
Comment 34•10 years ago
|
||
(In reply to Jason Orendorff [:jorendorff] from comment #31)
> I'm not sure I understand the question. Why do we need a pre-built SM
> binary? Isn't building a new standalone SM binary already a really early
> step in the build process?
To elaborate on what gps said, we build a standalone native SM early on. For this packaging minification, we need a standalone host SM later.
> Uploading a new build to tooltool doesn't sound hard, but it seems like we
> shouldn't have the possibility of those two things getting out of sync to
> begin with.
Uploading a new build to tooltool seems pretty easy, once you have permissions:
https://wiki.mozilla.org/ReleaseEngineering/Applications/Tooltool#How_to_upload_files_to_tooltool
Assignee | ||
Comment 35•10 years ago
|
||
Assignee | ||
Comment 36•10 years ago
|
||
This patch adds a SpiderMonkey jsshell binary via tooltool and arranges
for it to be used in automation.
We'd prefer to build a host SpiderMonkey jsshell, but that's not trivial
right now; and we'd prefer to use a minifier better than the existing
Python jsmin (possibly written in JavaScript), but one step at a time.
Try run build is green, and I see a small omni.ja containing minified
sources \o/
-rw-rw-rw- 7942784 29-Oct-2014 14:37:18 assets/omni.ja
Attachment #8457738 -
Attachment is obsolete: true
Attachment #8513702 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 37•10 years ago
|
||
Comment 38•10 years ago
|
||
Comment on attachment 8513702 [details] [diff] [review]
Minify the Fennec JS code. r=glandium
Review of attachment 8513702 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/config/js_wrapper.sh
@@ +12,5 @@
> +topsrcdir=`cd \`dirname $0\`/../../..; pwd`
> +
> +JS_BINARY="$topsrcdir/jsshell/js"
> +
> +LD_LIBRARY_PATH="$topsrcdir/jsshell:$LD_LIBRARY_PATH"
LD_LIBRARY_PATH="$topsrcdir/jsshell${LD_LIBRARY_PATH+:$LD_LIBRARY_PATH}"
then
export LD_LIBRARY_PATH
@@ +15,5 @@
> +
> +LD_LIBRARY_PATH="$topsrcdir/jsshell:$LD_LIBRARY_PATH"
> +
> +# Pass through all arguments and exit with status from js shell.
> +$JS_BINARY "$@"
exec "$JS_BINARY" "$@"
::: mobile/android/config/mozconfigs/common
@@ +53,5 @@
> mk_add_options "export ANT_HOME=$topsrcdir/apache-ant"
>
> mk_add_options "export PATH=$topsrcdir/gcc/bin:$topsrcdir/apache-ant/bin:$PATH"
> +
> +export JS_BINARY="$topsrcdir/mobile/android/config/js_wrapper.sh"
No need to export, here.
Attachment #8513702 -
Flags: review?(mh+mozilla) → review+
Comment 39•10 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #32)
> The question is: how often will an old binary encounter an unreadable AST in
> Firefox code? i.e. how often are new AST nodes being introduced or
> refactored?
Annoyingly often. Let's say once every six weeks on average. We add new syntax to JS about that often, and each new feature eventually gets used. For example:
- We got template strings this summer.
alert(`template strings are ${adjective}!`);
- I'm reviewing a patch for "default values in destructuring" today.
var {name="anonymous"} = possiblyNamedObject;
- After that, I think ES6 classes are next in line.
class Animal { roar() { console.log("raar!"); } }
new Animal().roar();
- But we will also be changing the AST for global 'let', mmmmaybe this year.
let x = 3; // currently the same AST as var; this is a bug
Flags: needinfo?(jorendorff)
Assignee | ||
Comment 40•10 years ago
|
||
Comment 41•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Assignee | ||
Comment 42•10 years ago
|
||
Comment on attachment 8513702 [details] [diff] [review]
Minify the Fennec JS code. r=glandium
Approval Request Comment
[Feature/regressing bug #]: minifying Fennec JS. This is build time only. In Bug 1093718, we've done the testing in Nightly that this works; it's time to uplift to Aurora and revert Nightly to the debugging friendly (un-minified) state.
[User impact if declined]: none.
[Describe test coverage new/current, TBPL]: it's been happy on TBPL for Nightly for a few days.
[Risks and why]: very, very low. This is a build time thing, and we're *very* careful about not changing the JS in a substantive way at build time. I expect 0 fallout if the build succeeds.
[String/UUID change made/needed]: none.
Attachment #8513702 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8513702 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 43•10 years ago
|
||
Hi sheriff -- have I mentioned how good looking y'all are? Could you uplift https://hg.mozilla.org/mozilla-central/rev/35a4be7fb0a8 to mozilla-aurora when you make a pass. Thanks!
Keywords: checkin-needed
Comment 44•10 years ago
|
||
Assignee | ||
Comment 45•10 years ago
|
||
mfinkle: the builds you were looking at were too recent. I see:
chocho:Downloads nalexander$ curl -I http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2014-11-05-00-40-06-mozilla-aurora-android/fennec-35.0a2.multi.android-arm.apk
Content-Length: 41861266
chocho:Downloads nalexander$ curl -I http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2014-11-06-00-40-01-mozilla-aurora-android/fennec-35.0a2.multi.android-arm.apk
Content-Length: 40559134
So (- 41861266 40559134) is a change of 1302132b ~= 1.3Megs.
Updated•5 years ago
|
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 36 → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•