- Revision
- 255112
- Author
- [email protected]
- Date
- 2020-01-24 17:35:58 -0800 (Fri, 24 Jan 2020)
Log Message
IntlObject's cached strings should be immortal and safe for concurrent access.
https://bugs.webkit.org/show_bug.cgi?id=206779
<rdar://problem/58831763>
Reviewed by Yusuke Suzuki.
JSTests:
* stress/numberingSystemsForLocale-cached-strings-should-be-immortal-and-safe-for-concurrent-access.js: Added.
Source/_javascript_Core:
In IntlObject's numberingSystemsForLocale(), we have a never destroyed
cachedNumberingSystems which is a singleton vector of Strings which are shared
multiple VMs. Hence, the strings in this vector should be a StaticStringImpl
so that it will be immortal, and can be access concurrently from multiple VMs
on different threads without any ref/deref'ing race issues.
* runtime/IntlObject.cpp:
(JSC::numberingSystemsForLocale):
Source/WTF:
Add a factory for creating a dynamically allocated StaticStringImpl.
Note: StaticStringImpl is guaranteed to have the same shape as StringImpl.
The only difference is that s_refCountFlagIsStaticString is set on the refCount
for StaticStringImpl. Since the client will use the StaticStringImpl as a
StringImpl, we implement the factory by using StringImpl::createInternal() for
simplicity, and set the s_refCountFlagIsStaticString flag thereafter.
* wtf/text/StringImpl.cpp:
(WTF::StringImpl::createStaticStringImpl):
* wtf/text/StringImpl.h:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (255111 => 255112)
--- trunk/JSTests/ChangeLog 2020-01-25 01:28:03 UTC (rev 255111)
+++ trunk/JSTests/ChangeLog 2020-01-25 01:35:58 UTC (rev 255112)
@@ -1,3 +1,13 @@
+2020-01-24 Mark Lam <[email protected]>
+
+ IntlObject's cached strings should be immortal and safe for concurrent access.
+ https://bugs.webkit.org/show_bug.cgi?id=206779
+ <rdar://problem/58831763>
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/numberingSystemsForLocale-cached-strings-should-be-immortal-and-safe-for-concurrent-access.js: Added.
+
2020-01-24 Yusuke Suzuki <[email protected]>
REGRESSION (r254964-r254970?): Catalina Debug JSC bot timing out while running tests
Added: trunk/JSTests/stress/numberingSystemsForLocale-cached-strings-should-be-immortal-and-safe-for-concurrent-access.js (0 => 255112)
--- trunk/JSTests/stress/numberingSystemsForLocale-cached-strings-should-be-immortal-and-safe-for-concurrent-access.js (rev 0)
+++ trunk/JSTests/stress/numberingSystemsForLocale-cached-strings-should-be-immortal-and-safe-for-concurrent-access.js 2020-01-25 01:35:58 UTC (rev 255112)
@@ -0,0 +1,10 @@
+//@ runDefault
+
+let theCode = `
+for (let i=0; i<10000; i++) {
+ 0 .toLocaleString();
+}
+`;
+
+for (let i = 0; i < 100; i++)
+ $.agent.start(theCode);
Modified: trunk/Source/_javascript_Core/ChangeLog (255111 => 255112)
--- trunk/Source/_javascript_Core/ChangeLog 2020-01-25 01:28:03 UTC (rev 255111)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-01-25 01:35:58 UTC (rev 255112)
@@ -1,3 +1,20 @@
+2020-01-24 Mark Lam <[email protected]>
+
+ IntlObject's cached strings should be immortal and safe for concurrent access.
+ https://bugs.webkit.org/show_bug.cgi?id=206779
+ <rdar://problem/58831763>
+
+ Reviewed by Yusuke Suzuki.
+
+ In IntlObject's numberingSystemsForLocale(), we have a never destroyed
+ cachedNumberingSystems which is a singleton vector of Strings which are shared
+ multiple VMs. Hence, the strings in this vector should be a StaticStringImpl
+ so that it will be immortal, and can be access concurrently from multiple VMs
+ on different threads without any ref/deref'ing race issues.
+
+ * runtime/IntlObject.cpp:
+ (JSC::numberingSystemsForLocale):
+
2020-01-24 Caio Lima <[email protected]>
[ARMv7][JIT] Implement checkpoint support
Modified: trunk/Source/_javascript_Core/runtime/IntlObject.cpp (255111 => 255112)
--- trunk/Source/_javascript_Core/runtime/IntlObject.cpp 2020-01-25 01:28:03 UTC (rev 255111)
+++ trunk/Source/_javascript_Core/runtime/IntlObject.cpp 2020-01-25 01:35:58 UTC (rev 255112)
@@ -1,7 +1,7 @@
/*
* Copyright (C) 2015 Andy VanWagoner ([email protected])
* Copyright (C) 2015 Sukolsak Sakshuwong ([email protected])
- * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -891,28 +891,26 @@
static NeverDestroyed<Vector<String>> cachedNumberingSystems;
Vector<String>& availableNumberingSystems = cachedNumberingSystems.get();
- if (UNLIKELY(availableNumberingSystems.isEmpty())) {
- static Lock cachedNumberingSystemsMutex;
- std::lock_guard<Lock> lock(cachedNumberingSystemsMutex);
- if (availableNumberingSystems.isEmpty()) {
- UErrorCode status = U_ZERO_ERROR;
- UEnumeration* numberingSystemNames = unumsys_openAvailableNames(&status);
+ static std::once_flag initializeOnce;
+ std::call_once(initializeOnce, [&] {
+ ASSERT(availableNumberingSystems.isEmpty());
+ UErrorCode status = U_ZERO_ERROR;
+ UEnumeration* numberingSystemNames = unumsys_openAvailableNames(&status);
+ ASSERT(U_SUCCESS(status));
+
+ int32_t resultLength;
+ // Numbering system names are always ASCII, so use char[].
+ while (const char* result = uenum_next(numberingSystemNames, &resultLength, &status)) {
ASSERT(U_SUCCESS(status));
-
- int32_t resultLength;
- // Numbering system names are always ASCII, so use char[].
- while (const char* result = uenum_next(numberingSystemNames, &resultLength, &status)) {
- ASSERT(U_SUCCESS(status));
- auto numsys = unumsys_openByName(result, &status);
- ASSERT(U_SUCCESS(status));
- // Only support algorithmic if it is the default fot the locale, handled below.
- if (!unumsys_isAlgorithmic(numsys))
- availableNumberingSystems.append(String(result, resultLength));
- unumsys_close(numsys);
- }
- uenum_close(numberingSystemNames);
+ auto numsys = unumsys_openByName(result, &status);
+ ASSERT(U_SUCCESS(status));
+ // Only support algorithmic if it is the default fot the locale, handled below.
+ if (!unumsys_isAlgorithmic(numsys))
+ availableNumberingSystems.append(String(StringImpl::createStaticStringImpl(result, resultLength)));
+ unumsys_close(numsys);
}
- }
+ uenum_close(numberingSystemNames);
+ });
UErrorCode status = U_ZERO_ERROR;
UNumberingSystem* defaultSystem = unumsys_open(locale.utf8().data(), &status);
Modified: trunk/Source/WTF/ChangeLog (255111 => 255112)
--- trunk/Source/WTF/ChangeLog 2020-01-25 01:28:03 UTC (rev 255111)
+++ trunk/Source/WTF/ChangeLog 2020-01-25 01:35:58 UTC (rev 255112)
@@ -1,3 +1,23 @@
+2020-01-24 Mark Lam <[email protected]>
+
+ IntlObject's cached strings should be immortal and safe for concurrent access.
+ https://bugs.webkit.org/show_bug.cgi?id=206779
+ <rdar://problem/58831763>
+
+ Reviewed by Yusuke Suzuki.
+
+ Add a factory for creating a dynamically allocated StaticStringImpl.
+
+ Note: StaticStringImpl is guaranteed to have the same shape as StringImpl.
+ The only difference is that s_refCountFlagIsStaticString is set on the refCount
+ for StaticStringImpl. Since the client will use the StaticStringImpl as a
+ StringImpl, we implement the factory by using StringImpl::createInternal() for
+ simplicity, and set the s_refCountFlagIsStaticString flag thereafter.
+
+ * wtf/text/StringImpl.cpp:
+ (WTF::StringImpl::createStaticStringImpl):
+ * wtf/text/StringImpl.h:
+
2020-01-24 Keith Rollin <[email protected]>
Fix internal Apple builds after r254411
Modified: trunk/Source/WTF/wtf/text/StringImpl.cpp (255111 => 255112)
--- trunk/Source/WTF/wtf/text/StringImpl.cpp 2020-01-25 01:28:03 UTC (rev 255111)
+++ trunk/Source/WTF/wtf/text/StringImpl.cpp 2020-01-25 01:35:58 UTC (rev 255112)
@@ -2,7 +2,7 @@
* Copyright (C) 1999 Lars Knoll ([email protected])
* (C) 1999 Antti Koivisto ([email protected])
* (C) 2001 Dirk Mueller ( [email protected] )
- * Copyright (C) 2003-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2003-2020 Apple Inc. All rights reserved.
* Copyright (C) 2006 Andrew Wellington ([email protected])
*
* This library is free software; you can redistribute it and/or
@@ -281,6 +281,14 @@
return createInternal(characters, length);
}
+Ref<StringImpl> StringImpl::createStaticStringImpl(const char* characters, unsigned length)
+{
+ ASSERT(charactersAreAllASCII<LChar>(reinterpret_cast<const LChar*>(characters), length));
+ Ref<StringImpl> result = createInternal(reinterpret_cast<const LChar*>(characters), length);
+ result->m_refCount |= s_refCountFlagIsStaticString;
+ return result;
+}
+
Ref<StringImpl> StringImpl::create8BitIfPossible(const UChar* characters, unsigned length)
{
if (!characters || !length)
Modified: trunk/Source/WTF/wtf/text/StringImpl.h (255111 => 255112)
--- trunk/Source/WTF/wtf/text/StringImpl.h 2020-01-25 01:28:03 UTC (rev 255111)
+++ trunk/Source/WTF/wtf/text/StringImpl.h 2020-01-25 01:35:58 UTC (rev 255112)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 1999 Lars Knoll ([email protected])
- * Copyright (C) 2005-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2020 Apple Inc. All rights reserved.
* Copyright (C) 2009 Google Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
@@ -255,6 +255,8 @@
WTF_EXPORT_PRIVATE static Ref<StringImpl> createUninitialized(unsigned length, UChar*&);
template<typename CharacterType> static RefPtr<StringImpl> tryCreateUninitialized(unsigned length, CharacterType*&);
+ WTF_EXPORT_PRIVATE static Ref<StringImpl> createStaticStringImpl(const char*, unsigned length);
+
// 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.
