72551 - Opening chrome JARs inefficient

Status: RESOLVED FIXED

(Core :: Networking: JAR, defect)

Description

[Daniel Veditz [:dveditz]]

nsZipArchive reads too much and in too many small chunks when opening JAR 
archives, and we open 9 or so on startup. Averaging three runs after reboots I 
see about 2.8 seconds spent in BuildFileList(); on a second start it averages 
about .08 seconds on WinNT. On a first load the time is roughly proportional to 
the number of files in the archive, not the size of the archive (which makes 
sense, we're processing the archive's directory). One exception is en-US.jar 
which seems to take a disproportionate amount of time to load.

The worst thing libjar does is while reading the central directory it PR_Seeks 
back to the local header for each file, reads 30 bytes (to figure out the true 
offset of the file data), and then seeks back to the central directory to get 
the next file's data.

Instead we should 1) read and process the central directory in big chunks, and 
2) leave the file data offset unset until needed. This will eliminate 3 
PR_Reads and 2 PR_Seeks for each archived file -- thousands of OS calls 
eliminated.

I'm not sure how much of the 2.8 seconds on a first start can actually be saved 
though, since the bulk of the time (as judged by the difference preloading 
makes) seems to be in reading the file data.

Comment 1

[Daniel Veditz [:dveditz]]

nominating and tying into the master perf tracking bug

Comment 2

[Simon Fraser [no longer active]]

Attachment: Stack traces from PR_Read calls

Comment 3

[Simon Fraser [no longer active]]

The attachment shows stack traces from PR_Read calls, sorted by unique filenam & 
stack. Note that nsZipArchive::BuildFileList() is causing huge numbers of reads 
(2051 for modern.jar).

Comment 4

[Daniel Veditz [:dveditz]]

That's what I said when I opened this bug -- 3 reads and 2 seeks for each file 
in an archive. modern has 680 or so chrome files so ~2000 reads is exactly what 
I was describing.

I'm testing my fix for this now.

Comment 5

[Daniel Veditz [:dveditz]]

Attaching a patch that reduces the first run "open jar" times from 2.8 seconds 
on average to 0.4 seconds. This was a big and pleasant surprise -- I figured 
that since the second-run time was only 0.08 seconds before that the bulk of 
the time was in the raw disk reading that we would still be doing. (second-run 
times are now around 0.02 seconds.)

For a first run this shaves about 10% off my win32 start.

The patch:
#defines in the zipstub.h and nsZipArchive.h header files are to reduce the 
clutter of #ifdef STANDALONE in the nzZipArchive code; otherwise the only 
change here is a minor signature change and removal of an unused member.

The real change is in nsZipArchive.cpp where the BuildFileList() algorithm 
changed as outlined in the opening comment of this bug, namely read in big 
chunks and leave item->offset null until needed to avoid seeking up to the top 
of the archive and back for each file.

The changes in nsJARInputStream.* are just opportunistic cleanups.

Comment 6

[Daniel Veditz [:dveditz]]

Attachment: patch to mozilla/modules/libjar

Comment 7

[Sean Su]

wow, very nice.  even the installers (both win mozilla and win ns) seem quicker 
now during the xpinstall phase.  The bits are also getting uncompressed 
correctly.

r=ssu

Comment 8

[Simon Fraser [no longer active]]

waterson gave verbal sr=waterson on this at the performance meeting.

Comment 9

[Daniel Veditz [:dveditz]]

fix checked in 2001/4/3

Comment 10

[Simon Fraser [no longer active]]

Attachment: PR_read traces after the fix

Comment 11

[Cathleen]

this is great!
2051 reads down to 54 reads for modern and other files.

Thanks!  :-)

Comment 12

[Daniel Veditz [:dveditz]]

2051 vs 54 isn't the right way to read the attachments.

Originally there were 2051 modern.jar reads in BuildFileList and 99 in 
various InflateItem stack traces.

Now there are 18 modern.jar reads in BuildFileList, 106 in InflateItem (must 
have added 7 chrome files to startup), and 99 in SeekToItem.

Mostly my patch was to BuildFileList (2051 -> 18 reads), but that gain came in 
part by defering some reads (the 99 new SeekToItem reads).

Comment 13

[Christian Mattar]

*** Bug 69140 has been marked as a duplicate of this bug. ***