190730 - use NS_NewBlockingInputStream instead of NS_NewBufferedInputStream where appropriate

Status: NEW

(Core :: Layout, defect, P3)

Description

[Alec Flett]

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.

Comment 1

[Alec Flett]

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)

Comment 2

[Alec Flett]

see also bug 11232 for making a helper function, and bug 113529 for other places
where we need to do better I/O.

Comment 3

[Alec Flett]

nominating some of my footprint/perf bugs for topembed

Comment 4

[Alec Flett]

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

Comment 5

[Darin Fisher]

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?

Comment 6

[Boris Zbarsky [:bzbarsky]]

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

Comment 7

[John Morrison]

> 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.

Comment 8

[Alec Flett]

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)

Comment 9

[Darin Fisher]

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.

Comment 10

[Alec Flett]

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...

Comment 11

[Juan José Mata]

Discussed at EDT: plussing nomination.

Comment 12

[Brendan Eich [:brendan]]

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

Comment 13

[Juan José Mata]

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

Comment 14

[Sylvestre Ledru [:Sylvestre]]

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