Open Bug 190730 Opened 22 years ago Updated 2 years ago

use NS_NewBlockingInputStream instead of NS_NewBufferedInputStream where appropriate

Categories

(Core :: Layout, defect, P3)

x86
Windows 2000
defect

Tracking

()

mozilla1.5alpha

People

(Reporter: alecf, Unassigned)

References

Details

(Keywords: perf, topembed-)

There are a few places in the code that we're using buffered input streams at the same time that we're synchronously loading code. This is kinda lame, as we really should allow the load to happen on background threads. I have a patch which fixes them.
just for reference, these are: - nsSyncLoadService (Spun off the libjar bug, should help startup) - nsXULPrototypeCache (for fastload files - should help startup) - nsHTMLInputElement (for input type="file" - should help patch attachment, etc)
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.4alpha
see also bug 11232 for making a helper function, and bug 113529 for other places where we need to do better I/O.
nominating some of my footprint/perf bugs for topembed
Keywords: topembed
this is going to be much easier once bug 11232 is fixed I fixed 11232 in my tree though, and was able to try a few of these out: - nsSyncLoadService worked very well - nsXULPrototypeCache is going to be more difficult, if not impossible. the problem is that they're expecting a seekable stream, and I'm just not sure we can do that easily over a pipe - right now nsPipe3 does implement nsISeekableStream, but only for Tell(). We'll have to come up with a way to proxy the Seek() which will also mean throwing out any of our existing buffers, which sucks. - nsHTMLInputElement should actually be pretty straight forward. the other place I sort of hooked this up was in RDF, and it worked well there too. The trick with RDF is that you still have to manually pump the input stream to call back into the nsIStreamListener. But that is the 2nd part of bug 11232
Depends on: 11232
Summary: use nsIStreamTransportService instead of NS_NewBufferedInputStream where appropriate → use NS_NewBlockingInputStream instead of NS_NewBufferedInputStream where appropriate
yeah, the pipe is not designed to support seeking. an attribute of a pipe is that it discard/reuse its segments once they are read. i'm very curious about what kind of seeking fastload requires. i.e., does it really need random access to the stream?
Per alecf's request... The place where you will want this in CSS code is in nsCSSLoader::LoadSheet inside the branch that handles aData objects with mSyncLoad set (right now it just does an Open() on the channel).
> i.e., does it [fastload] really need random access to the stream? I'm not the definitive source, but yeah, pretty much, this needs random access to the stream. A serialized document (xul or js) may refer to a serialized object that is located anywhere else in the disk file, and to do the deserialization of the doc means that it has to seek, read and deserialize the obj and then re-seek back to the current position in the doc.
yeah, what jrgm said - its not that its seeking constantly all over the file, its more that it has some understanding of where "streams" are in the file, and so it does a seek, and then streams in a precompiled file from that position. (Which is why I would even consider making nsPipe seek - even if we threw out the old buffers, we'd still get long streams loading on a background thread)
alec: i don't think we should mess with trying to do this for fastload until we determine if "optimizing" the others actually makes an improvement.
I agree.. I'm not going to mess with nsPipe3 itself any more than I have to.. once we've squeezed as much performance as we can out of the low-hanging fruit, then we can go after harder problems like that...
QA Contact: ian → jrgm
Discussed at EDT: plussing nomination.
Keywords: topembedtopembed+
I don't think using a background thread for FastLoad file input is going to save startup time -- FastLoad of XUL prototype elements and scripts is on the critical path to getting a window up, so talking to a thread just wastes time in that schedule without letting anything good happen in the foreground. /be
Priority: P3 → P2
Target Milestone: mozilla1.4alpha → mozilla1.5alpha
5/5 EDT triage: minusing topembed+ status. Dropping this from the radar to better focus on existing working set. (ed. note: I just love playing ping-pong with nominations, makes us all feel warm and fuzzy inside, doesn't it?)
Keywords: topembed+topembed-
QA Contact: jrgmorrison → layout
Moving to p3 because no activity for at least 1 year(s). See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Priority: P2 → P3
Assignee: alecf → nobody
Status: ASSIGNED → NEW
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.