611320 - XHR of local files slower than Chrome/Safari

Status: NEW

(Core :: DOM: Core & HTML, defect, P5)

Description

[David Humphrey (:humph)]

Andor has been complaining about how slow it is to load files locally with xhr, especially when Chrome and Safari are so fast.  He's got a test case here:

http://scotland.proximity.on.ca/asalga/tests/local_xhr/Local_XHR.zip

Video of test running on Mac here:

http://www.youtube.com/watch?v=mXQkUgIG5DE

Talking to smaug on irc, he mentions:

"Based on Shark profile the slowness happens in a code which, IIRC, bz changed some time ago."

He also mentions that the test Andor is doing is strange in the way it uses .responseText all the time vs. progress events; however, that quirk notwithstanding, this needs to be a lot faster.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Ok, I think bz didn't touch this particular code after all.
It is old code.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

We're spending ~18% in ConvertBodyToText to assign string value (I think the string should just adopt the outbuffer).

Unicode conversion takes ~17%. Not sure how to optimize that.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

We also call GC quite a bit.

Comment 4

[Andor Salga]

Attachment: Local XHR timing test

I'm running:
OSX 10.5.8
2.2GHz Intel Core 2 Duo
2GB RAM
Minefield 4.0b8pre

Comment 5

[Andor Salga]

I found the performance get progressively worse as the file 
size increases. I attached a screenshot of some tests I ran last night (see above)

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Andor, btw, may I ask why you're using responseText to check how
much data has been downloaded? Why not use progress events?
http://mozilla.pettay.fi/xhr_upload/xhr_file_upload_demo_with_speed.html is a 
rather simple test for progress events.

But anyway, need to speed up our responseText handling during
XHR load. A patch coming hopefully soon to reduce string overhead.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> uses .responseText all the time vs. progress events; 

"Don't Do That".  See bug 552573 and bug 560077 (of which this seems to be a duplicate) for more information.

Reducing the string overhead is still a good idea, but the fundamental problem is just that we have to redecode at all (bug 560077); it makes the load O(N^2) in the file size.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

But isn't poking .responseText done in all browsers? So while that seems like a perf opportunity for the website, it still doesn't excuse us being that much slower, right?

I think we should assume that other people are poking .responseText too and so it's worth optimizing that pattern as well.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> it still doesn't excuse us being that much slower, right?

Yes, that's what bug 560077 is about.  If you can make it so we know whether the XML parser has already set the "final" charset on the document, we can fix that bug...

(Oh, and there's an interesting side issue: what happens when responseText is gotten mid-character.  I wonder what webkit does in that situation....)

Comment 10

[Andor Salga]

Adding progress events did the trick. Local XHR performance is now fine.
Thanks Olli.

Comment 11

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.