[webkit-changes] cvs commit: WebKit/Misc.subproj WebNSURLExtras.m

[Geoffrey]

ggaren      05/10/21 11:43:12

  Modified:    .        ChangeLog
               Misc.subproj WebNSURLExtras.m
  Log:
          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]):
  
  Revision  Changes    Path
  1.3356    +27 -0     WebKit/ChangeLog
  
  Index: ChangeLog
  ===================================================================
  RCS file: /cvs/root/WebKit/ChangeLog,v
  retrieving revision 1.3355
  retrieving revision 1.3356
  diff -u -r1.3355 -r1.3356
  --- ChangeLog	21 Oct 2005 16:35:22 -0000	1.3355
  +++ ChangeLog	21 Oct 2005 18:42:58 -0000	1.3356
  @@ -1,3 +1,30 @@
  +2005-10-21  Geoffrey Garen  <ggaren at apple.com>
  +
  +        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  <opendarwin.org at mitzpettel.com>
   
           Reviewed and landed by Darin.
  
  
  
  1.39      +9 -14     WebKit/Misc.subproj/WebNSURLExtras.m
  
  Index: WebNSURLExtras.m
  ===================================================================
  RCS file: /cvs/root/WebKit/Misc.subproj/WebNSURLExtras.m,v
  retrieving revision 1.38
  retrieving revision 1.39
  diff -u -r1.38 -r1.39
  --- WebNSURLExtras.m	14 Jul 2005 02:43:51 -0000	1.38
  +++ WebNSURLExtras.m	21 Oct 2005 18:43:11 -0000	1.39
  @@ -382,28 +382,23 @@
   
   - (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;
       }
   }