Open Bug 190730 Opened 17 years ago Updated 1 year ago

use NS_NewBlockingInputStream instead of NS_NewBufferedInputStream where appropriate


(Core :: Layout, defect, P3)

Windows 2000





(Reporter: alecf, Assigned: alecf)



(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)

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.

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 for more information
Priority: P2 → P3
You need to log in before you can comment on or make changes to this bug.