Attachment 361608 Details for Bug 194474 – ROLLOUT of r241237

[patch] ROLLOUT of r241237

bug-194474-20190209095002.patch (text/plain), 4.56 KB, created by WebKit Commit Bot on 2019-02-09 09:50:03 PST

(hide)

Description:

Filename:

MIME Type:

Creator: WebKit Commit Bot

Created: 2019-02-09 09:50:03 PST

Size: 4.56 KB

patch

obsolete

>Subversion Revision: 241240
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 068ff09c94a9f80c71571ef33b06c24001d4ce19..69b4b0efcb6d1c32c772b5ae232921b461670ce3 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-02-09  Commit Queue  <commit-queue@webkit.org>
>+
>+        Unreviewed, rolling out r241237.
>+        https://bugs.webkit.org/show_bug.cgi?id=194474
>+
>+        Shows significant memory increase in WSL (Requested by
>+        yusukesuzuki on #webkit).
>+
>+        Reverted changeset:
>+
>+        "[WTF] Use BufferInternal StringImpl if substring StringImpl
>+        takes more memory"
>+        https://bugs.webkit.org/show_bug.cgi?id=194469
>+        https://trac.webkit.org/changeset/241237
>+
> 2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [WTF] Use BufferInternal StringImpl if substring StringImpl takes more memory
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 058dbb7cb8865fa22a19c2f5b4c07d771f9e29d2..e7bce65195fa66548a16a91cf193f91876a8001c 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-02-09  Commit Queue  <commit-queue@webkit.org>
>+
>+        Unreviewed, rolling out r241237.
>+        https://bugs.webkit.org/show_bug.cgi?id=194474
>+
>+        Shows significant memory increase in WSL (Requested by
>+        yusukesuzuki on #webkit).
>+
>+        Reverted changeset:
>+
>+        "[WTF] Use BufferInternal StringImpl if substring StringImpl
>+        takes more memory"
>+        https://bugs.webkit.org/show_bug.cgi?id=194469
>+        https://trac.webkit.org/changeset/241237
>+
> 2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [WTF] Use BufferInternal StringImpl if substring StringImpl takes more memory
>diff --git a/Source/JavaScriptCore/runtime/JSString.h b/Source/JavaScriptCore/runtime/JSString.h
>index 932b9f2f5caf84c046c49d47f7fe945cfd591ad8..112283f80bcfc138c3ac9b12d21342850cc55a77 100644
>--- a/Source/JavaScriptCore/runtime/JSString.h
>+++ b/Source/JavaScriptCore/runtime/JSString.h
>@@ -636,10 +636,7 @@ inline JSString* jsSubstring(VM* vm, const String& s, unsigned offset, unsigned
>         if (c <= maxSingleCharacterString)
>             return vm->smallStrings.singleCharacterString(c);
>     }
>-    auto impl = StringImpl::createSubstringSharingImpl(*s.impl(), offset, length);
>-    if (impl->isSubString())
>-        return JSString::createHasOtherOwner(*vm, WTFMove(impl));
>-    return JSString::create(*vm, WTFMove(impl));
>+    return JSString::createHasOtherOwner(*vm, StringImpl::createSubstringSharingImpl(*s.impl(), offset, length));
> }
> 
> 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 85ca50e249a97c54217c28f5902f3c54dc352373..1321771e535bbd7183455bda73fc3d827bac8cb4 100644
>--- a/Source/WTF/wtf/text/StringImpl.h
>+++ b/Source/WTF/wtf/text/StringImpl.h
>@@ -297,7 +297,9 @@ public:
>     
>     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);
>@@ -935,20 +937,10 @@ 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(substringSize));
>+    auto* stringImpl = static_cast<StringImpl*>(fastMalloc(allocationSize<StringImpl*>(1)));
>     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));

Actions: View | Formatted Diff | Diff

Attachments on bug 194474: 361608