- Revision
- 241954
- Author
- [email protected]
- Date
- 2019-02-22 11:04:40 -0800 (Fri, 22 Feb 2019)
Log Message
[JSC] SmallStringsStorage is unnecessary
https://bugs.webkit.org/show_bug.cgi?id=194939
Reviewed by Mark Lam.
SmallStrings hold common small JSStrings. Their underlying StringImpl is also held by SmallStringsStorage.
But it is duplicate since we can get StringImpl from small JSStrings. This patch removes SmallStringsStorage,
and get StringImpls from JSStrings if necessary.
We also add m_canAccessHeap flag to SmallStrings. At the time of VM destruction, JSStrings are destroyed when
VM's Heap is finalized. We must not touch JSStrings before VM's heap (and JSStrings in SmallStrings) is initialized,
and after VM's Heap is destroyed. We add this m_canAccessHeap flag to allow users to get StringImpl during the
this sensitive period. If m_canAccessHeap is false, we get StringImpl from AtomicStringImpl::add.
* runtime/SmallStrings.cpp:
(JSC::SmallStrings::initializeCommonStrings):
(JSC::SmallStrings::singleCharacterStringRep):
(JSC::SmallStringsStorage::rep): Deleted.
(JSC::SmallStringsStorage::SmallStringsStorage): Deleted.
(JSC::SmallStrings::createSingleCharacterString): Deleted.
* runtime/SmallStrings.h:
(JSC::SmallStrings::setCanAccessHeap):
* runtime/VM.cpp:
(JSC::VM::VM):
(JSC::VM::~VM):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (241953 => 241954)
--- trunk/Source/_javascript_Core/ChangeLog 2019-02-22 18:46:29 UTC (rev 241953)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-02-22 19:04:40 UTC (rev 241954)
@@ -1,3 +1,31 @@
+2019-02-22 Yusuke Suzuki <[email protected]>
+
+ [JSC] SmallStringsStorage is unnecessary
+ https://bugs.webkit.org/show_bug.cgi?id=194939
+
+ Reviewed by Mark Lam.
+
+ SmallStrings hold common small JSStrings. Their underlying StringImpl is also held by SmallStringsStorage.
+ But it is duplicate since we can get StringImpl from small JSStrings. This patch removes SmallStringsStorage,
+ and get StringImpls from JSStrings if necessary.
+
+ We also add m_canAccessHeap flag to SmallStrings. At the time of VM destruction, JSStrings are destroyed when
+ VM's Heap is finalized. We must not touch JSStrings before VM's heap (and JSStrings in SmallStrings) is initialized,
+ and after VM's Heap is destroyed. We add this m_canAccessHeap flag to allow users to get StringImpl during the
+ this sensitive period. If m_canAccessHeap is false, we get StringImpl from AtomicStringImpl::add.
+
+ * runtime/SmallStrings.cpp:
+ (JSC::SmallStrings::initializeCommonStrings):
+ (JSC::SmallStrings::singleCharacterStringRep):
+ (JSC::SmallStringsStorage::rep): Deleted.
+ (JSC::SmallStringsStorage::SmallStringsStorage): Deleted.
+ (JSC::SmallStrings::createSingleCharacterString): Deleted.
+ * runtime/SmallStrings.h:
+ (JSC::SmallStrings::setCanAccessHeap):
+ * runtime/VM.cpp:
+ (JSC::VM::VM):
+ (JSC::VM::~VM):
+
2019-02-22 Tadeu Zagallo <[email protected]>
Cache CompactVariableMap::Handle instead of VariableEnvironment for UnlinkedFunctionExecutable
Modified: trunk/Source/_javascript_Core/runtime/SmallStrings.cpp (241953 => 241954)
--- trunk/Source/_javascript_Core/runtime/SmallStrings.cpp 2019-02-22 18:46:29 UTC (rev 241953)
+++ trunk/Source/_javascript_Core/runtime/SmallStrings.cpp 2019-02-22 19:04:40 UTC (rev 241954)
@@ -34,30 +34,6 @@
namespace JSC {
-class SmallStringsStorage {
- WTF_MAKE_NONCOPYABLE(SmallStringsStorage); WTF_MAKE_FAST_ALLOCATED;
-public:
- SmallStringsStorage();
-
- StringImpl& rep(unsigned char character)
- {
- return *m_reps[character].get();
- }
-
-private:
- static const unsigned singleCharacterStringCount = maxSingleCharacterString + 1;
-
- RefPtr<StringImpl> m_reps[singleCharacterStringCount];
-};
-
-SmallStringsStorage::SmallStringsStorage()
-{
- for (unsigned i = 0; i < singleCharacterStringCount; ++i) {
- const LChar string[] = { static_cast<LChar>(i) };
- m_reps[i] = AtomicStringImpl::add(StringImpl::create(string, 1).ptr());
- }
-}
-
SmallStrings::SmallStrings()
{
COMPILE_ASSERT(singleCharacterStringCount == sizeof(m_singleCharacterStrings) / sizeof(m_singleCharacterStrings[0]), IsNumCharactersConstInSyncWithClassUsage);
@@ -69,8 +45,14 @@
void SmallStrings::initializeCommonStrings(VM& vm)
{
createEmptyString(&vm);
- for (unsigned i = 0; i <= maxSingleCharacterString; ++i)
- createSingleCharacterString(&vm, i);
+
+ for (unsigned i = 0; i < singleCharacterStringCount; ++i) {
+ ASSERT(!m_singleCharacterStrings[i]);
+ const LChar string[] = { static_cast<LChar>(i) };
+ m_singleCharacterStrings[i] = JSString::createHasOtherOwner(vm, AtomicStringImpl::add(string, 1).releaseNonNull());
+ ASSERT(m_needsToBeVisited);
+ }
+
#define JSC_COMMON_STRINGS_ATTRIBUTE_INITIALIZE(name) initialize(&vm, m_##name, #name);
JSC_COMMON_STRINGS_EACH_NAME(JSC_COMMON_STRINGS_ATTRIBUTE_INITIALIZE)
#undef JSC_COMMON_STRINGS_ATTRIBUTE_INITIALIZE
@@ -77,6 +59,8 @@
initialize(&vm, m_objectStringStart, "[object ");
initialize(&vm, m_nullObjectString, "[object Null]");
initialize(&vm, m_undefinedObjectString, "[object Undefined]");
+
+ setInitialized(true);
}
void SmallStrings::visitStrongReferences(SlotVisitor& visitor)
@@ -104,22 +88,14 @@
ASSERT(m_needsToBeVisited);
}
-void SmallStrings::createSingleCharacterString(VM* vm, unsigned char character)
+Ref<StringImpl> SmallStrings::singleCharacterStringRep(unsigned char character)
{
- if (!m_storage)
- m_storage = std::make_unique<SmallStringsStorage>();
- ASSERT(!m_singleCharacterStrings[character]);
- m_singleCharacterStrings[character] = JSString::createHasOtherOwner(*vm, m_storage->rep(character));
- ASSERT(m_needsToBeVisited);
+ if (LIKELY(m_isInitialized))
+ return *const_cast<StringImpl*>(m_singleCharacterStrings[character]->tryGetValueImpl());
+ const LChar string[] = { static_cast<LChar>(character) };
+ return AtomicStringImpl::add(string, 1).releaseNonNull();
}
-StringImpl& SmallStrings::singleCharacterStringRep(unsigned char character)
-{
- if (!m_storage)
- m_storage = std::make_unique<SmallStringsStorage>();
- return m_storage->rep(character);
-}
-
void SmallStrings::initialize(VM* vm, JSString*& string, const char* value)
{
string = JSString::create(*vm, AtomicStringImpl::add(value).releaseNonNull());
Modified: trunk/Source/_javascript_Core/runtime/SmallStrings.h (241953 => 241954)
--- trunk/Source/_javascript_Core/runtime/SmallStrings.h 2019-02-22 18:46:29 UTC (rev 241953)
+++ trunk/Source/_javascript_Core/runtime/SmallStrings.h 2019-02-22 19:04:40 UTC (rev 241954)
@@ -51,7 +51,6 @@
class VM;
class JSString;
-class SmallStringsStorage;
class SlotVisitor;
static const unsigned maxSingleCharacterString = 0xFF;
@@ -72,8 +71,10 @@
return m_singleCharacterStrings[character];
}
- JS_EXPORT_PRIVATE WTF::StringImpl& singleCharacterStringRep(unsigned char character);
+ JS_EXPORT_PRIVATE Ref<StringImpl> singleCharacterStringRep(unsigned char character);
+ void setIsInitialized(bool isInitialized) { m_isInitialized = isInitialized; }
+
JSString** singleCharacterStrings() { return &m_singleCharacterStrings[0]; }
void initializeCommonStrings(VM&);
@@ -127,7 +128,6 @@
static const unsigned singleCharacterStringCount = maxSingleCharacterString + 1;
void createEmptyString(VM*);
- void createSingleCharacterString(VM*, unsigned char);
void initialize(VM*, JSString*&, const char* value);
@@ -139,8 +139,8 @@
JSString* m_nullObjectString { nullptr };
JSString* m_undefinedObjectString { nullptr };
JSString* m_singleCharacterStrings[singleCharacterStringCount] { nullptr };
- std::unique_ptr<SmallStringsStorage> m_storage;
bool m_needsToBeVisited { true };
+ bool m_isInitialized { false };
};
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (241953 => 241954)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2019-02-22 18:46:29 UTC (rev 241953)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2019-02-22 19:04:40 UTC (rev 241954)
@@ -340,11 +340,14 @@
// Need to be careful to keep everything consistent here
JSLockHolder lock(this);
AtomicStringTable* existingEntryAtomicStringTable = Thread::current().setCurrentAtomicStringTable(m_atomicStringTable);
- propertyNames = new CommonIdentifiers(this);
structureStructure.set(*this, Structure::createStructure(*this));
structureRareDataStructure.set(*this, StructureRareData::createStructure(*this, 0, jsNull()));
+ stringStructure.set(*this, JSString::createStructure(*this, 0, jsNull()));
+
+ smallStrings.initializeCommonStrings(*this);
+
+ propertyNames = new CommonIdentifiers(this);
terminatedExecutionErrorStructure.set(*this, TerminatedExecutionError::createStructure(*this, 0, jsNull()));
- stringStructure.set(*this, JSString::createStructure(*this, 0, jsNull()));
propertyNameEnumeratorStructure.set(*this, JSPropertyNameEnumerator::createStructure(*this, 0, jsNull()));
customGetterSetterStructure.set(*this, CustomGetterSetter::createStructure(*this, 0, jsNull()));
domAttributeGetterSetterStructure.set(*this, DOMAttributeGetterSetter::createStructure(*this, 0, jsNull()));
@@ -401,8 +404,6 @@
sentinelSetBucket.set(*this, JSSet::BucketType::createSentinel(*this));
sentinelMapBucket.set(*this, JSMap::BucketType::createSentinel(*this));
- smallStrings.initializeCommonStrings(*this);
-
Thread::current().setCurrentAtomicStringTable(existingEntryAtomicStringTable);
#if ENABLE(JIT)
@@ -539,6 +540,7 @@
ASSERT(currentThreadIsHoldingAPILock());
m_apiLock->willDestroyVM(this);
+ smallStrings.setInitialized(false);
heap.lastChanceToFinalize();
JSRunLoopTimer::Manager::shared().unregisterVM(*this);
