diff --git a/WebKit/ChangeLog b/WebKit/ChangeLog index 809c3abed019..03aa54373775 100644 --- a/WebKit/ChangeLog +++ b/WebKit/ChangeLog @@ -1,3 +1,30 @@ +2005-10-21 Geoffrey Garen + + Patch by TimO, Reviewed by hyatt, tested and landed by me. + + Found what appears to be a misguided optimization that actually causes a measurable performance problem. + A fixed-size buffer was allocated on the stack to pass into CFURLGetBytes(), presumably to avoid malloc() + for URLs less than 2048 bytes. There was also a fallback which malloc()'ed a buffer in case the fixed-size + buffer was too small to hold the URL's bytes. This malloc()'ed buffer was then wrapped in an NSData using + +dataWithBytesNoCopy:length:, avoiding a memory copy (yay!) + + The problem with this approach is two-fold: + + 1. Regardless of how the buffer was allocated and filled, it is immediately wrapped in an NSData using + +dataWithBytes:length:, which copies the input bytes. This is pretty much unavoidable; we need to get + the data into a malloc()'ed buffer to return it to the caller, unless the caller provides its own storage + (which would be super inconvenient). + + 2. The size of the fixed buffer was large enough that it fit most (if not all) URLs involved in our Page + Load Test. This means the unintentionally-inefficient case was by far the most *common* case! + + My fix is to malloc() the buffer from the start, and then use +[NSData dataWithBytes:length:freeWhenDone:] + to wrap the buffer in an NSData. This avoids a memory copy for the normal case where a URL is less than + 2048 bytes, and keeps the efficient behavior for the uncommon long URL case. + + * Misc.subproj/WebNSURLExtras.m: + (-[NSURL _web_originalData]): + 2005-10-21 Mitz Pettel Reviewed and landed by Darin. diff --git a/WebKit/Misc.subproj/WebNSURLExtras.m b/WebKit/Misc.subproj/WebNSURLExtras.m index 4d7a51221fae..6a77e4a7dabe 100644 --- a/WebKit/Misc.subproj/WebNSURLExtras.m +++ b/WebKit/Misc.subproj/WebNSURLExtras.m @@ -382,28 +382,23 @@ + (NSURL *)_web_URLWithData:(NSData *)data relativeToURL:(NSURL *)baseURL - (NSData *)_web_originalData { - NSData *data = nil; - - UInt8 static_buffer[URL_BYTES_BUFFER_LENGTH]; - CFIndex bytesFilled = CFURLGetBytes((CFURLRef)self, static_buffer, URL_BYTES_BUFFER_LENGTH); - if (bytesFilled != -1) { - data = [NSData dataWithBytes:static_buffer length:bytesFilled]; - } - else { + UInt8 *buffer = (UInt8 *)malloc(URL_BYTES_BUFFER_LENGTH); + CFIndex bytesFilled = CFURLGetBytes((CFURLRef)self, buffer, URL_BYTES_BUFFER_LENGTH); + if (bytesFilled == -1) { CFIndex bytesToAllocate = CFURLGetBytes((CFURLRef)self, NULL, 0); - UInt8 *buffer = malloc(bytesToAllocate); + buffer = (UInt8 *)realloc(buffer, bytesToAllocate); bytesFilled = CFURLGetBytes((CFURLRef)self, buffer, bytesToAllocate); ASSERT(bytesFilled == bytesToAllocate); - // buffer is adopted by the NSData - data = [NSData dataWithBytesNoCopy:buffer length:bytesFilled]; } - + + // buffer is adopted by the NSData + NSData *data = [NSData dataWithBytesNoCopy:buffer length:bytesFilled freeWhenDone:YES]; + NSURL *baseURL = (NSURL *)CFURLGetBaseURL((CFURLRef)self); if (baseURL) { NSURL *URL = [NSURL _web_URLWithData:data relativeToURL:baseURL]; return [URL _web_originalData]; - } - else { + } else { return data; } }