14.88 KB, patch
|Details | Diff | Splinter Review|
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'
Alfred: feel free to contribute a patch :)
Target Milestone: --- → Future
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.
Created attachment 168041 [details] [diff] [review] 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).
Assignee: darin → alfredkayser
Status: NEW → ASSIGNED
Created attachment 221429 [details] [diff] [review] V2: updated to current trunk and some cleanups
Created attachment 221430 [details] [diff] [review] 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)
Created attachment 221431 [details] [diff] [review] 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).
Assignee: alfredkayser → nobody
Status: ASSIGNED → NEW
QA Contact: benc → networking.jar
There may be chance that this can still be done.
Assignee: nobody → alfredkayser
Status: NEW → ASSIGNED
I'm going to close this as inactive. Feel free to reopen if you've got an updated patch and benchmark
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.