Attachment 361596 Details for Bug 194469 – Patch

[patch] Patch

bug-194469-20190208221614.patch (text/plain), 4.70 KB, created by Yusuke Suzuki on 2019-02-08 22:16:14 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-02-08 22:16:14 PST

Size: 4.70 KB

patch

obsolete

>Subversion Revision: 241233
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index e014af22e574e054359497fd02191a9d713ea56b..06d5a9debfca08300eedd8caf17de64a8f4066e6 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [WTF] Use BufferInternal StringImpl if substring StringImpl takes more memory
>+        https://bugs.webkit.org/show_bug.cgi?id=194469
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * runtime/JSString.h:
>+        (JSC::jsSubstring):
>+
> 2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] String.fromCharCode's slow path always generates 16bit string
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index e45ad93c1e472acbf5066018dd2a237aeafd04dd..603b9bc1ec078dea11e5c71aa60a63c049ffe9d5 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [WTF] Use BufferInternal StringImpl if substring StringImpl takes more memory
>+        https://bugs.webkit.org/show_bug.cgi?id=194469
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Because pointer is large and aligned in 64bit in 64bit architecture, BufferSubstring StringImpl
>+        implementation takes more memory than BufferInternal StringImpl implementation for small strings.
>+        And BufferInternal StringImpl does not have a problem like, small substring StringImpl keeps super
>+        large owner StringImpl. This patch calculates the required size of memory and selects the more efficient one.
>+
>+        * wtf/text/StringImpl.h:
>+        (WTF::StringImpl::isSubString const):
>+        (WTF::StringImpl::createSubstringSharingImpl):
>+
> 2019-02-08  Alex Christensen  <achristensen@webkit.org>
> 
>         Add SPI to use networking daemon instead of XPC service
>diff --git a/Source/JavaScriptCore/runtime/JSString.h b/Source/JavaScriptCore/runtime/JSString.h
>index 112283f80bcfc138c3ac9b12d21342850cc55a77..932b9f2f5caf84c046c49d47f7fe945cfd591ad8 100644
>--- a/Source/JavaScriptCore/runtime/JSString.h
>+++ b/Source/JavaScriptCore/runtime/JSString.h
>@@ -636,7 +636,10 @@ inline JSString* jsSubstring(VM* vm, const String& s, unsigned offset, unsigned
>         if (c <= maxSingleCharacterString)
>             return vm->smallStrings.singleCharacterString(c);
>     }
>-    return JSString::createHasOtherOwner(*vm, StringImpl::createSubstringSharingImpl(*s.impl(), offset, length));
>+    auto impl = StringImpl::createSubstringSharingImpl(*s.impl(), offset, length);
>+    if (impl->isSubString())
>+        return JSString::createHasOtherOwner(*vm, WTFMove(impl));
>+    return JSString::create(*vm, WTFMove(impl));
> }
> 
> inline JSString* jsOwnedString(VM* vm, const String& s)
>diff --git a/Source/WTF/wtf/text/StringImpl.h b/Source/WTF/wtf/text/StringImpl.h
>index 1321771e535bbd7183455bda73fc3d827bac8cb4..85ca50e249a97c54217c28f5902f3c54dc352373 100644
>--- a/Source/WTF/wtf/text/StringImpl.h
>+++ b/Source/WTF/wtf/text/StringImpl.h
>@@ -297,9 +297,7 @@ class StringImpl : private StringImplShape {
>     
>     bool isExternal() const { return bufferOwnership() == BufferExternal; }
> 
>-#if STRING_STATS
>     bool isSubString() const { return bufferOwnership() == BufferSubstring; }
>-#endif
> 
>     static WTF_EXPORT_PRIVATE Expected<CString, UTF8ConversionError> utf8ForCharacters(const LChar* characters, unsigned length);
>     static WTF_EXPORT_PRIVATE Expected<CString, UTF8ConversionError> utf8ForCharacters(const UChar* characters, unsigned length, ConversionMode = LenientConversion);
>@@ -937,10 +935,20 @@ ALWAYS_INLINE Ref<StringImpl> StringImpl::createSubstringSharingImpl(StringImpl&
>     if (!length)
>         return *empty();
> 
>+    // Coyping the thing would save more memory sometimes, largely due to the size of pointer.
>+    size_t substringSize = allocationSize<StringImpl*>(1);
>+    if (rep.is8Bit()) {
>+        if (substringSize >= allocationSize<LChar>(length))
>+            return create(rep.m_data8 + offset, length);
>+    } else {
>+        if (substringSize >= allocationSize<UChar>(length))
>+            return create(rep.m_data16 + offset, length);
>+    }
>+
>     auto* ownerRep = ((rep.bufferOwnership() == BufferSubstring) ? rep.substringBuffer() : &rep);
> 
>     // We allocate a buffer that contains both the StringImpl struct as well as the pointer to the owner string.
>-    auto* stringImpl = static_cast<StringImpl*>(fastMalloc(allocationSize<StringImpl*>(1)));
>+    auto* stringImpl = static_cast<StringImpl*>(fastMalloc(substringSize));
>     if (rep.is8Bit())
>         return adoptRef(*new (NotNull, stringImpl) StringImpl(rep.m_data8 + offset, length, *ownerRep));
>     return adoptRef(*new (NotNull, stringImpl) StringImpl(rep.m_data16 + offset, length, *ownerRep));

Flags:

ggaren: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 194469: 361596 | 361601