size of the windows installer now over 13MB

RESOLVED WONTFIX

Status

()

Firefox
General
RESOLVED WONTFIX
7 years ago
3 years ago

People

(Reporter: chris hofmann, Assigned: mwu)

Tracking

(Depends on: 1 bug, {memory-footprint})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 10 obsolete attachments)

1.42 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
8.44 KB, patch
ted
: review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
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
It's because bug 633645 added startup cache (roughly 4MB after unpacking omni.jar).
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.
(Assignee)

Comment 3

7 years ago
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.
Sure, but it's stuff we have to transfer over the wire (and through updates).
(Assignee)

Comment 5

7 years ago
Created attachment 525126 [details] [diff] [review]
Remove js files that were serialized
Assignee: nobody → mwu
Attachment #525126 - Flags: review?(khuey)
(Assignee)

Updated

7 years ago
Attachment #525126 - Flags: review?(tglek)
(Assignee)

Comment 6

7 years ago
Created attachment 525128 [details] [diff] [review]
Remove js files that were serialized, v2
Attachment #525126 - Attachment is obsolete: true
Attachment #525126 - Flags: review?(tglek)
Attachment #525126 - Flags: review?(khuey)
Attachment #525128 - Flags: review?(tglek)
Attachment #525128 - Flags: review?(khuey)

Comment 7

7 years ago
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
Attachment #525128 - Flags: review?(tglek) → review+
(Assignee)

Comment 8

7 years ago
Created attachment 525175 [details] [diff] [review]
Remove js files that were serialized, v3

review comments addressed.
Attachment #525128 - Attachment is obsolete: true
Attachment #525128 - Flags: review?(khuey)
Attachment #525175 - Flags: review?(khuey)
(Assignee)

Comment 9

7 years ago
Created attachment 525188 [details] [diff] [review]
hg patch ready patch
Attachment #525175 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Keywords: checkin-needed

Comment 10

7 years ago
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>.
Keywords: checkin-needed

Updated

7 years ago
Depends on: 631392

Comment 11

7 years ago
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

7 years ago
(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.
(Assignee)

Comment 13

7 years ago
Created attachment 528477 [details] [diff] [review]
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.
Attachment #528477 - Flags: review?(bent.mozilla)
Attachment #528477 - Flags: review?(bent.mozilla) → review+

Comment 14

7 years ago
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

7 years ago
Brilliant, you just spotted an easy/fun optimization. I filed bug 653143 for it.
Depends on: 653143
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.
(Assignee)

Comment 17

7 years ago
Created attachment 528949 [details] [diff] [review]
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.
Attachment #525188 - Attachment is obsolete: true
Attachment #528949 - Flags: review?(tglek)
(Assignee)

Comment 18

7 years ago
Created attachment 528966 [details] [diff] [review]
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.
Attachment #528949 - Attachment is obsolete: true
Attachment #528949 - Flags: review?(tglek)
Attachment #528966 - Flags: review?(tglek)

Comment 19

7 years ago
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?
Attachment #528966 - Flags: review?(tglek) → review+
(Assignee)

Comment 20

7 years ago
(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.
(Assignee)

Comment 21

7 years ago
Created attachment 528988 [details] [diff] [review]
Remove js files that were serialized, v6
Attachment #528966 - Attachment is obsolete: true
Attachment #528988 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 22

7 years ago
Created attachment 528989 [details] [diff] [review]
Remove js files that were serialized, v7

forgot to run qref.
Attachment #528988 - Attachment is obsolete: true
Attachment #528988 - Flags: review?(ted.mielczarek)
Attachment #528989 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 23

7 years ago
Created attachment 529852 [details] [diff] [review]
Remove js files that were serialized, v8

Needed to make some changes to make windows happy.
Attachment #528989 - Attachment is obsolete: true
Attachment #528989 - Flags: review?(ted.mielczarek)
Attachment #529852 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 24

7 years ago
Created attachment 529855 [details] [diff] [review]
Interdiff between version 7 and 8

Added a small change to have the gecko runtime figure out the absolute path itself.
Attachment #529855 - Flags: review?(tglek)

Comment 25

7 years ago
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.
Attachment #529855 - Flags: review?(tglek) → review+
(Assignee)

Comment 26

7 years ago
Created attachment 531398 [details] [diff] [review]
Remove js files that were serialized, v9

Unbitrotted
Attachment #529852 - Attachment is obsolete: true
Attachment #529855 - Attachment is obsolete: true
Attachment #529852 - Flags: review?(ted.mielczarek)
Attachment #531398 - Flags: review?(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?
Attachment #531398 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 28

7 years ago
(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)
Depends on: 659407
(Assignee)

Updated

7 years ago
Depends on: 660325
(Assignee)

Updated

7 years ago
Attachment #528477 - Attachment description: Stop shipping WorkerTest.jsm and friends → Stop shipping WorkerTest.jsm and friends (checked in in bug 660325)

Updated

7 years ago
Depends on: 706100

Updated

7 years ago
No longer depends on: 706100

Updated

7 years ago
Depends on: 706103

Updated

7 years ago
Depends on: 711811
See bug 711811 for a not-really-trivial way of shaving off 1.5-2MB from the Windows installer.

Comment 31

7 years ago
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.
(Reporter)

Comment 32

6 years ago
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

6 years ago
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

6 years ago
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.
(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.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.