Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (209308 => 209309)
--- trunk/Source/_javascript_Core/ChangeLog 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-12-04 00:28:49 UTC (rev 209309)
@@ -1,3 +1,15 @@
+2016-12-03 Yusuke Suzuki <utatane....@gmail.com>
+
+ Refactor SymbolImpl layout
+ https://bugs.webkit.org/show_bug.cgi?id=165247
+
+ Reviewed by Darin Adler.
+
+ Use SymbolImpl::{create, createNullSymbol} instead.
+
+ * runtime/PrivateName.h:
+ (JSC::PrivateName::PrivateName):
+
2016-12-03 JF Bastien <jfbast...@apple.com>
WebAssembly: update binary format to 0xD version
Modified: trunk/Source/_javascript_Core/runtime/PrivateName.h (209308 => 209309)
--- trunk/Source/_javascript_Core/runtime/PrivateName.h 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/_javascript_Core/runtime/PrivateName.h 2016-12-04 00:28:49 UTC (rev 209309)
@@ -32,7 +32,7 @@
class PrivateName {
public:
PrivateName()
- : m_uid(StringImpl::createNullSymbol())
+ : m_uid(SymbolImpl::createNullSymbol())
{
}
@@ -43,7 +43,7 @@
enum DescriptionTag { Description };
explicit PrivateName(DescriptionTag, const String& description)
- : m_uid(StringImpl::createSymbol(*description.impl()))
+ : m_uid(SymbolImpl::create(*description.impl()))
{
}
Modified: trunk/Source/WTF/ChangeLog (209308 => 209309)
--- trunk/Source/WTF/ChangeLog 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/ChangeLog 2016-12-04 00:28:49 UTC (rev 209309)
@@ -1,3 +1,62 @@
+2016-12-03 Yusuke Suzuki <utatane....@gmail.com>
+
+ Refactor SymbolImpl layout
+ https://bugs.webkit.org/show_bug.cgi?id=165247
+
+ Reviewed by Darin Adler.
+
+ This patch moves SymbolImpl initialization from StringImpl to SymbolImpl.
+ In SymbolImpl, we create the appropriate fields. At that time, these fields
+ should be aligned to the BufferSubstring StringImpl.
+
+ And we newly create the `m_flags` in SymbolImpl. Instead of using special
+ StringImpl::null(), we store s_flagIsNullSymbol flag here. In WTF, we have
+ the invariant that StringImpl::empty() is the only atomic empty string.
+ But StringImpl::null() breaks this invariant. Using a special flag is safer
+ way to represent the null Symbol `Symbol()`.
+
+ * WTF.xcodeproj/project.pbxproj:
+ * wtf/CMakeLists.txt:
+ * wtf/StdLibExtras.h:
+ (WTF::roundUpToMultipleOfImpl0):
+ (WTF::roundUpToMultipleOfImpl):
+ (WTF::roundUpToMultipleOf):
+ * wtf/text/StringImpl.cpp:
+ (WTF::StringImpl::~StringImpl):
+ (WTF::StringImpl::createSymbol): Deleted.
+ (WTF::StringImpl::createNullSymbol): Deleted.
+ * wtf/text/StringImpl.h:
+ (WTF::StringImpl::isAtomic):
+ (WTF::StringImpl::StringImpl):
+ (WTF::StringImpl::requiresCopy):
+ (WTF::StringImpl::isNullSymbol): Deleted.
+ (WTF::StringImpl::symbolAwareHash): Deleted.
+ (WTF::StringImpl::existingSymbolAwareHash): Deleted.
+ (WTF::StringImpl::null): Deleted.
+ (WTF::StringImpl::extractFoldedStringInSymbol): Deleted.
+ (WTF::StringImpl::symbolRegistry): Deleted.
+ (WTF::StringImpl::hashForSymbol): Deleted.
+ * wtf/text/StringStatics.cpp:
+ (WTF::StringImpl::nextHashForSymbol): Deleted.
+ * wtf/text/SymbolImpl.cpp: Copied from Source/WTF/wtf/text/SymbolRegistry.cpp.
+ (WTF::SymbolImpl::nextHashForSymbol):
+ (WTF::SymbolImpl::create):
+ (WTF::SymbolImpl::createNullSymbol):
+ * wtf/text/SymbolImpl.h:
+ (WTF::SymbolImpl::hashForSymbol):
+ (WTF::SymbolImpl::symbolRegistry):
+ (WTF::SymbolImpl::isNullSymbol):
+ (WTF::SymbolImpl::extractFoldedString):
+ (WTF::SymbolImpl::SymbolImpl):
+ (WTF::StringImpl::symbolAwareHash):
+ (WTF::StringImpl::existingSymbolAwareHash):
+ * wtf/text/SymbolRegistry.cpp:
+ (WTF::SymbolRegistry::~SymbolRegistry):
+ (WTF::SymbolRegistry::symbolForKey):
+ (WTF::SymbolRegistry::keyForSymbol):
+ * wtf/text/UniquedStringImpl.h:
+ (WTF::UniquedStringImpl::UniquedStringImpl):
+
2016-12-01 Yusuke Suzuki <utatane....@gmail.com>
Introduce StringImpl::StaticStringImpl with constexpr constructor
Modified: trunk/Source/WTF/WTF.xcodeproj/project.pbxproj (209308 => 209309)
--- trunk/Source/WTF/WTF.xcodeproj/project.pbxproj 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/WTF.xcodeproj/project.pbxproj 2016-12-04 00:28:49 UTC (rev 209309)
@@ -355,6 +355,7 @@
FE8925B01D00DAEC0046907E /* Indenter.h in Headers */ = {isa = PBXBuildFile; fileRef = FE8925AF1D00DAEC0046907E /* Indenter.h */; };
FEDACD3D1630F83F00C69634 /* StackStats.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FEDACD3B1630F83F00C69634 /* StackStats.cpp */; };
FEDACD3E1630F83F00C69634 /* StackStats.h in Headers */ = {isa = PBXBuildFile; fileRef = FEDACD3C1630F83F00C69634 /* StackStats.h */; };
+ 52183012C99E476A84EEBEA8 /* SymbolImpl.cpp in Sources */ = {isa = PBXBuildFile; fileRef = F72BBDB107FA424886178B9E /* SymbolImpl.cpp */; };
/* End PBXBuildFile section */
/* Begin PBXContainerItemProxy section */
@@ -722,6 +723,7 @@
FE8925AF1D00DAEC0046907E /* Indenter.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Indenter.h; sourceTree = "<group>"; };
FEDACD3B1630F83F00C69634 /* StackStats.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = StackStats.cpp; sourceTree = "<group>"; };
FEDACD3C1630F83F00C69634 /* StackStats.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StackStats.h; sourceTree = "<group>"; };
+ F72BBDB107FA424886178B9E /* SymbolImpl.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SymbolImpl.cpp; path = SymbolImpl.cpp; sourceTree = "<group>"; };
/* End PBXFileReference section */
/* Begin PBXFrameworksBuildPhase section */
@@ -1187,6 +1189,7 @@
70ECA60C1B02426800449739 /* UniquedStringImpl.h */,
A8A4732D151A825B004123FF /* WTFString.cpp */,
A8A4732E151A825B004123FF /* WTFString.h */,
+ F72BBDB107FA424886178B9E /* SymbolImpl.cpp */,
);
path = text;
sourceTree = "<group>";
@@ -1710,6 +1713,7 @@
E4A0AD3D1A96253C00536DF6 /* WorkQueueCocoa.cpp in Sources */,
A8A47445151A825B004123FF /* WTFString.cpp in Sources */,
A8A47486151A825B004123FF /* WTFThreadData.cpp in Sources */,
+ 52183012C99E476A84EEBEA8 /* SymbolImpl.cpp in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Modified: trunk/Source/WTF/wtf/CMakeLists.txt (209308 => 209309)
--- trunk/Source/WTF/wtf/CMakeLists.txt 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/wtf/CMakeLists.txt 2016-12-04 00:28:49 UTC (rev 209309)
@@ -246,6 +246,7 @@
text/StringImpl.cpp
text/StringStatics.cpp
text/StringView.cpp
+ text/SymbolImpl.cpp
text/SymbolRegistry.cpp
text/TextBreakIterator.cpp
text/WTFString.cpp
Modified: trunk/Source/WTF/wtf/StdLibExtras.h (209308 => 209309)
--- trunk/Source/WTF/wtf/StdLibExtras.h 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/wtf/StdLibExtras.h 2016-12-04 00:28:49 UTC (rev 209309)
@@ -186,18 +186,27 @@
#endif
#define WTF_ARRAY_LENGTH(array) sizeof(::WTF::ArrayLengthHelperFunction(array))
+ALWAYS_INLINE constexpr size_t roundUpToMultipleOfImpl0(size_t remainderMask, size_t x)
+{
+ return (x + remainderMask) & ~remainderMask;
+}
+
+ALWAYS_INLINE constexpr size_t roundUpToMultipleOfImpl(size_t divisor, size_t x)
+{
+ return roundUpToMultipleOfImpl0(divisor - 1, x);
+}
+
// Efficient implementation that takes advantage of powers of two.
inline size_t roundUpToMultipleOf(size_t divisor, size_t x)
{
ASSERT(divisor && !(divisor & (divisor - 1)));
- size_t remainderMask = divisor - 1;
- return (x + remainderMask) & ~remainderMask;
+ return roundUpToMultipleOfImpl(divisor, x);
}
-template<size_t divisor> inline size_t roundUpToMultipleOf(size_t x)
+template<size_t divisor> inline constexpr size_t roundUpToMultipleOf(size_t x)
{
static_assert(divisor && !(divisor & (divisor - 1)), "divisor must be a power of two!");
- return roundUpToMultipleOf(divisor, x);
+ return roundUpToMultipleOfImpl(divisor, x);
}
enum BinarySearchMode {
Modified: trunk/Source/WTF/wtf/text/StringImpl.cpp (209308 => 209309)
--- trunk/Source/WTF/wtf/text/StringImpl.cpp 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/wtf/text/StringImpl.cpp 2016-12-04 00:28:49 UTC (rev 209309)
@@ -101,7 +101,6 @@
}
#endif
-StringImpl::StaticStringImpl StringImpl::s_atomicNullString("", StringImpl::StringAtomic);
StringImpl::StaticStringImpl StringImpl::s_atomicEmptyString("", StringImpl::StringAtomic);
StringImpl::~StringImpl()
@@ -115,8 +114,12 @@
if (isAtomic() && length() && !isSymbol())
AtomicStringImpl::remove(static_cast<AtomicStringImpl*>(this));
- if (isSymbol() && symbolRegistry())
- symbolRegistry()->remove(static_cast<SymbolImpl&>(*this));
+ if (isSymbol()) {
+ auto& symbol = static_cast<SymbolImpl&>(*this);
+ auto* symbolRegistry = symbol.symbolRegistry();
+ if (symbolRegistry)
+ symbolRegistry->remove(symbol);
+ }
BufferOwnership ownership = bufferOwnership();
@@ -292,26 +295,6 @@
return create(string, length);
}
-Ref<SymbolImpl> StringImpl::createSymbol(StringImpl& rep)
-{
- auto* ownerRep = (rep.bufferOwnership() == BufferSubstring) ? rep.substringBuffer() : &rep;
-
- // We allocate a buffer that contains
- // 1. the StringImpl struct
- // 2. the pointer to the owner string
- // 3. the pointer to the symbol registry
- // 4. the placeholder for symbol aware hash value (allocated size is pointer size, but only 4 bytes are used)
- auto* stringImpl = static_cast<StringImpl*>(fastMalloc(allocationSize<StringImpl*>(3)));
- if (rep.is8Bit())
- return adoptRef(static_cast<SymbolImpl&>(*new (NotNull, stringImpl) StringImpl(CreateSymbol, rep.m_data8, rep.length(), *ownerRep)));
- return adoptRef(static_cast<SymbolImpl&>(*new (NotNull, stringImpl) StringImpl(CreateSymbol, rep.m_data16, rep.length(), *ownerRep)));
-}
-
-Ref<SymbolImpl> StringImpl::createNullSymbol()
-{
- return createSymbol(*null());
-}
-
bool StringImpl::containsOnlyWhitespace()
{
// FIXME: The definition of whitespace here includes a number of characters
Modified: trunk/Source/WTF/wtf/text/StringImpl.h (209308 => 209309)
--- trunk/Source/WTF/wtf/text/StringImpl.h 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/wtf/text/StringImpl.h 2016-12-04 00:28:49 UTC (rev 209309)
@@ -139,6 +139,7 @@
friend struct WTF::UCharBufferTranslator;
friend class JSC::LLInt::Data;
friend class JSC::LLIntOffsetsExtractor;
+ friend class SymbolImpl;
private:
enum BufferOwnership {
@@ -281,43 +282,6 @@
STRING_STATS_ADD_16BIT_STRING2(m_length, true);
}
- enum CreateSymbolTag { CreateSymbol };
- // Used to create new symbol strings that holds existing 8-bit [[Description]] string as a substring buffer (BufferSubstring).
- StringImpl(CreateSymbolTag, const LChar* characters, unsigned length, Ref<StringImpl>&& base)
- : m_refCount(s_refCountIncrement)
- , m_length(length)
- , m_data8(characters)
- , m_hashAndFlags(s_hashFlag8BitBuffer | StringSymbol | BufferSubstring)
- {
- ASSERT(is8Bit());
- ASSERT(m_data8);
- ASSERT(base->bufferOwnership() != BufferSubstring);
-
- substringBuffer() = &base.leakRef();
- symbolRegistry() = nullptr;
- hashForSymbol() = nextHashForSymbol();
-
- STRING_STATS_ADD_8BIT_STRING2(m_length, true);
- }
-
- // Used to create new symbol strings that holds existing 16-bit [[Description]] string as a substring buffer (BufferSubstring).
- StringImpl(CreateSymbolTag, const UChar* characters, unsigned length, Ref<StringImpl>&& base)
- : m_refCount(s_refCountIncrement)
- , m_length(length)
- , m_data16(characters)
- , m_hashAndFlags(StringSymbol | BufferSubstring)
- {
- ASSERT(!is8Bit());
- ASSERT(m_data16);
- ASSERT(base->bufferOwnership() != BufferSubstring);
-
- substringBuffer() = &base.leakRef();
- symbolRegistry() = nullptr;
- hashForSymbol() = nextHashForSymbol();
-
- STRING_STATS_ADD_16BIT_STRING2(m_length, true);
- }
-
public:
WTF_EXPORT_STRING_API static void destroy(StringImpl*);
@@ -390,9 +354,6 @@
return constructInternal<T>(resultImpl, length);
}
- WTF_EXPORT_STRING_API static Ref<SymbolImpl> createNullSymbol();
- WTF_EXPORT_STRING_API static Ref<SymbolImpl> createSymbol(StringImpl& rep);
-
// Reallocate the StringImpl. The originalString must be only owned by the Ref,
// and the buffer ownership must be BufferInternal. Just like the input pointer of realloc(),
// the originalString can't be used after this function.
@@ -466,7 +427,6 @@
StringKind stringKind() const { return static_cast<StringKind>(m_hashAndFlags & s_hashMaskStringKind); }
bool isSymbol() const { return m_hashAndFlags & s_hashFlagStringKindIsSymbol; }
bool isAtomic() const { return m_hashAndFlags & s_hashFlagStringKindIsAtomic; }
- bool isNullSymbol() const { return isSymbol() && (substringBuffer() == null()); }
void setIsAtomic(bool isAtomic)
{
@@ -536,20 +496,9 @@
WTF_EXPORT_PRIVATE unsigned concurrentHash() const;
- unsigned symbolAwareHash() const
- {
- if (isSymbol())
- return hashForSymbol();
- return hash();
- }
+ unsigned symbolAwareHash() const;
+ unsigned existingSymbolAwareHash() const;
- unsigned existingSymbolAwareHash() const
- {
- if (isSymbol())
- return hashForSymbol();
- return existingHash();
- }
-
bool isStatic() const { return m_refCount & s_refCountFlagIsStaticString; }
inline size_t refCount() const
@@ -620,9 +569,7 @@
unsigned m_hashAndFlags;
};
- WTF_EXPORTDATA static StaticStringImpl s_atomicNullString;
WTF_EXPORTDATA static StaticStringImpl s_atomicEmptyString;
- ALWAYS_INLINE static StringImpl* null() { return reinterpret_cast<StringImpl*>(&s_atomicNullString); }
ALWAYS_INLINE static StringImpl* empty() { return reinterpret_cast<StringImpl*>(&s_atomicEmptyString); }
// FIXME: Does this really belong in StringImpl?
@@ -778,52 +725,47 @@
ALWAYS_INLINE static StringStats& stringStats() { return m_stringStats; }
#endif
- Ref<StringImpl> extractFoldedStringInSymbol()
- {
- ASSERT(isSymbol());
- ASSERT(bufferOwnership() == BufferSubstring);
- ASSERT(substringBuffer());
- ASSERT(!substringBuffer()->isSymbol());
- return createSubstringSharingImpl(*this, 0, length());
- }
+protected:
+ ~StringImpl();
- SymbolRegistry* const& symbolRegistry() const
- {
- ASSERT(isSymbol());
- return *(tailPointer<SymbolRegistry*>() + 1);
- }
+ enum CreateSymbolTag { CreateSymbol };
- SymbolRegistry*& symbolRegistry()
+ // Used to create new symbol strings that holds existing 8-bit [[Description]] string as a substring buffer (BufferSubstring).
+ StringImpl(CreateSymbolTag, const LChar* characters, unsigned length)
+ : m_refCount(s_refCountIncrement)
+ , m_length(length)
+ , m_data8(characters)
+ , m_hashAndFlags(s_hashFlag8BitBuffer | StringSymbol | BufferSubstring)
{
- ASSERT(isSymbol());
- return *(tailPointer<SymbolRegistry*>() + 1);
+ ASSERT(is8Bit());
+ ASSERT(m_data8);
+ STRING_STATS_ADD_8BIT_STRING2(m_length, true);
}
- const unsigned& hashForSymbol() const
+ // Used to create new symbol strings that holds existing 16-bit [[Description]] string as a substring buffer (BufferSubstring).
+ StringImpl(CreateSymbolTag, const UChar* characters, unsigned length)
+ : m_refCount(s_refCountIncrement)
+ , m_length(length)
+ , m_data16(characters)
+ , m_hashAndFlags(StringSymbol | BufferSubstring)
{
- return const_cast<StringImpl*>(this)->hashForSymbol();
+ ASSERT(!is8Bit());
+ ASSERT(m_data16);
+ STRING_STATS_ADD_16BIT_STRING2(m_length, true);
}
- unsigned& hashForSymbol()
+ // Null symbol.
+ StringImpl(CreateSymbolTag)
+ : m_refCount(s_refCountIncrement)
+ , m_length(0)
+ , m_data8(empty()->characters8())
+ , m_hashAndFlags(s_hashFlag8BitBuffer | StringSymbol | BufferSubstring)
{
- ASSERT(isSymbol());
- return *reinterpret_cast<unsigned*>((tailPointer<SymbolRegistry*>() + 2));
+ ASSERT(is8Bit());
+ ASSERT(m_data8);
+ STRING_STATS_ADD_8BIT_STRING2(m_length, true);
}
-protected:
- ~StringImpl();
-
-private:
- bool requiresCopy() const
- {
- if (bufferOwnership() != BufferInternal)
- return true;
-
- if (is8Bit())
- return m_data8 == tailPointer<LChar>();
- return m_data16 == tailPointer<UChar>();
- }
-
template<typename T>
static size_t allocationSize(unsigned tailElementCount)
{
@@ -841,6 +783,17 @@
#endif
}
+private:
+ bool requiresCopy() const
+ {
+ if (bufferOwnership() != BufferInternal)
+ return true;
+
+ if (is8Bit())
+ return m_data8 == tailPointer<LChar>();
+ return m_data16 == tailPointer<UChar>();
+ }
+
template<typename T>
const T* tailPointer() const
{
@@ -882,7 +835,6 @@
template <typename CharType> static Ref<StringImpl> reallocateInternal(Ref<StringImpl>&&, unsigned, CharType*&);
template <typename CharType> static Ref<StringImpl> createInternal(const CharType*, unsigned);
WTF_EXPORT_PRIVATE NEVER_INLINE unsigned hashSlowCase() const;
- WTF_EXPORT_PRIVATE static unsigned nextHashForSymbol();
// The bottom bit in the ref count indicates a static (immortal) string.
static const unsigned s_refCountFlagIsStaticString = 0x1;
Modified: trunk/Source/WTF/wtf/text/StringStatics.cpp (209308 => 209309)
--- trunk/Source/WTF/wtf/text/StringStatics.cpp 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/wtf/text/StringStatics.cpp 2016-12-04 00:28:49 UTC (rev 209309)
@@ -41,20 +41,6 @@
namespace WTF {
-// In addition to the normal hash value, store specialized hash value for
-// symbolized StringImpl*. And don't use the normal hash value for symbolized
-// StringImpl* when they are treated as Identifiers. Unique nature of these
-// symbolized StringImpl* keys means that we don't need them to match any other
-// string (in fact, that's exactly the oposite of what we want!), and the
-// normal hash would lead to lots of conflicts.
-unsigned StringImpl::nextHashForSymbol()
-{
- static unsigned s_nextHashForSymbol = 0;
- s_nextHashForSymbol += 1 << s_flagCount;
- s_nextHashForSymbol |= 1 << 31;
- return s_nextHashForSymbol;
-}
-
WTF_EXPORTDATA DEFINE_GLOBAL(AtomicString, nullAtom)
WTF_EXPORTDATA DEFINE_GLOBAL(AtomicString, emptyAtom)
WTF_EXPORTDATA DEFINE_GLOBAL(AtomicString, starAtom)
Copied: trunk/Source/WTF/wtf/text/SymbolImpl.cpp (from rev 209308, trunk/Source/WTF/wtf/text/SymbolRegistry.cpp) (0 => 209309)
--- trunk/Source/WTF/wtf/text/SymbolImpl.cpp (rev 0)
+++ trunk/Source/WTF/wtf/text/SymbolImpl.cpp 2016-12-04 00:28:49 UTC (rev 209309)
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2016 Yusuke Suzuki <utatane....@gmail.com>.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "SymbolImpl.h"
+
+namespace WTF {
+
+// In addition to the normal hash value, store specialized hash value for
+// symbolized StringImpl*. And don't use the normal hash value for symbolized
+// StringImpl* when they are treated as Identifiers. Unique nature of these
+// symbolized StringImpl* keys means that we don't need them to match any other
+// string (in fact, that's exactly the oposite of what we want!), and the
+// normal hash would lead to lots of conflicts.
+unsigned SymbolImpl::nextHashForSymbol()
+{
+ static unsigned s_nextHashForSymbol = 0;
+ s_nextHashForSymbol += 1 << s_flagCount;
+ s_nextHashForSymbol |= 1 << 31;
+ return s_nextHashForSymbol;
+}
+
+Ref<SymbolImpl> SymbolImpl::create(StringImpl& rep)
+{
+ auto* ownerRep = (rep.bufferOwnership() == BufferSubstring) ? rep.substringBuffer() : &rep;
+ ASSERT(ownerRep->bufferOwnership() != BufferSubstring);
+ if (rep.is8Bit())
+ return adoptRef(*new SymbolImpl(rep.m_data8, rep.length(), *ownerRep));
+ return adoptRef(*new SymbolImpl(rep.m_data16, rep.length(), *ownerRep));
+}
+
+Ref<SymbolImpl> SymbolImpl::createNullSymbol()
+{
+ return adoptRef(*new SymbolImpl);
+}
+
+} // namespace WTF
Modified: trunk/Source/WTF/wtf/text/SymbolImpl.h (209308 => 209309)
--- trunk/Source/WTF/wtf/text/SymbolImpl.h 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/wtf/text/SymbolImpl.h 2016-12-04 00:28:49 UTC (rev 209309)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015 Yusuke Suzuki <utatane....@gmail.com>.
+ * Copyright (C) 2015-2016 Yusuke Suzuki <utatane....@gmail.com>.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -23,8 +23,7 @@
* THE POSSIBILITY OF SUCH DAMAGE.
*/
-#ifndef SymbolImpl_h
-#define SymbolImpl_h
+#pragma once
#include <wtf/text/UniquedStringImpl.h>
@@ -34,9 +33,77 @@
// It is uniqued string impl, but is not registered in Atomic String tables, so it's not atomic.
class SymbolImpl : public UniquedStringImpl {
private:
- SymbolImpl() = delete;
+ static constexpr const unsigned s_flagIsNullSymbol = 1u;
+
+public:
+ unsigned hashForSymbol() const { return m_hashForSymbol; }
+ SymbolRegistry* const& symbolRegistry() const { return m_symbolRegistry; }
+ SymbolRegistry*& symbolRegistry() { return m_symbolRegistry; }
+ bool isNullSymbol() const { return m_flags & s_flagIsNullSymbol; }
+
+ WTF_EXPORT_STRING_API static Ref<SymbolImpl> createNullSymbol();
+ WTF_EXPORT_STRING_API static Ref<SymbolImpl> create(StringImpl& rep);
+
+ Ref<StringImpl> extractFoldedString()
+ {
+ ASSERT(substringBuffer());
+ ASSERT(substringBuffer() == m_owner);
+ ASSERT(!substringBuffer()->isSymbol());
+ return createSubstringSharingImpl(*this, 0, length());
+ }
+
+private:
+ WTF_EXPORT_PRIVATE static unsigned nextHashForSymbol();
+
+ friend class StringImpl;
+
+ SymbolImpl(const LChar* characters, unsigned length, Ref<StringImpl>&& base)
+ : UniquedStringImpl(CreateSymbol, characters, length)
+ , m_owner(&base.leakRef())
+ , m_hashForSymbol(nextHashForSymbol())
+ {
+ ASSERT(StringImpl::tailOffset<StringImpl*>() == OBJECT_OFFSETOF(SymbolImpl, m_owner));
+ }
+
+ SymbolImpl(const UChar* characters, unsigned length, Ref<StringImpl>&& base)
+ : UniquedStringImpl(CreateSymbol, characters, length)
+ , m_owner(&base.leakRef())
+ , m_hashForSymbol(nextHashForSymbol())
+ {
+ ASSERT(StringImpl::tailOffset<StringImpl*>() == OBJECT_OFFSETOF(SymbolImpl, m_owner));
+ }
+
+ SymbolImpl()
+ : UniquedStringImpl(CreateSymbol)
+ , m_owner(StringImpl::empty())
+ , m_hashForSymbol(nextHashForSymbol())
+ , m_flags(s_flagIsNullSymbol)
+ {
+ ASSERT(StringImpl::tailOffset<StringImpl*>() == OBJECT_OFFSETOF(SymbolImpl, m_owner));
+ }
+
+ // The pointer to the owner string should be immediately following after the StringImpl layout,
+ // since we would like to align the layout of SymbolImpl to the one of BufferSubstring StringImpl.
+ StringImpl* m_owner;
+ SymbolRegistry* m_symbolRegistry { nullptr };
+ unsigned m_hashForSymbol;
+ unsigned m_flags { 0 };
};
+inline unsigned StringImpl::symbolAwareHash() const
+{
+ if (isSymbol())
+ return static_cast<const SymbolImpl*>(this)->hashForSymbol();
+ return hash();
+}
+
+inline unsigned StringImpl::existingSymbolAwareHash() const
+{
+ if (isSymbol())
+ return static_cast<const SymbolImpl*>(this)->hashForSymbol();
+ return existingHash();
+}
+
#if !ASSERT_DISABLED
// SymbolImpls created from StaticASCIILiteral will ASSERT
// in the generic ValueCheck<T>::checkConsistency
@@ -57,5 +124,3 @@
} // namespace WTF
using WTF::SymbolImpl;
-
-#endif // SymbolImpl_h
Modified: trunk/Source/WTF/wtf/text/SymbolRegistry.cpp (209308 => 209309)
--- trunk/Source/WTF/wtf/text/SymbolRegistry.cpp 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/wtf/text/SymbolRegistry.cpp 2016-12-04 00:28:49 UTC (rev 209309)
@@ -31,7 +31,7 @@
SymbolRegistry::~SymbolRegistry()
{
for (auto& key : m_table)
- key.impl()->symbolRegistry() = nullptr;
+ static_cast<SymbolImpl&>(*key.impl()).symbolRegistry() = nullptr;
}
Ref<SymbolImpl> SymbolRegistry::symbolForKey(const String& rep)
@@ -40,7 +40,7 @@
if (!addResult.isNewEntry)
return *static_cast<SymbolImpl*>(addResult.iterator->impl());
- auto symbol = StringImpl::createSymbol(*rep.impl());
+ auto symbol = SymbolImpl::create(*rep.impl());
symbol->symbolRegistry() = this;
*addResult.iterator = SymbolRegistryKey(&symbol.get());
return symbol;
@@ -49,7 +49,7 @@
String SymbolRegistry::keyForSymbol(SymbolImpl& uid)
{
ASSERT(uid.symbolRegistry() == this);
- return uid.extractFoldedStringInSymbol();
+ return uid.extractFoldedString();
}
void SymbolRegistry::remove(SymbolImpl& uid)
Modified: trunk/Source/WTF/wtf/text/UniquedStringImpl.h (209308 => 209309)
--- trunk/Source/WTF/wtf/text/UniquedStringImpl.h 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Source/WTF/wtf/text/UniquedStringImpl.h 2016-12-04 00:28:49 UTC (rev 209309)
@@ -35,6 +35,10 @@
class UniquedStringImpl : public StringImpl {
private:
UniquedStringImpl() = delete;
+protected:
+ UniquedStringImpl(CreateSymbolTag, const LChar* characters, unsigned length) : StringImpl(CreateSymbol, characters, length) { }
+ UniquedStringImpl(CreateSymbolTag, const UChar* characters, unsigned length) : StringImpl(CreateSymbol, characters, length) { }
+ UniquedStringImpl(CreateSymbolTag) : StringImpl(CreateSymbol) { }
};
#if !ASSERT_DISABLED
Modified: trunk/Tools/ChangeLog (209308 => 209309)
--- trunk/Tools/ChangeLog 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Tools/ChangeLog 2016-12-04 00:28:49 UTC (rev 209309)
@@ -1,3 +1,13 @@
+2016-12-03 Yusuke Suzuki <utatane....@gmail.com>
+
+ Refactor SymbolImpl layout
+ https://bugs.webkit.org/show_bug.cgi?id=165247
+
+ Reviewed by Darin Adler.
+
+ * TestWebKitAPI/Tests/WTF/StringImpl.cpp:
+ (TestWebKitAPI::TEST):
+
2016-12-03 Dan Bernstein <m...@apple.com>
Fixed the build after r209307.
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp (209308 => 209309)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp 2016-12-04 00:11:41 UTC (rev 209308)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp 2016-12-04 00:28:49 UTC (rev 209309)
@@ -516,7 +516,7 @@
TEST(WTF, StringImplCreateNullSymbol)
{
- auto reference = StringImpl::createNullSymbol();
+ auto reference = SymbolImpl::createNullSymbol();
ASSERT_TRUE(reference->isSymbol());
ASSERT_TRUE(reference->isNullSymbol());
ASSERT_FALSE(reference->isAtomic());
@@ -527,7 +527,7 @@
TEST(WTF, StringImplCreateSymbol)
{
auto original = stringFromUTF8("original");
- auto reference = StringImpl::createSymbol(original);
+ auto reference = SymbolImpl::create(original);
ASSERT_TRUE(reference->isSymbol());
ASSERT_FALSE(reference->isNullSymbol());
ASSERT_FALSE(reference->isAtomic());
@@ -537,12 +537,11 @@
ASSERT_TRUE(equal(reference.ptr(), "original"));
auto empty = stringFromUTF8("");
- auto emptyReference = StringImpl::createSymbol(empty);
+ auto emptyReference = SymbolImpl::create(empty);
ASSERT_TRUE(emptyReference->isSymbol());
ASSERT_FALSE(emptyReference->isNullSymbol());
ASSERT_FALSE(emptyReference->isAtomic());
ASSERT_FALSE(empty->isSymbol());
- ASSERT_FALSE(empty->isNullSymbol());
ASSERT_TRUE(empty->isAtomic());
ASSERT_EQ(empty->length(), emptyReference->length());
ASSERT_TRUE(equal(emptyReference.ptr(), ""));
@@ -551,7 +550,7 @@
TEST(WTF, StringImplSymbolToAtomicString)
{
auto original = stringFromUTF8("original");
- auto reference = StringImpl::createSymbol(original);
+ auto reference = SymbolImpl::create(original);
ASSERT_TRUE(reference->isSymbol());
ASSERT_FALSE(reference->isAtomic());
@@ -564,7 +563,7 @@
TEST(WTF, StringImplNullSymbolToAtomicString)
{
- auto reference = StringImpl::createNullSymbol();
+ auto reference = SymbolImpl::createNullSymbol();
ASSERT_TRUE(reference->isSymbol());
ASSERT_FALSE(reference->isAtomic());
@@ -587,8 +586,6 @@
TEST(WTF, StringImplEmpty)
{
ASSERT_FALSE(StringImpl::empty()->length());
- ASSERT_FALSE(StringImpl::null()->length());
- ASSERT_NE(StringImpl::null(), StringImpl::empty());
}
} // namespace TestWebKitAPI
