Closed
Bug 112312
Opened 24 years ago
Closed 24 years ago
JAR uses 32K buffer, affecting startup time
Categories
(SeaMonkey :: General, defect, P2)
SeaMonkey
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla0.9.7
People
(Reporter: cathleennscp, Assigned: dp)
Details
Attachments
(1 file, 2 obsolete files)
2.88 KB,
patch
|
dveditz
:
review+
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•24 years ago
|
||
Comment on attachment 59461 [details] [diff] [review]
Reduce 32k mallocs from libjar - darin & dp
r=dbaron
Attachment #59461 -
Flags: review+
Comment 3•24 years ago
|
||
Comment on attachment 59461 [details] [diff] [review]
Reduce 32k mallocs from libjar - darin & dp
sr=darin
Attachment #59461 -
Flags: superreview+
Assignee | ||
Comment 4•24 years ago
|
||
Fixing this improved startup by 10% when darin and me measured it.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.7
Assignee | ||
Comment 5•24 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 6•24 years ago
|
||
reopened. Installation fails. :-( Need to see why the standalone installer
doesn't like this change
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•24 years ago
|
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 7•24 years ago
|
||
Ccing sean for debugging help on installer.
Comment 8•24 years ago
|
||
The install is complaining about the CRC check which uses
nsZipArchive::TestItem(). Dunno if it's relevant but this routine still
allocates like the one you changed, although now with a smaller buffer.
Maybe Samir would have a clue. There are now reports that the Linux installer is
failing.
Assignee | ||
Comment 9•24 years ago
|
||
So at 6:48am I backed out my changes. There is a bug that still says there are
download errors. http://bugzilla.mozilla.org/show_bug.cgi?id=112631
Comment 10•24 years ago
|
||
I don't see why changing the buffer size would affect CRC checking. And as long
as the archive is not being tested by the jar: protocol handler the CRC checking
code is not used when opening jars. Making a similar change to the CRC checking
code would however benefit install but that's a separate issue.
Comment 11•24 years ago
|
||
The issue is that in addition to removing (some of) the mallocs dp also shrank
the buffer size. The bufsize change is the only thing that would affect
::TestItem, but it's a crucial difference. At 32K we had few or no files that
took more than one buffer, at 4K we probably have several. Not all .xpi files
failed though, so there must be some boundary condition that only comes up
occasionally. (Actually it could still be simply overflowing the buffer since we
don't have many files > 4K either.)
dp, here's a potentially easier way to test: Using a broken build click on one
of the broken .xpi files to install it. Before unpacking the archive XPInstall
will also run through nsZipArchive::TestItem so you could try it out without
having to build and debug the installer itself. I'd recommend the smaller
artext.xpi over browser.xpi because it's probably only failing on a specific
file or two.
A simple way to prove it's in ::TestItem before the tedium of stepping through
the above is to apply your malloc-removing changes but leave the buffer size at
32K -- I'd expect a test like the above to then work.
Assignee | ||
Comment 12•24 years ago
|
||
zlib like odd size buffers. TestItem fails on inflate after it has inflated
completely. Now I understand why the old code was usin 32k-1 and not 32k
I changed the buffer to 4k-1 and things are happy. Thanks for Cathleen for the
dev installer dev environment for debugging this. Patch comming up soon. Will
change TestItem too so installer will benefit from this too.
Assignee | ||
Comment 13•24 years ago
|
||
Attachment #59461 -
Attachment is obsolete: true
Assignee | ||
Comment 14•24 years ago
|
||
Attachment #59860 -
Attachment is obsolete: true
Reporter | ||
Comment 15•24 years ago
|
||
r=cathleen :-)
Comment 16•24 years ago
|
||
Comment on attachment 59863 [details] [diff] [review]
Remove 32k allocs with 4k-1 stack buffers
sr=darin
Attachment #59863 -
Flags: superreview+
Comment 17•24 years ago
|
||
Comment on attachment 59863 [details] [diff] [review]
Remove 32k allocs with 4k-1 stack buffers
r=dveditz too
Attachment #59863 -
Flags: review+
Assignee | ||
Comment 18•24 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Updated•21 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•