Attachment 27205 Details for Bug 23622 – patch -- this time I think it handles threading acceptably

[patch] patch -- this time I think it handles threading acceptably

StringCFPatch.txt (text/plain), 8.01 KB, created by Darin Adler on 2009-01-30 17:56:38 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Darin Adler

Created: 2009-01-30 17:56:38 PST

Size: 8.01 KB

patch

obsolete

>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 40434)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2009-01-30  Darin Adler  <darin@apple.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Bug 23622: create CFString and NSString objects from WebCore::String without copying the characters
>+        https://bugs.webkit.org/show_bug.cgi?id=23622
>+
>+        * platform/text/cf/StringImplCF.cpp:
>+        (WebCore::StringWrapperCFAllocator::allocator): Added. Returns the allocator.
>+        (WebCore::StringWrapperCFAllocator::retain): Added. Callback for allocator.
>+        (WebCore::StringWrapperCFAllocator::release): Ditto.
>+        (WebCore::StringWrapperCFAllocator::copyDescription): Ditto.
>+        (WebCore::StringWrapperCFAllocator::allocate): Ditto.
>+        (WebCore::StringWrapperCFAllocator::reallocate): Ditto.
>+        (WebCore::StringWrapperCFAllocator::deallocate): Ditto.
>+        (WebCore::StringWrapperCFAllocator::preferredSize): Ditto.
>+        (WebCore::StringWrapperCFAllocator::create): Added. Creates the allocator, but
>+        returns 0 if garbage collection is enabled.
>+        (WebCore::StringImpl::createCFString): Use StringWrapperCFAllocator if possible.
>+
>+        * platform/text/mac/StringImplMac.mm:
>+        (WebCore::StringImpl::operator NSString *): Use CFString and toll-free bridging,
>+        rather than using NSString directly. This lets NSString benefit from the above.
>+
> 2009-01-30  Simon Fraser  <simon.fraser@apple.com>
> 
>         Reviewed by Dave Hyatt
>Index: WebCore/platform/text/cf/StringImplCF.cpp
>===================================================================
>--- WebCore/platform/text/cf/StringImplCF.cpp	(revision 40413)
>+++ WebCore/platform/text/cf/StringImplCF.cpp	(working copy)
>@@ -1,5 +1,5 @@
>-/**
>- * Copyright (C) 2006 Apple Computer, Inc.
>+/*
>+ * Copyright (C) 2006, 2009 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -24,14 +24,143 @@
> #if PLATFORM(CF) || (PLATFORM(QT) && PLATFORM(DARWIN))
> 
> #include <CoreFoundation/CoreFoundation.h>
>+#include <wtf/MainThread.h>
>+#include <wtf/PassRefPtr.h>
>+#include <wtf/Threading.h>
>+
>+#if PLATFORM(MAC) && !defined(BUILDING_ON_TIGER)
>+#include <objc/objc-auto.h>
>+#endif
> 
> namespace WebCore {
> 
>+namespace StringWrapperCFAllocator {
>+
>+    static CFAllocatorRef create();
>+
>+    static StringImpl* currentString;
>+
>+    static CFAllocatorRef allocator()
>+    {
>+        static CFAllocatorRef allocator = create();
>+        return allocator;
>+    }
>+
>+    static const void* retain(const void* info)
>+    {
>+        return info;
>+    }
>+
>+    static void release(const void*)
>+    {
>+        ASSERT_NOT_REACHED();
>+    }
>+
>+    static CFStringRef copyDescription(const void*)
>+    {
>+        return CFSTR("WebCore::String-based allocator");
>+    }
>+
>+    static void* allocate(CFIndex size, CFOptionFlags, void*)
>+    {
>+        StringImpl* underlyingString = isMainThread() ? currentString : 0;
>+        size_t allocationSize = sizeof(StringImpl*) + size;
>+        void* block;
>+        if (!underlyingString)
>+            block = malloc(allocationSize);
>+        else {
>+            currentString = 0;
>+            underlyingString->ref(); // Balanced by call to deref in deallocate below.
>+            block = fastMalloc(allocationSize);
>+        }
>+        StringImpl** header = static_cast<StringImpl**>(block);
>+        *header = underlyingString;
>+        return header + 1;
>+    }
>+
>+    static void* reallocate(void* pointer, CFIndex newSize, CFOptionFlags, void*)
>+    {
>+        size_t newAllocationSize = sizeof(StringImpl*) + newSize;
>+        StringImpl** header = static_cast<StringImpl**>(pointer) - 1;
>+        ASSERT(!*header);
>+        header = static_cast<StringImpl**>(realloc(header, newAllocationSize));
>+        return header + 1;
>+    }
>+
>+    static void deallocateOnMainThread(void* headerPointer)
>+    {
>+        StringImpl** header = static_cast<StringImpl**>(headerPointer);
>+        StringImpl* underlyingString = *header;
>+        ASSERT(underlyingString);
>+        underlyingString->deref(); // Balanced by call to ref in allocate above.
>+        fastFree(header);
>+    }
>+
>+    static void deallocate(void* pointer, void*)
>+    {
>+        StringImpl** header = static_cast<StringImpl**>(pointer) - 1;
>+        StringImpl* underlyingString = *header;
>+        if (!underlyingString)
>+            free(header);
>+        else {
>+            if (!isMainThread())
>+                callOnMainThread(deallocateOnMainThread, header);
>+            else {
>+                underlyingString->deref(); // Balanced by call to ref in allocate above.
>+                fastFree(header);
>+            }
>+        }
>+    }
>+
>+    CFIndex preferredSize(CFIndex size, CFOptionFlags, void*)
>+    {
>+        // FIXME: If FastMalloc provided a "good size" callback, we'd want to use it here.
>+        // Note that this optimization would help performance for strings created with the
>+        // allocator that are mutable, and those typically are only created by callers who
>+        // make a new string using the old string's allocator, such as some of the call
>+        // sites in CFURL.
>+        return size;
>+    }
>+
>+    static CFAllocatorRef create()
>+    {
>+#if PLATFORM(MAC) && !defined(BUILDING_ON_TIGER)
>+        // Since garbage collection isn't compatible with custom allocators, don't use this at all when garbage collection is active.
>+        if (objc_collectingEnabled())
>+            return 0;
>+#endif
>+        CFAllocatorContext context = { 0, 0, retain, release, copyDescription, allocate, reallocate, deallocate, preferredSize };
>+        return CFAllocatorCreate(0, &context);
>+    }
>+
>+}
>+
> CFStringRef StringImpl::createCFString()
> {
>-    return CFStringCreateWithCharacters(NULL, reinterpret_cast<const UniChar*>(m_data), m_length);
>+    CFAllocatorRef allocator = (m_length && isMainThread()) ? StringWrapperCFAllocator::allocator() : 0;
>+    if (!allocator)
>+        return CFStringCreateWithCharacters(0, reinterpret_cast<const UniChar*>(m_data), m_length);
>+
>+    // Put pointer to the StringImpl in a global so the allocator can store it with the CFString.
>+    ASSERT(!StringWrapperCFAllocator::currentString);
>+    StringWrapperCFAllocator::currentString = this;
>+
>+    CFStringRef string = CFStringCreateWithCharactersNoCopy(allocator, reinterpret_cast<const UniChar*>(m_data), m_length, kCFAllocatorNull);
>+
>+    // The allocator cleared the global when it read it, but also clear it here just in case.
>+    ASSERT(!StringWrapperCFAllocator::currentString);
>+    StringWrapperCFAllocator::currentString = 0;
>+
>+    return string;
> }
> 
>+// On StringImpl creation we could check if the allocator is the StringWrapperCFAllocator.
>+// If it is, then we could find the original StringImpl and just return that. But to
>+// do that we'd have to compute the offset from CFStringRef to the allocated block;
>+// the CFStringRef is *not* at the start of an allocated block. Testing shows 1000x
>+// more calls to createCFString than calls to the create functions with the appropriate
>+// allocator, so it's probably not urgent optimize that case.
>+
> }
> 
> #endif // PLATFORM(CF) || (PLATFORM(QT) && PLATFORM(DARWIN))
>Index: WebCore/platform/text/mac/StringImplMac.mm
>===================================================================
>--- WebCore/platform/text/mac/StringImplMac.mm	(revision 40413)
>+++ WebCore/platform/text/mac/StringImplMac.mm	(working copy)
>@@ -1,5 +1,5 @@
>-/**
>- * Copyright (C) 2006 Apple Computer, Inc.
>+/*
>+ * Copyright (C) 2006, 2009 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -21,13 +21,13 @@
> #include "config.h"
> #include "StringImpl.h"
> 
>-#include <Foundation/Foundation.h>
>+#include "FoundationExtras.h"
> 
> namespace WebCore {
> 
> StringImpl::operator NSString *()
> {
>-    return [NSString stringWithCharacters:m_data length:m_length];
>+    return HardAutorelease(createCFString());
> }
> 
> }

Flags:

mrowe: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 23622: 27150 | 27186 | 27205 | 27206 | 27208