[Webkit-unassigned] [Bug 16159] New: String optimization- remove realloc from StringImpl::adopt

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=16159

           Summary: String optimization- remove realloc from
                    StringImpl::adopt
           Product: WebKit
           Version: 525+ (Nightly build)
          Platform: PC
        OS/Version: Mac OS X 10.4
            Status: UNCONFIRMED
          Severity: Normal
          Priority: P2
         Component: Platform
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: mike at belshe.com
                CC: mrowe at apple.com


While doing some work which needed to import external strings into WebCore
strings, I tried to use adopt to avoid buffer copies, but found it to be slow. 
It does both a realloc() and also a malloc().

>From discussing with bdash, it seems that on platforms using fastmalloc the
realloc() call may be cheap.  However, on the windows build w/o fastmalloc, the
realloc call is very significant; in my profiling it accounts for 46% of the
time spent in StringImpl::adopt().  You can see the source to realloc() on
windows in the CRT runtime sources (usually located in c:\Program
Files\Microsoft Visual Studio 8\VC\crt\src\realloc.c).  You can see that it is
not cheap :-)

I believe there is a simple optimization which dramatically helps the adopt()
performance in StringImpl.cpp:

StringImpl* StringImpl::adopt(Vector<UChar>& buffer)
{
    size_t size = buffer.size();
    UChar* data = buffer.releaseBuffer();

    // Just remove this line!
    //data = static_cast<UChar*>(fastRealloc(data, size * sizeof(UChar)));   

    StringImpl* result = new StringImpl;
    result->m_length = size;
    result->m_data = data;
    return result;
}

There is a potential downside to this; if the vector being adopted had a large
unused capacity, then this call would no longer reclaim that extra space. 
However, from my testing, I've found no cases where this is significant, while
the performance delta is significant.

BTW - I did discover that I can use StringImpl::newUninitialized() as a
workaround in my case.  But I still believe adopt() should be cheap and that
this is a good optimization.


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