Title: [134950] trunk/Source/WebCore
Revision
134950
Author
commit-qu...@webkit.org
Date
2012-11-16 07:47:06 -0800 (Fri, 16 Nov 2012)

Log Message

add 7 bit strings capabilities to the v8 binding layer
https://bugs.webkit.org/show_bug.cgi?id=91850

Patch by Dan Carney <dcar...@google.com> on 2012-11-16
Reviewed by Adam Barth.

This change enables the v8 binding layer to make use of webkit's
8 bit string capabilities. Using 8 bit strings leads to certain
benchmark performance improvemnts as can be seen in
https://bug-91850-attachments.webkit.org/attachment.cgi?id=163334.

No new tests.  Test coverage already extensive.

* bindings/v8/V8PerIsolateData.cpp:
(WebCore::V8PerIsolateData::visitExternalStrings):
* bindings/v8/V8StringResource.cpp:
(StringTraits):
(WebCore::false):
(WebCore):
(WebCore::true):
(WebCore::v8StringToWebCoreString):
* bindings/v8/V8ValueCache.cpp:
(WebCore::makeExternalString):
(WebCore::WebCoreStringResourceBase::toWebCoreStringResourceBase):
(WebCore):
(WebCore::WebCoreStringResourceBase::visitStrings):
* bindings/v8/V8ValueCache.h:
(WebCoreStringResourceBase):
(WebCore::WebCoreStringResourceBase::WebCoreStringResourceBase):
(WebCore::WebCoreStringResourceBase::~WebCoreStringResourceBase):
(WebCore::WebCoreStringResourceBase::atomicString):
(WebCore::WebCoreStringResourceBase::memoryConsumption):
(WebCoreStringResource16):
(WebCore::WebCoreStringResource16::WebCoreStringResource16):
(WebCore):
(WebCoreStringResource8):
(WebCore::WebCoreStringResource8::WebCoreStringResource8):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (134949 => 134950)


--- trunk/Source/WebCore/ChangeLog	2012-11-16 15:44:12 UTC (rev 134949)
+++ trunk/Source/WebCore/ChangeLog	2012-11-16 15:47:06 UTC (rev 134950)
@@ -1,3 +1,42 @@
+2012-11-16  Dan Carney  <dcar...@google.com>
+
+        add 7 bit strings capabilities to the v8 binding layer
+        https://bugs.webkit.org/show_bug.cgi?id=91850
+
+        Reviewed by Adam Barth.
+
+        This change enables the v8 binding layer to make use of webkit's
+        8 bit string capabilities. Using 8 bit strings leads to certain
+        benchmark performance improvemnts as can be seen in
+        https://bug-91850-attachments.webkit.org/attachment.cgi?id=163334.
+
+        No new tests.  Test coverage already extensive.
+
+        * bindings/v8/V8PerIsolateData.cpp:
+        (WebCore::V8PerIsolateData::visitExternalStrings):
+        * bindings/v8/V8StringResource.cpp:
+        (StringTraits):
+        (WebCore::false):
+        (WebCore):
+        (WebCore::true):
+        (WebCore::v8StringToWebCoreString):
+        * bindings/v8/V8ValueCache.cpp:
+        (WebCore::makeExternalString):
+        (WebCore::WebCoreStringResourceBase::toWebCoreStringResourceBase):
+        (WebCore):
+        (WebCore::WebCoreStringResourceBase::visitStrings):
+        * bindings/v8/V8ValueCache.h:
+        (WebCoreStringResourceBase):
+        (WebCore::WebCoreStringResourceBase::WebCoreStringResourceBase):
+        (WebCore::WebCoreStringResourceBase::~WebCoreStringResourceBase):
+        (WebCore::WebCoreStringResourceBase::atomicString):
+        (WebCore::WebCoreStringResourceBase::memoryConsumption):
+        (WebCoreStringResource16):
+        (WebCore::WebCoreStringResource16::WebCoreStringResource16):
+        (WebCore):
+        (WebCoreStringResource8):
+        (WebCore::WebCoreStringResource8::WebCoreStringResource8):
+
 2012-11-16  Erik Arvidsson  <a...@chromium.org>
 
         Update DOMException name: InvalidAccessError

Modified: trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp (134949 => 134950)


--- trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp	2012-11-16 15:44:12 UTC (rev 134949)
+++ trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp	2012-11-16 15:47:06 UTC (rev 134950)
@@ -121,7 +121,7 @@
         virtual ~VisitorImpl() { }
         virtual void VisitExternalString(v8::Handle<v8::String> string)
         {
-            WebCoreStringResource* resource = static_cast<WebCoreStringResource*>(string->GetExternalStringResource());
+            WebCoreStringResourceBase* resource = WebCoreStringResourceBase::toWebCoreStringResourceBase(string);
             if (resource)
                 resource->visitStrings(m_visitor);
         }

Modified: trunk/Source/WebCore/bindings/v8/V8StringResource.cpp (134949 => 134950)


--- trunk/Source/WebCore/bindings/v8/V8StringResource.cpp	2012-11-16 15:44:12 UTC (rev 134949)
+++ trunk/Source/WebCore/bindings/v8/V8StringResource.cpp	2012-11-16 15:47:06 UTC (rev 134950)
@@ -30,72 +30,129 @@
 
 namespace WebCore {
 
-template <class StringClass> struct StringTraits {
-    static const StringClass& fromStringResource(WebCoreStringResource*);
+template<class StringClass> struct StringTraits {
+    static const StringClass& fromStringResource(WebCoreStringResourceBase*);
+    static bool is16BitAtomicString(StringClass&);
+    template<bool ascii>
     static StringClass fromV8String(v8::Handle<v8::String>, int);
 };
 
 template<>
 struct StringTraits<String> {
-    static const String& fromStringResource(WebCoreStringResource* resource)
+    static const String& fromStringResource(WebCoreStringResourceBase* resource)
     {
         return resource->webcoreString();
     }
-
-    static String fromV8String(v8::Handle<v8::String> v8String, int length)
+    static bool is16BitAtomicString(String& string)
     {
-        ASSERT(v8String->Length() == length);
-        // NOTE: as of now, String(const UChar*, int) performs String::createUninitialized
-        // anyway, so no need to optimize like we do for AtomicString below.
-        UChar* buffer;
-        String result = String::createUninitialized(length, buffer);
-        v8String->Write(reinterpret_cast<uint16_t*>(buffer), 0, length);
-        return result;
+        return false;
     }
+    template<bool ascii>
+    static String fromV8String(v8::Handle<v8::String>, int);
 };
 
 template<>
 struct StringTraits<AtomicString> {
-    static const AtomicString& fromStringResource(WebCoreStringResource* resource)
+    static const AtomicString& fromStringResource(WebCoreStringResourceBase* resource)
     {
         return resource->atomicString();
     }
-
-    static AtomicString fromV8String(v8::Handle<v8::String> v8String, int length)
+    static bool is16BitAtomicString(AtomicString& string)
     {
-        ASSERT(v8String->Length() == length);
-        static const int inlineBufferSize = 16;
-        if (length <= inlineBufferSize) {
-            UChar inlineBuffer[inlineBufferSize];
-            v8String->Write(reinterpret_cast<uint16_t*>(inlineBuffer), 0, length);
-            return AtomicString(inlineBuffer, length);
-        }
-        UChar* buffer;
-        String string = String::createUninitialized(length, buffer);
-        v8String->Write(reinterpret_cast<uint16_t*>(buffer), 0, length);
-        return AtomicString(string);
+        return !string.string().is8Bit();
     }
+    template<bool ascii>
+    static AtomicString fromV8String(v8::Handle<v8::String>, int);
 };
 
-template <typename StringType>
+template<>
+String StringTraits<String>::fromV8String<false>(v8::Handle<v8::String> v8String, int length)
+{
+    ASSERT(v8String->Length() == length);
+    UChar* buffer;
+    String result = String::createUninitialized(length, buffer);
+    v8String->Write(reinterpret_cast<uint16_t*>(buffer), 0, length);
+    return result;
+}
+
+template<>
+AtomicString StringTraits<AtomicString>::fromV8String<false>(v8::Handle<v8::String> v8String, int length)
+{
+    ASSERT(v8String->Length() == length);
+    static const int inlineBufferSize = 16;
+    if (length <= inlineBufferSize) {
+        UChar inlineBuffer[inlineBufferSize];
+        v8String->Write(reinterpret_cast<uint16_t*>(inlineBuffer), 0, length);
+        return AtomicString(inlineBuffer, length);
+    }
+    UChar* buffer;
+    String result = String::createUninitialized(length, buffer);
+    v8String->Write(reinterpret_cast<uint16_t*>(buffer), 0, length);
+    return AtomicString(result);
+}
+
+template<>
+String StringTraits<String>::fromV8String<true>(v8::Handle<v8::String> v8String, int length)
+{
+    ASSERT(v8String->Length() == length);
+    LChar* buffer;
+    String result = String::createUninitialized(length, buffer);
+    v8String->WriteAscii(reinterpret_cast<char*>(buffer), 0, length, v8::String::PRESERVE_ASCII_NULL);
+    return result;
+}
+
+template<>
+AtomicString StringTraits<AtomicString>::fromV8String<true>(v8::Handle<v8::String> v8String, int length)
+{
+    ASSERT(v8String->Length() == length);
+    static const int inlineBufferSize = 32;
+    if (length <= inlineBufferSize) {
+        LChar inlineBuffer[inlineBufferSize];
+        v8String->WriteAscii(reinterpret_cast<char*>(inlineBuffer), 0, length, v8::String::PRESERVE_ASCII_NULL);
+        return AtomicString(inlineBuffer, length);
+    }
+    LChar* buffer;
+    String string = String::createUninitialized(length, buffer);
+    v8String->WriteAscii(reinterpret_cast<char*>(buffer), 0, length, v8::String::PRESERVE_ASCII_NULL);
+    return AtomicString(string);
+}
+
+template<typename StringType>
 StringType v8StringToWebCoreString(v8::Handle<v8::String> v8String, ExternalMode external)
 {
-    WebCoreStringResource* stringResource = WebCoreStringResource::toStringResource(v8String);
-    if (stringResource)
-        return StringTraits<StringType>::fromStringResource(stringResource);
+    {
+        // A lot of WebCoreStringResourceBase::toWebCoreStringResourceBase is copied here by hand for performance reasons.
+        // This portion of this function is very hot in certain Dromeao benchmarks.
+        v8::String::Encoding encoding;
+        v8::String::ExternalStringResourceBase* resource = v8String->GetExternalStringResourceBase(&encoding);
+        if (LIKELY(!!resource)) {
+            WebCoreStringResourceBase* base;
+            if (encoding == v8::String::ASCII_ENCODING)
+                base = static_cast<WebCoreStringResource8*>(resource);
+            else
+                base = static_cast<WebCoreStringResource16*>(resource);
+            return StringTraits<StringType>::fromStringResource(base);
+        }
+    }
 
     int length = v8String->Length();
-    if (!length)
+    if (UNLIKELY(!length))
         return String("");
 
-    StringType result(StringTraits<StringType>::fromV8String(v8String, length));
+    bool nonAscii = v8String->MayContainNonAscii();
+    StringType result(nonAscii ? StringTraits<StringType>::template fromV8String<false>(v8String, length) : StringTraits<StringType>::template fromV8String<true>(v8String, length));
 
-    if (external == Externalize && v8String->CanMakeExternal()) {
-        stringResource = new WebCoreStringResource(result);
-        if (!v8String->MakeExternal(stringResource)) {
-            // In case of a failure delete the external resource as it was not used.
+    if (external != Externalize || !v8String->CanMakeExternal())
+        return result;
+
+    if (!nonAscii && !StringTraits<StringType>::is16BitAtomicString(result)) {
+        WebCoreStringResource8* stringResource = new WebCoreStringResource8(result);
+        if (UNLIKELY(!v8String->MakeExternal(stringResource)))
             delete stringResource;
-        }
+    } else {
+        WebCoreStringResource16* stringResource = new WebCoreStringResource16(result);
+        if (UNLIKELY(!v8String->MakeExternal(stringResource)))
+            delete stringResource;
     }
     return result;
 }

Modified: trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp (134949 => 134950)


--- trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp	2012-11-16 15:44:12 UTC (rev 134949)
+++ trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp	2012-11-16 15:47:06 UTC (rev 134950)
@@ -33,7 +33,15 @@
 
 static v8::Local<v8::String> makeExternalString(const String& string)
 {
-    WebCoreStringResource* stringResource = new WebCoreStringResource(string);
+    if (string.is8Bit() && string.containsOnlyASCII()) {
+        WebCoreStringResource8* stringResource = new WebCoreStringResource8(string);
+        v8::Local<v8::String> newString = v8::String::NewExternal(stringResource);
+        if (newString.IsEmpty())
+            delete stringResource;
+        return newString;
+    }
+
+    WebCoreStringResource16* stringResource = new WebCoreStringResource16(string);
     v8::Local<v8::String> newString = v8::String::NewExternal(stringResource);
     if (newString.IsEmpty())
         delete stringResource;
@@ -93,8 +101,19 @@
     return newString;
 }
 
-void WebCoreStringResource::visitStrings(ExternalStringVisitor* visitor)
+WebCoreStringResourceBase* WebCoreStringResourceBase::toWebCoreStringResourceBase(v8::Handle<v8::String> string)
 {
+    v8::String::Encoding encoding;
+    v8::String::ExternalStringResourceBase* resource = string->GetExternalStringResourceBase(&encoding);
+    if (!resource)
+        return 0;
+    if (encoding == v8::String::ASCII_ENCODING)
+        return static_cast<WebCoreStringResource8*>(resource);
+    return static_cast<WebCoreStringResource16*>(resource);
+}
+
+void WebCoreStringResourceBase::visitStrings(ExternalStringVisitor* visitor)
+{
     visitor->visitJSExternalString(m_plainString.impl());
     if (m_plainString.impl() != m_atomicString.impl() && !m_atomicString.isNull())
         visitor->visitJSExternalString(m_atomicString.impl());

Modified: trunk/Source/WebCore/bindings/v8/V8ValueCache.h (134949 => 134950)


--- trunk/Source/WebCore/bindings/v8/V8ValueCache.h	2012-11-16 15:44:12 UTC (rev 134949)
+++ trunk/Source/WebCore/bindings/v8/V8ValueCache.h	2012-11-16 15:47:06 UTC (rev 134950)
@@ -72,19 +72,21 @@
 
 // WebCoreStringResource is a helper class for v8ExternalString. It is used
 // to manage the life-cycle of the underlying buffer of the external string.
-class WebCoreStringResource : public v8::String::ExternalStringResource {
+class WebCoreStringResourceBase {
 public:
-    explicit WebCoreStringResource(const String& string)
+    static WebCoreStringResourceBase* toWebCoreStringResourceBase(v8::Handle<v8::String>);
+
+    explicit WebCoreStringResourceBase(const String& string)
         : m_plainString(string)
     {
 #ifndef NDEBUG
         m_threadId = WTF::currentThread();
 #endif
         ASSERT(!string.isNull());
-        v8::V8::AdjustAmountOfExternalAllocatedMemory(2 * string.length());
+        v8::V8::AdjustAmountOfExternalAllocatedMemory(memoryConsumption(string));
     }
 
-    explicit WebCoreStringResource(const AtomicString& string)
+    explicit WebCoreStringResourceBase(const AtomicString& string)
         : m_plainString(string.string())
         , m_atomicString(string)
     {
@@ -92,27 +94,20 @@
         m_threadId = WTF::currentThread();
 #endif
         ASSERT(!string.isNull());
-        v8::V8::AdjustAmountOfExternalAllocatedMemory(2 * string.length());
+        v8::V8::AdjustAmountOfExternalAllocatedMemory(memoryConsumption(string));
     }
 
-    virtual ~WebCoreStringResource()
+    virtual ~WebCoreStringResourceBase()
     {
 #ifndef NDEBUG
         ASSERT(m_threadId == WTF::currentThread());
 #endif
-        int reducedExternalMemory = -2 * m_plainString.length();
+        int reducedExternalMemory = -memoryConsumption(m_plainString);
         if (m_plainString.impl() != m_atomicString.impl() && !m_atomicString.isNull())
-            reducedExternalMemory *= 2;
+            reducedExternalMemory -= memoryConsumption(m_atomicString.string());
         v8::V8::AdjustAmountOfExternalAllocatedMemory(reducedExternalMemory);
     }
 
-    virtual const uint16_t* data() const
-    {
-        return reinterpret_cast<const uint16_t*>(m_plainString.impl()->characters());
-    }
-
-    virtual size_t length() const { return m_plainString.impl()->length(); }
-
     const String& webcoreString() { return m_plainString; }
 
     const AtomicString& atomicString()
@@ -124,19 +119,14 @@
             m_atomicString = AtomicString(m_plainString);
             ASSERT(!m_atomicString.isNull());
             if (m_plainString.impl() != m_atomicString.impl())
-                v8::V8::AdjustAmountOfExternalAllocatedMemory(2 * m_atomicString.length());
+                v8::V8::AdjustAmountOfExternalAllocatedMemory(memoryConsumption(m_atomicString.string()));
         }
         return m_atomicString;
     }
 
     void visitStrings(ExternalStringVisitor*);
 
-    static WebCoreStringResource* toStringResource(v8::Handle<v8::String> v8String)
-    {
-        return static_cast<WebCoreStringResource*>(v8String->GetExternalStringResource());
-    }
-
-private:
+protected:
     // A shallow copy of the string. Keeps the string buffer alive until the V8 engine garbage collects it.
     String m_plainString;
     // If this string is atomic or has been made atomic earlier the
@@ -146,11 +136,40 @@
     // into that string.
     AtomicString m_atomicString;
 
+private:
+    static int memoryConsumption(const String& string)
+    {
+        return string.length() * (string.is8Bit() ? sizeof(LChar) : sizeof(UChar));
+    }
 #ifndef NDEBUG
     WTF::ThreadIdentifier m_threadId;
 #endif
 };
 
+class WebCoreStringResource16 : public WebCoreStringResourceBase, public v8::String::ExternalStringResource {
+public:
+    explicit WebCoreStringResource16(const String& string) : WebCoreStringResourceBase(string) { }
+    explicit WebCoreStringResource16(const AtomicString& string) : WebCoreStringResourceBase(string) { }
+
+    virtual size_t length() const OVERRIDE { return m_plainString.impl()->length(); }
+    virtual const uint16_t* data() const OVERRIDE
+    {
+        return reinterpret_cast<const uint16_t*>(m_plainString.impl()->characters());
+    }
+};
+
+class WebCoreStringResource8 : public WebCoreStringResourceBase, public v8::String::ExternalAsciiStringResource {
+public:
+    explicit WebCoreStringResource8(const String& string) : WebCoreStringResourceBase(string) { }
+    explicit WebCoreStringResource8(const AtomicString& string) : WebCoreStringResourceBase(string) { }
+
+    virtual size_t length() const OVERRIDE { return m_plainString.impl()->length(); }
+    virtual const char* data() const OVERRIDE
+    {
+        return reinterpret_cast<const char*>(m_plainString.impl()->characters8());
+    }
+};
+
 class IntegerCache {
 public:
      IntegerCache() : m_initialized(false) { };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to