[Webkit-unassigned] [Bug 43987] Downloading using XHR is much slower than before

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=43987


Gavin Barraclough <barraclough at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #69648|review?                     |review-
               Flag|                            |




--- Comment #11 from Gavin Barraclough <barraclough at apple.com>  2010-10-08 13:46:00 PST ---
(From update of attachment 69648)
Okay, I don't think this is quite how we want to fix this.  Fundamentally this patch is doing the right thing, in that we should be using a smarter string building layer rather that resolving a fresh string every time new data is available, but it's doing it in slightly the wrong way:

(1) I think this patch is fixing the problem in the wrong place.  We don't want to be adding more code to ScriptString - in fact, we don't really want ScriptString to exist at all! (this class only existed because XMLHttpRequest used to hold a UString, which wasn't great from design perspective in the first place, and clearly isn't ).  Strings in WebCore are by design immutable, so XMLHttpRequest holding onto a string and appending to it is not the correct approach.  The place this problem needs to be fixed is in XMLHttpRequest.  (Moreover, all uses of ScriptString are now wrong.  XMLHttpRequest only held a StringString under the assumption this can be used for fast concatenation, which is no longer (necessarily) the case – as demonstrated by this bug.  New code in fileapi is currently using ScriptString to mimic XMLHttpRequest, and as such will also be mimicking this O(N^2) performance too.  Whatever fix is made for this bug should also be applied to FileReader/FileReaderSync - and having done so StringString will be redundant.)

(2) We shouldn't be adding a new type to build Strings – we already have StringBuilder.  XMLHttpRequest should just use a StringBuilder.  If StringBuilder does not have the performance characteristics we require, we should fix it ...

(3) ... and StringBuilder probably doesn't have the performance characteristics we require. :-)  Building the string up as a rope saves resolving the string on every append, but accesses to the responseText string while data is being loaded will still result in a copy to resolve, and as such repeated calls to access the responseText could still result in repeated copying of the character data in the String.  Instead we should probably look at using a allocating a string buffer with overcapacity, such that appends can write into the existing buffer.  We should be able support with using the existing createUninitialized & substring mechanisms supported by WTF StringImpls.  Using a 2x overcapacity should make string construction O(N), with zero cost to resolve.

I have a patch for (1) and (2), will attach, and most of the code written for (3) too.

G.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.