[webkit-changes] [219510] trunk

Fri, 14 Jul 2017 10:37:59 -0700

Title: [219510] trunk
Revision
219510
Author
[email protected]
Date
2017-07-14 10:37:26 -0700 (Fri, 14 Jul 2017)

Log Message

[WTF] Newly added AtomicStringImpl should use BufferInternal static string if StringImpl is static
https://bugs.webkit.org/show_bug.cgi?id=174501

Reviewed by Darin Adler.

Source/WTF:

When creating AtomicStringImpl from static StringImpl, we can just use createWithoutCopying
to create a BufferInternal AtomicStringImpl which m_data{8,16} is static string's one.

* wtf/text/AtomicStringImpl.cpp:
(WTF::CStringTranslator::hash):
(WTF::CStringTranslator::equal):
(WTF::CStringTranslator::translate):
(WTF::AtomicStringImpl::add):
(WTF::HashTranslatorCharBuffer::HashTranslatorCharBuffer):
(WTF::UCharBufferTranslator::hash):
(WTF::UCharBufferTranslator::equal):
(WTF::UCharBufferTranslator::translate):
(WTF::LCharBufferTranslator::hash):
(WTF::LCharBufferTranslator::equal):
(WTF::LCharBufferTranslator::translate):
(WTF::BufferFromStaticDataTranslator::hash):
(WTF::BufferFromStaticDataTranslator::equal):
(WTF::BufferFromStaticDataTranslator::translate):
(WTF::AtomicStringImpl::addLiteral):
(WTF::addSymbol):
(WTF::addStatic):
(WTF::AtomicStringImpl::addSlowCase):
(WTF::AtomicStringImpl::lookUp):
(WTF::CharBufferFromLiteralDataTranslator::hash): Deleted.
(WTF::CharBufferFromLiteralDataTranslator::equal): Deleted.
(WTF::CharBufferFromLiteralDataTranslator::translate): Deleted.
(WTF::addSubstring): Deleted.
* wtf/text/StringImpl.h:

Tools:

* TestWebKitAPI/Tests/WTF/StringImpl.cpp:
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (219509 => 219510)


--- trunk/Source/WTF/ChangeLog	2017-07-14 17:33:28 UTC (rev 219509)
+++ trunk/Source/WTF/ChangeLog	2017-07-14 17:37:26 UTC (rev 219510)
@@ -1,5 +1,41 @@
 2017-07-14  Yusuke Suzuki  <[email protected]>
 
+        [WTF] Newly added AtomicStringImpl should use BufferInternal static string if StringImpl is static
+        https://bugs.webkit.org/show_bug.cgi?id=174501
+
+        Reviewed by Darin Adler.
+
+        When creating AtomicStringImpl from static StringImpl, we can just use createWithoutCopying
+        to create a BufferInternal AtomicStringImpl which m_data{8,16} is static string's one.
+
+        * wtf/text/AtomicStringImpl.cpp:
+        (WTF::CStringTranslator::hash):
+        (WTF::CStringTranslator::equal):
+        (WTF::CStringTranslator::translate):
+        (WTF::AtomicStringImpl::add):
+        (WTF::HashTranslatorCharBuffer::HashTranslatorCharBuffer):
+        (WTF::UCharBufferTranslator::hash):
+        (WTF::UCharBufferTranslator::equal):
+        (WTF::UCharBufferTranslator::translate):
+        (WTF::LCharBufferTranslator::hash):
+        (WTF::LCharBufferTranslator::equal):
+        (WTF::LCharBufferTranslator::translate):
+        (WTF::BufferFromStaticDataTranslator::hash):
+        (WTF::BufferFromStaticDataTranslator::equal):
+        (WTF::BufferFromStaticDataTranslator::translate):
+        (WTF::AtomicStringImpl::addLiteral):
+        (WTF::addSymbol):
+        (WTF::addStatic):
+        (WTF::AtomicStringImpl::addSlowCase):
+        (WTF::AtomicStringImpl::lookUp):
+        (WTF::CharBufferFromLiteralDataTranslator::hash): Deleted.
+        (WTF::CharBufferFromLiteralDataTranslator::equal): Deleted.
+        (WTF::CharBufferFromLiteralDataTranslator::translate): Deleted.
+        (WTF::addSubstring): Deleted.
+        * wtf/text/StringImpl.h:
+
+2017-07-14  Yusuke Suzuki  <[email protected]>
+
         [WTF] Use std::unique_ptr for StackTrace
         https://bugs.webkit.org/show_bug.cgi?id=174495

Modified: trunk/Source/WTF/wtf/text/AtomicStringImpl.cpp (219509 => 219510)


--- trunk/Source/WTF/wtf/text/AtomicStringImpl.cpp	2017-07-14 17:33:28 UTC (rev 219509)
+++ trunk/Source/WTF/wtf/text/AtomicStringImpl.cpp	2017-07-14 17:37:26 UTC (rev 219510)
@@ -95,55 +95,70 @@
 }
 
 struct CStringTranslator {
-    static unsigned hash(const LChar* c)
+    static unsigned hash(const LChar* characters)
     {
-        return StringHasher::computeHashAndMaskTop8Bits(c);
+        return StringHasher::computeHashAndMaskTop8Bits(characters);
     }
 
-    static inline bool equal(StringImpl* r, const LChar* s)
+    static inline bool equal(StringImpl* str, const LChar* characters)
     {
-        return WTF::equal(r, s);
+        return WTF::equal(str, characters);
     }
 
-    static void translate(StringImpl*& location, const LChar* const& c, unsigned hash)
+    static void translate(StringImpl*& location, const LChar* const& characters, unsigned hash)
     {
-        location = &StringImpl::create(c).leakRef();
+        location = &StringImpl::create(characters).leakRef();
         location->setHash(hash);
         location->setIsAtomic(true);
     }
 };
 
-RefPtr<AtomicStringImpl> AtomicStringImpl::add(const LChar* c)
+RefPtr<AtomicStringImpl> AtomicStringImpl::add(const LChar* characters)
 {
-    if (!c)
+    if (!characters)
         return nullptr;
-    if (!*c)
+    if (!*characters)
         return static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    return addToStringTable<const LChar*, CStringTranslator>(c);
+    return addToStringTable<const LChar*, CStringTranslator>(characters);
 }
 
 template<typename CharacterType>
 struct HashTranslatorCharBuffer {
-    const CharacterType* s;
+    const CharacterType* characters;
     unsigned length;
+    unsigned hash;
+
+    HashTranslatorCharBuffer(const CharacterType* characters, unsigned length)
+        : characters(characters)
+        , length(length)
+        , hash(StringHasher::computeHashAndMaskTop8Bits(characters, length))
+    {
+    }
+
+    HashTranslatorCharBuffer(const CharacterType* characters, unsigned length, unsigned hash)
+        : characters(characters)
+        , length(length)
+        , hash(hash)
+    {
+    }
 };
 
-typedef HashTranslatorCharBuffer<UChar> UCharBuffer;
+using UCharBuffer = HashTranslatorCharBuffer<UChar>;
 struct UCharBufferTranslator {
     static unsigned hash(const UCharBuffer& buf)
     {
-        return StringHasher::computeHashAndMaskTop8Bits(buf.s, buf.length);
+        return buf.hash;
     }
 
     static bool equal(StringImpl* const& str, const UCharBuffer& buf)
     {
-        return WTF::equal(str, buf.s, buf.length);
+        return WTF::equal(str, buf.characters, buf.length);
     }
 
     static void translate(StringImpl*& location, const UCharBuffer& buf, unsigned hash)
     {
-        location = &StringImpl::create8BitIfPossible(buf.s, buf.length).leakRef();
+        location = &StringImpl::create8BitIfPossible(buf.characters, buf.length).leakRef();
         location->setHash(hash);
         location->setIsAtomic(true);
     }
@@ -217,31 +232,31 @@
     }
 };
 
-RefPtr<AtomicStringImpl> AtomicStringImpl::add(const UChar* s, unsigned length)
+RefPtr<AtomicStringImpl> AtomicStringImpl::add(const UChar* characters, unsigned length)
 {
-    if (!s)
+    if (!characters)
         return nullptr;
 
     if (!length)
         return static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    UCharBuffer buffer = { s, length };
+    UCharBuffer buffer { characters, length };
     return addToStringTable<UCharBuffer, UCharBufferTranslator>(buffer);
 }
 
-RefPtr<AtomicStringImpl> AtomicStringImpl::add(const UChar* s)
+RefPtr<AtomicStringImpl> AtomicStringImpl::add(const UChar* characters)
 {
-    if (!s)
+    if (!characters)
         return nullptr;
 
     unsigned length = 0;
-    while (s[length] != UChar(0))
+    while (characters[length] != UChar(0))
         ++length;
 
     if (!length)
         return static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    UCharBuffer buffer = { s, length };
+    UCharBuffer buffer { characters, length };
     return addToStringTable<UCharBuffer, UCharBufferTranslator>(buffer);
 }
 
@@ -305,55 +320,56 @@
     return addToStringTable<SubstringLocation, SubstringTranslator16>(buffer);
 }
     
-typedef HashTranslatorCharBuffer<LChar> LCharBuffer;
+using LCharBuffer = HashTranslatorCharBuffer<LChar>;
 struct LCharBufferTranslator {
     static unsigned hash(const LCharBuffer& buf)
     {
-        return StringHasher::computeHashAndMaskTop8Bits(buf.s, buf.length);
+        return buf.hash;
     }
 
     static bool equal(StringImpl* const& str, const LCharBuffer& buf)
     {
-        return WTF::equal(str, buf.s, buf.length);
+        return WTF::equal(str, buf.characters, buf.length);
     }
 
     static void translate(StringImpl*& location, const LCharBuffer& buf, unsigned hash)
     {
-        location = &StringImpl::create(buf.s, buf.length).leakRef();
+        location = &StringImpl::create(buf.characters, buf.length).leakRef();
         location->setHash(hash);
         location->setIsAtomic(true);
     }
 };
 
-typedef HashTranslatorCharBuffer<char> CharBuffer;
-struct CharBufferFromLiteralDataTranslator {
-    static unsigned hash(const CharBuffer& buf)
+template<typename CharType>
+struct BufferFromStaticDataTranslator {
+    using Buffer = HashTranslatorCharBuffer<CharType>;
+    static unsigned hash(const Buffer& buf)
     {
-        return StringHasher::computeHashAndMaskTop8Bits(reinterpret_cast<const LChar*>(buf.s), buf.length);
+        return buf.hash;
     }
 
-    static bool equal(StringImpl* const& str, const CharBuffer& buf)
+    static bool equal(StringImpl* const& str, const Buffer& buf)
     {
-        return WTF::equal(str, buf.s, buf.length);
+        return WTF::equal(str, buf.characters, buf.length);
     }
 
-    static void translate(StringImpl*& location, const CharBuffer& buf, unsigned hash)
+    static void translate(StringImpl*& location, const Buffer& buf, unsigned hash)
     {
-        location = &StringImpl::createFromLiteral(buf.s, buf.length).leakRef();
+        location = &StringImpl::createWithoutCopying(buf.characters, buf.length).leakRef();
         location->setHash(hash);
         location->setIsAtomic(true);
     }
 };
 
-RefPtr<AtomicStringImpl> AtomicStringImpl::add(const LChar* s, unsigned length)
+RefPtr<AtomicStringImpl> AtomicStringImpl::add(const LChar* characters, unsigned length)
 {
-    if (!s)
+    if (!characters)
         return nullptr;
 
     if (!length)
         return static_cast<AtomicStringImpl*>(StringImpl::empty());
 
-    LCharBuffer buffer = { s, length };
+    LCharBuffer buffer { characters, length };
     return addToStringTable<LCharBuffer, LCharBufferTranslator>(buffer);
 }
 
@@ -362,14 +378,14 @@
     ASSERT(characters);
     ASSERT(length);
 
-    CharBuffer buffer = { characters, length };
-    return addToStringTable<CharBuffer, CharBufferFromLiteralDataTranslator>(buffer);
+    LCharBuffer buffer { reinterpret_cast<const LChar*>(characters), length };
+    return addToStringTable<LCharBuffer, BufferFromStaticDataTranslator<LChar>>(buffer);
 }
 
-static inline Ref<AtomicStringImpl> addSubstring(AtomicStringTableLocker& locker, StringTableImpl& atomicStringTable, StringImpl& base)
+static Ref<AtomicStringImpl> addSymbol(AtomicStringTableLocker& locker, StringTableImpl& atomicStringTable, StringImpl& base)
 {
     ASSERT(base.length());
-    ASSERT(base.isSymbol() || base.isStatic());
+    ASSERT(base.isSymbol());
 
     SubstringLocation buffer = { &base, 0, base.length() };
     if (base.is8Bit())
@@ -377,20 +393,41 @@
     return addToStringTable<SubstringLocation, SubstringTranslator16>(locker, atomicStringTable, buffer);
 }
 
-static inline Ref<AtomicStringImpl> addSubstring(StringImpl& base)
+static inline Ref<AtomicStringImpl> addSymbol(StringImpl& base)
 {
     AtomicStringTableLocker locker;
-    return addSubstring(locker, stringTable(), base);
+    return addSymbol(locker, stringTable(), base);
 }
 
+static Ref<AtomicStringImpl> addStatic(AtomicStringTableLocker& locker, StringTableImpl& atomicStringTable, StringImpl& base)
+{
+    ASSERT(base.length());
+    ASSERT(base.isStatic());
+
+    if (base.is8Bit()) {
+        LCharBuffer buffer { base.characters8(), base.length(), base.hash() };
+        return addToStringTable<LCharBuffer, BufferFromStaticDataTranslator<LChar>>(locker, atomicStringTable, buffer);
+    }
+    UCharBuffer buffer { base.characters16(), base.length(), base.hash() };
+    return addToStringTable<UCharBuffer, BufferFromStaticDataTranslator<UChar>>(locker, atomicStringTable, buffer);
+}
+
+static inline Ref<AtomicStringImpl> addStatic(StringImpl& base)
+{
+    AtomicStringTableLocker locker;
+    return addStatic(locker, stringTable(), base);
+}
+
 Ref<AtomicStringImpl> AtomicStringImpl::addSlowCase(StringImpl& string)
 {
-    if (!string.length())
-        return *static_cast<AtomicStringImpl*>(StringImpl::empty());
+    ASSERT_WITH_MESSAGE(string.length(), "Empty string should be already handled.");
 
-    if (string.isSymbol() || string.isStatic())
-        return addSubstring(string);
+    if (string.isStatic())
+        return addStatic(string);
 
+    if (string.isSymbol())
+        return addSymbol(string);
+
     ASSERT_WITH_MESSAGE(!string.isAtomic(), "AtomicStringImpl should not hit the slow case if the string is already atomic.");
 
     AtomicStringTableLocker locker;
@@ -406,14 +443,18 @@
 
 Ref<AtomicStringImpl> AtomicStringImpl::addSlowCase(AtomicStringTable& stringTable, StringImpl& string)
 {
-    if (!string.length())
-        return *static_cast<AtomicStringImpl*>(StringImpl::empty());
+    ASSERT_WITH_MESSAGE(string.length(), "Empty string should be already handled.");
 
-    if (string.isSymbol() || string.isStatic()) {
+    if (string.isStatic()) {
         AtomicStringTableLocker locker;
-        return addSubstring(locker, stringTable.table(), string);
+        return addStatic(locker, stringTable.table(), string);
     }
 
+    if (string.isSymbol()) {
+        AtomicStringTableLocker locker;
+        return addSymbol(locker, stringTable.table(), string);
+    }
+
     ASSERT_WITH_MESSAGE(!string.isAtomic(), "AtomicStringImpl should not hit the slow case if the string is already atomic.");
 
     AtomicStringTableLocker locker;
@@ -482,7 +523,7 @@
     AtomicStringTableLocker locker;
     auto& table = stringTable();
 
-    UCharBuffer buffer = { characters, length };
+    UCharBuffer buffer { characters, length };
     auto iterator = table.find<UCharBufferTranslator>(buffer);
     if (iterator != table.end())
         return static_cast<AtomicStringImpl*>(*iterator);

Modified: trunk/Source/WTF/wtf/text/StringImpl.h (219509 => 219510)


--- trunk/Source/WTF/wtf/text/StringImpl.h	2017-07-14 17:33:28 UTC (rev 219509)
+++ trunk/Source/WTF/wtf/text/StringImpl.h	2017-07-14 17:37:26 UTC (rev 219510)
@@ -55,7 +55,7 @@
 class SymbolRegistry;
 
 struct CStringTranslator;
-struct CharBufferFromLiteralDataTranslator;
+template<typename> struct BufferFromStaticDataTranslator;
 struct HashAndUTF8CharactersTranslator;
 struct LCharBufferTranslator;
 struct StringHash;
@@ -182,12 +182,13 @@
     friend struct WTF::CStringTranslator;
     template<typename CharacterType> friend struct WTF::HashAndCharactersTranslator;
     friend struct WTF::HashAndUTF8CharactersTranslator;
-    friend struct WTF::CharBufferFromLiteralDataTranslator;
+    template<typename CharacterType> friend struct WTF::BufferFromStaticDataTranslator;
     friend struct WTF::LCharBufferTranslator;
     friend struct WTF::SubstringTranslator;
     friend struct WTF::UCharBufferTranslator;
     friend class JSC::LLInt::Data;
     friend class JSC::LLIntOffsetsExtractor;
+    friend class AtomicStringImpl;
     friend class SymbolImpl;
     friend class RegisteredSymbolImpl;

Modified: trunk/Tools/ChangeLog (219509 => 219510)


--- trunk/Tools/ChangeLog	2017-07-14 17:33:28 UTC (rev 219509)
+++ trunk/Tools/ChangeLog	2017-07-14 17:37:26 UTC (rev 219510)
@@ -1,3 +1,13 @@
+2017-07-14  Yusuke Suzuki  <[email protected]>
+
+        [WTF] Newly added AtomicStringImpl should use BufferInternal static string if StringImpl is static
+        https://bugs.webkit.org/show_bug.cgi?id=174501
+
+        Reviewed by Darin Adler.
+
+        * TestWebKitAPI/Tests/WTF/StringImpl.cpp:
+        (TestWebKitAPI::TEST):
+
 2017-07-14  Jonathan Bedard  <[email protected]>
 
         Bring mac expectations in-line with other platforms

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp (219509 => 219510)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp	2017-07-14 17:33:28 UTC (rev 219509)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp	2017-07-14 17:37:26 UTC (rev 219510)
@@ -55,6 +55,17 @@
     ASSERT_TRUE(equal(programmaticStringNoLength.get(), stringWithoutLengthLiteral));
     ASSERT_EQ(stringWithoutLengthLiteral, reinterpret_cast<const char*>(programmaticStringNoLength->characters8()));
     ASSERT_TRUE(programmaticStringNoLength->is8Bit());
+
+    // AtomicStringImpl from createFromLiteral should use the same underlying string.
+    auto atomicStringWithTemplate = AtomicStringImpl::add(stringWithTemplate.ptr());
+    ASSERT_TRUE(atomicStringWithTemplate->is8Bit());
+    ASSERT_EQ(atomicStringWithTemplate->characters8(), stringWithTemplate->characters8());
+    auto atomicProgrammaticString = AtomicStringImpl::add(programmaticString.ptr());
+    ASSERT_TRUE(atomicProgrammaticString->is8Bit());
+    ASSERT_EQ(atomicProgrammaticString->characters8(), programmaticString->characters8());
+    auto atomicProgrammaticStringNoLength = AtomicStringImpl::add(programmaticStringNoLength.ptr());
+    ASSERT_TRUE(atomicProgrammaticStringNoLength->is8Bit());
+    ASSERT_EQ(atomicProgrammaticStringNoLength->characters8(), programmaticStringNoLength->characters8());
 }
 
 TEST(WTF, StringImplReplaceWithLiteral)
@@ -609,8 +620,12 @@
     ASSERT_FALSE(original.isAtomic());
     ASSERT_TRUE(original.isStatic());
 
+    ASSERT_TRUE(atomic->is8Bit());
+    ASSERT_EQ(atomic->characters8(), original.characters8());
+
     auto result2 = AtomicStringImpl::lookUp(&original);
     ASSERT_TRUE(result2);
+    ASSERT_EQ(atomic, result2);
 }
 
 TEST(WTF, StringImplConstexprHasher)

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to