Closed Bug 647453 Opened 10 years ago Closed 9 years ago

size of the windows installer now over 13MB

Categories

(Firefox :: General, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: chofmann, Assigned: mwu)

References

(Depends on 1 open bug)

Details

(Keywords: memory-footprint)

Attachments

(2 files, 10 obsolete files)

1.42 KB, patch
bent.mozilla
: review+
Details | Diff | Splinter Review
8.44 KB, patch
ted
: review+
Details | Diff | Splinter Review
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.
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: nobody → mwu
Attachment #525126 - Flags: review?(khuey)
Attachment #525126 - Flags: review?(tglek)
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 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+
review comments addressed.
Attachment #525128 - Attachment is obsolete: true
Attachment #525128 - Flags: review?(khuey)
Attachment #525175 - Flags: review?(khuey)
Attached patch hg patch ready patch (obsolete) — Splinter Review
Attachment #525175 - Attachment is obsolete: true
Keywords: checkin-needed
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
Depends on: 631392
I just filled Bug 652967, is there way to check if files enclosed in omni.jar are actually used or are just dead load?
(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.
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+
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.
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.
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)
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 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+
(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.
Attachment #528966 - Attachment is obsolete: true
Attachment #528988 - Flags: review?(ted.mielczarek)
forgot to run qref.
Attachment #528988 - Attachment is obsolete: true
Attachment #528988 - Flags: review?(ted.mielczarek)
Attachment #528989 - Flags: review?(ted.mielczarek)
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)
Added a small change to have the gecko runtime figure out the absolute path itself.
Attachment #529855 - Flags: review?(tglek)
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+
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+
(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
Depends on: 660325
Attachment #528477 - Attachment description: Stop shipping WorkerTest.jsm and friends → Stop shipping WorkerTest.jsm and friends (checked in in bug 660325)
Depends on: 706100
No longer depends on: 706100
Depends on: 706103
Depends on: 711811
See bug 711811 for a not-really-trivial way of shaving off 1.5-2MB from the Windows installer.
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.
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
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.
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
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.