647453 - size of the windows installer now over 13MB

Status: RESOLVED WONTFIX

(Firefox :: General, defect)

Description

[chris hofmann]

not sure if we were just teetering under threshold or took a significant bump around 3/23, but this would be a bad path to be on and something we need to watch closer with short development cycles and not much time to respond.
 


http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2011-03-22-03-mozilla-central/
[ ]	Firefox Setup 4.0.exe	18-Mar-2011 10:53 	12M


http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2011-03-23-03-mozilla-central/

	firefox-4.0b13pre.en-US.win32.installer.exe	23-Mar-2011 06:13 	13M

Comment 1

[Masatoshi Kimura [:emk]]

It's because bug 633645 added startup cache (roughly 4MB after unpacking omni.jar).

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Yeah, we've added a bunch of gfx DLLs and the startup cache recently ...

I suppose the real question here is whether Bug 633645's additional size is worth the perf increase on (first-run only?) startup.

Comment 3

[Michael Wu [:mwu]]

Note that once installed, the bundled cache has no net effect on footprint on the user's system. The cache is going to be generated if it isn't there. I also wrote a number of patches to reduce the size footprint of the cache, and there is an additional patch in my queue that allows us to delete all the js files which are already compiled.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Sure, but it's stuff we have to transfer over the wire (and through updates).

Comment 5

[Michael Wu [:mwu]]

Attachment: Remove js files that were serialized

Comment 6

[Michael Wu [:mwu]]

Attachment: Remove js files that were serialized, v2

Comment 7

[(dormant account)]

Comment on attachment 525128 [details] [diff] [review]
Remove js files that were serialized, v2

stream.init(logFile, 0x02 | 0x08 | 0x20, 0666, 0); <-- need pretty constant names

STRIP_ORIGINAL_FILES = zip -d omni.jar `cat sc_log.txt` <-- Need to put in a comment like "the script outputs list of compiled js files[which is later used to remove them from packaging]"

r+ with that

Comment 8

[Michael Wu [:mwu]]

Attachment: Remove js files that were serialized, v3

review comments addressed.

Comment 9

[Michael Wu [:mwu]]

Attachment: hg patch ready patch

Comment 10

[(no longer active)]

This breaks packaging completely.  Landed on cedar <http://hg.mozilla.org/projects/cedar/rev/3b7b6b0bd9ef> then backed out immediately <http://hg.mozilla.org/projects/cedar/rev/6423303adcd0>.

Comment 11

[Frédéric Kayser]

I just filled Bug 652967, is there way to check if files enclosed in omni.jar are actually used or are just dead load?

Comment 12

[(dormant account)]

(In reply to comment #11)
> I just filled Bug 652967, is there way to check if files enclosed in omni.jar
> are actually used or are just dead load?

Good catch. There isn't a way to determine that something is not used.

You can do set the environment variable MOZ_JAR_LOG_DIR to point to some directory and run firefox for a while to get a log of what files were used during the session.

Comment 13

[Michael Wu [:mwu]]

Attachment: Stop shipping WorkerTest.jsm and friends (checked in in bug 660325)

JSMs don't have to be in resources://gre/modules/, so don't ship them. Shipping these files causes a test failure with the js stripping patch in this bug.

Comment 14

[Frédéric Kayser]

There are a lot of duplicate files in the Windows omni.jar.

If you sort the files based on their CRC-32:
unzip -v omni.jar | grep skin | grep -v 00000000 | colrm 1 48 > skin-listing.txt
sort skin-listing.txt

You get this:
0012592e  chrome/toolkit/skin/classic/aero/global/aboutCacheEntry.css
0012592e  chrome/toolkit/skin/classic/global/aboutCacheEntry.css
014071fd  chrome/toolkit/skin/classic/aero/global/tree.css
0180cb0c  chrome/toolkit/skin/classic/aero/global/customizeToolbar.css
0180cb0c  chrome/toolkit/skin/classic/global/customizeToolbar.css
01cbce26  chrome/toolkit/skin/classic/aero/mozapps/xpinstall/xpinstallConfirm.css
01cbce26  chrome/toolkit/skin/classic/mozapps/xpinstall/xpinstallConfirm.css
01fa4eb1  chrome/toolkit/skin/classic/aero/global/radio/radio-check.gif
01fa4eb1  chrome/toolkit/skin/classic/global/radio/radio-check.gif
01ff39fd  chrome/toolkit/skin/classic/aero/mozapps/viewsource/viewsource.css
01ff39fd  chrome/toolkit/skin/classic/mozapps/viewsource/viewsource.css
05c2d87f  chrome/toolkit/skin/classic/aero/mozapps/extensions/category-languages.png
05c2d87f  chrome/toolkit/skin/classic/aero/mozapps/extensions/localeGeneric.png
06a9fbd5  chrome/toolkit/skin/classic/aero/global/icons/error-48.png
071e5938  chrome/browser/skin/classic/browser/places/bookmarksToolbar.png
07d0388a  chrome/browser/skin/classic/aero/browser/livemark-folder.png
091e64ce  chrome/toolkit/skin/classic/aero/global/toolbar.css
091e64ce  chrome/toolkit/skin/classic/global/toolbar.css
0a01e6e1  chrome/browser/skin/classic/aero/browser/places/tag.png
0a01e6e1  chrome/toolkit/skin/classic/aero/mozapps/places/tagContainerIcon.png
0ae5e1b5  chrome/browser/skin/classic/aero/browser/mainwindow-dropdown-arrow.png
0c1f2642  chrome/browser/skin/classic/aero/browser/places/organizer.css
0c6f6023  chrome/toolkit/skin/classic/aero/mozapps/extensions/eula.css
0c6f6023  chrome/toolkit/skin/classic/mozapps/extensions/eula.css
0d79fab6  chrome/toolkit/skin/classic/global/dirListing/folder.png
0d902d35  chrome/browser/skin/classic/aero/browser/places/unsortedBookmarks.png
0e0376fc  chrome/toolkit/skin/classic/aero/global/arrow/panelarrow-horiz.png
0e41214b  chrome/toolkit/skin/classic/aero/mozapps/update/updates.css
0e41214b  chrome/toolkit/skin/classic/mozapps/update/updates.css
0e5f590f  chrome/browser/skin/classic/browser/places/bookmarksMenu.png
0f3c3fe7  chrome/toolkit/skin/classic/aero/global/arrow/panelarrow-up.png
0f9387cf  chrome/toolkit/skin/classic/global/arrow/panelarrow-up.png
0f9560fd  chrome/toolkit/skin/classic/aero/mozapps/extensions/category-search.png
0f9560fd  chrome/toolkit/skin/classic/mozapps/extensions/category-search.png
0fd2bae8  chrome/toolkit/skin/classic/aero/global/passwordmgr.css
0fd2bae8  chrome/toolkit/skin/classic/global/passwordmgr.css
...
Well you get the idea, many files are both in an aero and a non-aero folder.

Comment 15

[(dormant account)]

Brilliant, you just spotted an easy/fun optimization. I filed bug 653143 for it.

Comment 16

[Justin Dolske [:Dolske]]

All the aero/non-aero ones should be fixable right now via jar.mn. There are already lots of cases where we make both flavors refer to the same file in the tree.

Comment 17

[Michael Wu [:mwu]]

Attachment: Remove js files that were serialized, v4

Two changes:
1. MOZ_STARTUP_CACHE is set by the build system instead of the precompile script. This is so we don't have to worry about startup cache initializing before we get to set MOZ_STARTUP_CACHE.
2. Make StartupCache always attempt to initialize to a read only mode. This fixes xpcshell tests since there isn't always a profile to use.

Comment 18

[Michael Wu [:mwu]]

Attachment: Remove js files that were serialized, v5

Addresses IRC review comments:
1. Avoid calling code that requires mFile
2. Warn if a child tries to use the startup cache.
3. Add comments about this omnijar readonly mode.

Comment 19

[(dormant account)]

Comment on attachment 528966 [details] [diff] [review]
Remove js files that were serialized, v5


>   NS_ASSERTION(NS_IsMainThread(), "Startup cache only available on main thread");
>+  if (XRE_GetProcessType() != GeckoProcessType_Default) {
>+    NS_WARNING("Bug 627148 - Should not load JS components in the content process");
>+  }
>+

wonder if this should be an assertion

>@@ -344,7 +355,7 @@ StartupCache::WriteToDisk()
>     return;
> 
>   nsCOMPtr<nsIZipWriter> zipW = do_CreateInstance("@mozilla.org/zipwriter;1");
>-  if (!zipW)
>+  if (!zipW || !mFile)
>     return;


I think this guard belongs in ~StartupCache

> 
>   rv = zipW->Open(mFile, PR_RDWR | PR_CREATE_FILE);
>@@ -380,6 +391,8 @@ StartupCache::WriteToDisk()
> void
> StartupCache::InvalidateCache() 
> {
>+  if (!mFile)
>+    return;

I don't think you need this since the observer + only other callsite for this code wont get called due to NS_ENSURE_TRUE(mFile, NS_OK);

Perhaps ted can look at your makefilery?

Comment 20

[Michael Wu [:mwu]]

(In reply to comment #19)
> >@@ -344,7 +355,7 @@ StartupCache::WriteToDisk()
> >     return;
> > 
> >   nsCOMPtr<nsIZipWriter> zipW = do_CreateInstance("@mozilla.org/zipwriter;1");
> >-  if (!zipW)
> >+  if (!zipW || !mFile)
> >     return;
> 
> 
> I think this guard belongs in ~StartupCache
> 

Oh I missed this one, thanks.

> > 
> >   rv = zipW->Open(mFile, PR_RDWR | PR_CREATE_FILE);
> >@@ -380,6 +391,8 @@ StartupCache::WriteToDisk()
> > void
> > StartupCache::InvalidateCache() 
> > {
> >+  if (!mFile)
> >+    return;
> 
> I don't think you need this since the observer + only other callsite for this
> code wont get called due to NS_ENSURE_TRUE(mFile, NS_OK);
> 

I left this in since this is appears to be a public interface.

Comment 21

[Michael Wu [:mwu]]

Attachment: Remove js files that were serialized, v6

Comment 22

[Michael Wu [:mwu]]

Attachment: Remove js files that were serialized, v7

forgot to run qref.

Comment 23

[Michael Wu [:mwu]]

Attachment: Remove js files that were serialized, v8

Needed to make some changes to make windows happy.

Comment 24

[Michael Wu [:mwu]]

Attachment: Interdiff between version 7 and 8

Added a small change to have the gecko runtime figure out the absolute path itself.

Comment 25

[(dormant account)]

Comment on attachment 529855 [details] [diff] [review]
Interdiff between version 7 and 8

GRE_DIR is a bit of a strange place for the cache, but since it's pre-packaging, I guess that's ok.

Comment 26

[Michael Wu [:mwu]]

Attachment: Remove js files that were serialized, v9

Unbitrotted

Comment 27

[(not currently active) Ted Mielczarek]

Comment on attachment 531398 [details] [diff] [review]
Remove js files that were serialized, v9

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

The patch looks fine implementation-wise, but what is this going to look like if someone clicks on a chrome error message in the error console? I know we talked about this in the past, but did you do anything, or is that just going to fail?

::: browser/installer/Makefile.in
@@ +122,4 @@
>  _ABS_RUN_TEST_PROGRAM = $(call core_abspath,$(RUN_TEST_PROGRAM))
>  endif
>  
> +ifndef MOZ_DEBUG

Is there a particular reason you're keeping these in debug builds?

Comment 28

[Michael Wu [:mwu]]

(In reply to comment #27)
> Comment on attachment 531398 [details] [diff] [review] [review]
> Remove js files that were serialized, v9
> 
> Review of attachment 531398 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> The patch looks fine implementation-wise, but what is this going to look
> like if someone clicks on a chrome error message in the error console? I
> know we talked about this in the past, but did you do anything, or is that
> just going to fail?
> 

It'll probably just fail. 

> ::: browser/installer/Makefile.in
> @@ +122,4 @@
> >  _ABS_RUN_TEST_PROGRAM = $(call core_abspath,$(RUN_TEST_PROGRAM))
> >  endif
> >  
> > +ifndef MOZ_DEBUG
> 
> Is there a particular reason you're keeping these in debug builds?

Just to make sure clicking on error messages in the error console works for developers running debug builds.

Do we want to do something about this before we turn this on? I can just land it with it turned off for now. (I have some patches depending on this one)

Comment 29

[chris hofmann]

looks like we are on the rise again after recent releases.

ftp://ftp.mozilla.org/pub/firefox/releases/3.6.24/win32/en-US/   7706 KB
ftp://ftp.mozilla.org/pub/firefox/releases/4.0.1/win32/en-US/	12229 KB 
ftp://ftp.mozilla.org/pub/firefox/releases/5.0.1/win32/en-US/	13366 KB
ftp://ftp.mozilla.org/pub/firefox/releases/6.0.1/win32/en-US/   13648 KB
ftp://ftp.mozilla.org/pub/firefox/releases/7.0.1/win32/en-US/   13717 KB
ftp://ftp.mozilla.org/pub/firefox/releases/8.0.1/win32/en-US/   14416 KB
ftp://ftp.mozilla.org/pub/firefox/releases/9.0b3/win32/en-US/   15494 KB

ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-aurora
                    /firefox-10.0a2.en-US.win32.installer.exe  	16125 KB 	

ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central/
                    firefox-11.0a1.en-US.win32.installer.exe    15743 KB

Comment 30

[Carlo Alberto Ferraris]

See bug 711811 for a not-really-trivial way of shaving off 1.5-2MB from the Windows installer.

Comment 31

[Frédéric Kayser]

Just wondering, does it make sens to include:
chrome/en-US/locale/en-US/global-platform/mac/*.properties
chrome/en-US/locale/en-US/global-platform/unix/*.properties
within omni.jar on Windows?

And the other way around, why include:
chrome/en-US/locale/en-US/global-platform/win/*.properties
on Mac OS X or Unix systems?

These files are only a few hundreds byte long.

Comment 32

[chris hofmann]

re: comment 29

we aren't making any progress.

over 16mb on firefox 13
ftp://ftp.mozilla.org/pub/firefox/releases/13.0/win32/en-US/

Name 	Size 	Last Modified
File:Firefox Setup 13.0.exe 	16186 KB 	6/1/12 	8:46:00 AM

Comment 33

[(dormant account)]

I think this is a lost cause, webrtc, skia, etc are huge code imports as of late. We'll only be adding more stuff for k9o, etc.

Comment 34

[Frédéric Kayser]

There is one new feature that could save a few bytes, Google WebP image support… bug 600919
Since most of the PNGs inside the omni.jar could be replaced by smaller WebP files, it would probably at least compensate for the added code.

The new 0.1.4 version of WebP (with lossless and lossy+alpha support) is not there yet (it's a question of days now).
Lossless WebP files are really smaller than PNG ones (20% is quiet usual), for instance once turned into a lossless WebP file about-logo.png weights 38098 bytes down from 55620 bytes (or 49502 bytes as it is languishing in bug 653001).

Chrome will get the 'new 2012' WebP support shortly, Android 5 will follow (it's a nice potential speed advantage compared to iOS/Safari), and from what I have seen web-designers could easily fall in love with the lossy+alpha capabilities of WebP, I hope Firefox will not be lagging too far behind…

And have you considered applying some sort of on the fly minification to the JavaScript files just before they are compressed in the omni.jar? This is a well known practice that can improve compression.

Comment 35

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Taras Glek (:taras) from comment #33)
> I think this is a lost cause, webrtc, skia, etc are huge code imports as of
> late. We'll only be adding more stuff for k9o, etc.

I agree.  We've lost this battle.