247458 - Use 'inflateBack' instead of 'inflate' in ZipArchive.cpp

Status: RESOLVED INCOMPLETE

(Core :: Networking: JAR, enhancement)

Description

[Alfred Kayser]

Now that zlib 1.2.1 is used in Mozilla, we 
can try to see if 'inflateBack' can reduce buffer copies
in nsZipArchive.cpp.

See manual of zlib 1.2.1 for 'inflateBack'

Comment 1

[Darin Fisher]

Alfred: feel free to contribute a patch :)

Comment 2

[Alfred Kayser]

Just a quick update:
inflateBack doesn't seem usefull for the 'JARInputStream' with its own streaming
capabilities. However it is usefull for the 'extracting' of files.
Using inflateBack the 'window' used by inflate will also double as output buffer
to the file writes. inflateBack wants a 32K window buffer be passed, instead of
allocating itself.

So, an idea is to allocate this 32K window once for the ZIPReader, and re-use it
for each extraction (just as the filepointer is re-used). Furthermore instead of
using two (stack) buffers of 4K (one for read, and for write), only one (stack)
8K buffer could be used to read (or even more?).
Codesize wouldn't change much, but it would improve the speed of extracting
files (for installation of mozilla apps as well as for extensions and themes).

It is a small change, and a patch will be posted soon.

Comment 3

[Alfred Kayser]

Attachment: Proof-of-concept patch

This patch is diff against my patched version of libjar.
See Bug 214672, attachment jar_patch7.txt.

Changes:
* Use inflateBack for extraction (saves a buffer copy in writing)
* Use a re-used mExtractWindow for both inflation and copy
* Removed wildcard matching from Find* for the standalone (see bug 272451)
* Removed use of RecyclingAllocator as zlib only does one or two allocs per
'inflate' (one for the window, in case of normal inflate, and one for its own
data). 
* Removed nsJarShutdown, was only needed for the RecyclingAllocator...
* In total 1.5K code for normal and about 3K code reduction in standalone.

So, this saves code as well as improves the speed of extraction (less buffer
copying...). All in the quest for smaller and faster browsers (and other apps).

Comment 4

[Alfred Kayser]

Attachment: V2: updated to current trunk and some cleanups

Comment 5

[Alfred Kayser]

Attachment: V3: Small fix in CopyItemToDisk

Some notes:
* Use gzlibAllocator to also allocate the 32K 'inflateBack' buffer
* Changed calloc's into malloc's according to zlib documentation
* Removed the BY4ALLOC_ITEMS stuff (deadcode)
* CopyItemToDisk and InflateItem now both static functions (better for optimization)

Comment 6

[Alfred Kayser]

Attachment: v4: Don't mix the readbuffer with inflate buffer...

Oops, the readbuffer for inflateBack must not be the same as the inflate/write window buffer.

The impact of this all is thus to save buffer copies, and to use a 32K buffer instead of 4K buffers to the file I/O (sharing the inflation buffer with the file write buffer).

Comment 7

[Alfred Kayser]

There may be chance that this can still be done.

Comment 8

[Patrick McManus [:mcmanus]]

I'm going to close this as inactive. Feel free to reopen if you've got an updated patch and benchmark