Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (289514 => 289515)
--- trunk/Source/_javascript_Core/ChangeLog 2022-02-10 01:27:42 UTC (rev 289514)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-02-10 01:50:04 UTC (rev 289515)
@@ -1,3 +1,57 @@
+2022-02-09 Yusuke Suzuki <[email protected]>
+
+ Use local variable pointer for concurrently load value
+ https://bugs.webkit.org/show_bug.cgi?id=236387
+
+ Reviewed by Saam Barati.
+
+ Consistently using local pointer to load member fields in
+
+ 1. WriteBarrierStructureID
+ 2. JSString's fiber
+ 3. Weak
+ 4. WriteBarrier<SomeKindOfCell>
+
+ to encourage compilers not to load the field twice.
+
+ * heap/Weak.cpp:
+ (JSC::weakClearSlowCase):
+ * heap/Weak.h:
+ (JSC::Weak::isHashTableEmptyValue const):
+ (JSC::Weak::unsafeImpl const):
+ (JSC::Weak::clear):
+ (JSC::Weak::impl const):
+ * heap/WeakInlines.h:
+ (JSC::Weak<T>::isHashTableDeletedValue const):
+ (JSC:: const):
+ (JSC::Weak<T>::operator const):
+ (JSC::Weak<T>::get const):
+ (JSC::Weak<T>::leakImpl):
+ * runtime/JSString.cpp:
+ (JSC::JSString::dumpToStream):
+ (JSC::JSString::estimatedSize):
+ (JSC::JSString::visitChildrenImpl):
+ * runtime/JSString.h:
+ (JSC::JSString::fiberConcurrently const):
+ (JSC::JSString::is8Bit const):
+ (JSC::JSString::length const):
+ (JSC::JSString::tryGetValueImpl const):
+ (JSC::JSString::isSubstring const):
+ * runtime/WriteBarrier.h:
+ (JSC::WriteBarrierBase::get const):
+ (JSC::WriteBarrierBase::operator* const):
+ (JSC::WriteBarrierBase::operator-> const):
+ (JSC::WriteBarrierBase::operator bool const):
+ (JSC::WriteBarrierBase::operator! const):
+ (JSC::WriteBarrierBase::unvalidatedGet const):
+ (JSC::WriteBarrierBase::cell const):
+ (JSC::WriteBarrierStructureID::get const):
+ (JSC::WriteBarrierStructureID::operator* const):
+ (JSC::WriteBarrierStructureID::operator-> const):
+ (JSC::WriteBarrierStructureID::operator bool const):
+ (JSC::WriteBarrierStructureID::operator! const):
+ (JSC::WriteBarrierStructureID::unvalidatedGet const):
+
2022-02-09 Lauro Moura <[email protected]>
Non-unified build fixes after r289247
Modified: trunk/Source/_javascript_Core/heap/Weak.cpp (289514 => 289515)
--- trunk/Source/_javascript_Core/heap/Weak.cpp 2022-02-10 01:27:42 UTC (rev 289514)
+++ trunk/Source/_javascript_Core/heap/Weak.cpp 2022-02-10 01:50:04 UTC (rev 289515)
@@ -30,12 +30,10 @@
namespace JSC {
-void weakClearSlowCase(WeakImpl*& impl)
+void weakClearSlowCase(WeakImpl* impl)
{
ASSERT(impl);
-
WeakSet::deallocate(impl);
- impl = nullptr;
}
} // namespace JSC
Modified: trunk/Source/_javascript_Core/heap/Weak.h (289514 => 289515)
--- trunk/Source/_javascript_Core/heap/Weak.h 2022-02-10 01:27:42 UTC (rev 289514)
+++ trunk/Source/_javascript_Core/heap/Weak.h 2022-02-10 01:50:04 UTC (rev 289515)
@@ -27,6 +27,7 @@
#include "JSExportMacros.h"
#include <cstddef>
+#include <wtf/Atomics.h>
#include <wtf/HashTraits.h>
#include <wtf/Noncopyable.h>
#include <wtf/VectorTraits.h>
@@ -37,7 +38,7 @@
class WeakHandleOwner;
// This is a free function rather than a Weak<T> member function so we can put it in Weak.cpp.
-JS_EXPORT_PRIVATE void weakClearSlowCase(WeakImpl*&);
+JS_EXPORT_PRIVATE void weakClearSlowCase(WeakImpl*);
template<typename T> class Weak {
WTF_MAKE_NONCOPYABLE(Weak);
@@ -53,7 +54,7 @@
bool isHashTableDeletedValue() const;
Weak(WTF::HashTableDeletedValueType);
- constexpr bool isHashTableEmptyValue() const { return !m_impl; }
+ constexpr bool isHashTableEmptyValue() const { return !impl(); }
Weak(Weak&&);
@@ -76,16 +77,19 @@
inline explicit operator bool() const;
inline WeakImpl* leakImpl() WARN_UNUSED_RETURN;
- WeakImpl* unsafeImpl() const { return m_impl; }
+ WeakImpl* unsafeImpl() const { return impl(); }
void clear()
{
- if (!m_impl)
+ auto* pointer = impl();
+ if (!pointer)
return;
- weakClearSlowCase(m_impl);
+ weakClearSlowCase(pointer);
+ m_impl = nullptr;
}
private:
static inline WeakImpl* hashTableDeletedValue();
+ WeakImpl* impl() const { return m_impl; }
WeakImpl* m_impl { nullptr };
};
Modified: trunk/Source/_javascript_Core/heap/WeakInlines.h (289514 => 289515)
--- trunk/Source/_javascript_Core/heap/WeakInlines.h 2022-02-10 01:27:42 UTC (rev 289514)
+++ trunk/Source/_javascript_Core/heap/WeakInlines.h 2022-02-10 01:50:04 UTC (rev 289515)
@@ -38,7 +38,7 @@
template<typename T> inline bool Weak<T>::isHashTableDeletedValue() const
{
- return m_impl == hashTableDeletedValue();
+ return impl() == hashTableDeletedValue();
}
template<typename T> inline Weak<T>::Weak(WTF::HashTableDeletedValueType)
@@ -70,24 +70,27 @@
template<typename T> inline T* Weak<T>::operator->() const
{
- ASSERT(m_impl && m_impl->state() == WeakImpl::Live);
+ auto* pointer = impl();
+ ASSERT(pointer && pointer->state() == WeakImpl::Live);
// We can't use jsCast here since we could be called in a finalizer.
- return static_cast<T*>(m_impl->jsValue().asCell());
+ return static_cast<T*>(pointer->jsValue().asCell());
}
template<typename T> inline T& Weak<T>::operator*() const
{
- ASSERT(m_impl && m_impl->state() == WeakImpl::Live);
+ auto* pointer = impl();
+ ASSERT(pointer && pointer->state() == WeakImpl::Live);
// We can't use jsCast here since we could be called in a finalizer.
- return *static_cast<T*>(m_impl->jsValue().asCell());
+ return *static_cast<T*>(pointer->jsValue().asCell());
}
template<typename T> inline T* Weak<T>::get() const
{
- if (!m_impl || m_impl->state() != WeakImpl::Live)
+ auto* pointer = impl();
+ if (!pointer || pointer->state() != WeakImpl::Live)
return nullptr;
// We can't use jsCast here since we could be called in a finalizer.
- return static_cast<T*>(m_impl->jsValue().asCell());
+ return static_cast<T*>(pointer->jsValue().asCell());
}
template<typename T> inline bool Weak<T>::was(T* other) const
@@ -97,7 +100,8 @@
template<typename T> inline bool Weak<T>::operator!() const
{
- return !m_impl || !m_impl->jsValue() || m_impl->state() != WeakImpl::Live;
+ auto* pointer = impl();
+ return !pointer || !pointer->jsValue() || pointer->state() != WeakImpl::Live;
}
template<typename T> inline Weak<T>::operator bool() const
@@ -107,9 +111,9 @@
template<typename T> inline WeakImpl* Weak<T>::leakImpl()
{
- WeakImpl* impl = m_impl;
+ auto* pointer = impl();
m_impl = nullptr;
- return impl;
+ return pointer;
}
template<typename T> inline WeakImpl* Weak<T>::hashTableDeletedValue()
Modified: trunk/Source/_javascript_Core/runtime/JSString.cpp (289514 => 289515)
--- trunk/Source/_javascript_Core/runtime/JSString.cpp 2022-02-10 01:27:42 UTC (rev 289514)
+++ trunk/Source/_javascript_Core/runtime/JSString.cpp 2022-02-10 01:50:04 UTC (rev 289515)
@@ -68,7 +68,7 @@
VM& vm = cell->vm();
const JSString* thisObject = jsCast<const JSString*>(cell);
out.printf("<%p, %s, [%u], ", thisObject, thisObject->className(vm), thisObject->length());
- uintptr_t pointer = thisObject->m_fiber;
+ uintptr_t pointer = thisObject->fiberConcurrently();
if (pointer & isRopeInPointer) {
if (pointer & JSRopeString::isSubstringInPointer)
out.printf("[substring]");
@@ -103,7 +103,7 @@
size_t JSString::estimatedSize(JSCell* cell, VM& vm)
{
JSString* thisObject = asString(cell);
- uintptr_t pointer = thisObject->m_fiber;
+ uintptr_t pointer = thisObject->fiberConcurrently();
if (pointer & isRopeInPointer)
return Base::estimatedSize(cell, vm);
return Base::estimatedSize(cell, vm) + bitwise_cast<StringImpl*>(pointer)->costDuringGC();
@@ -116,7 +116,7 @@
ASSERT_GC_OBJECT_INHERITS(thisObject, info());
Base::visitChildren(thisObject, visitor);
- uintptr_t pointer = thisObject->m_fiber;
+ uintptr_t pointer = thisObject->fiberConcurrently();
if (pointer & isRopeInPointer) {
if (pointer & JSRopeString::isSubstringInPointer) {
visitor.appendUnbarriered(static_cast<JSRopeString*>(thisObject)->fiber1());
Modified: trunk/Source/_javascript_Core/runtime/JSString.h (289514 => 289515)
--- trunk/Source/_javascript_Core/runtime/JSString.h 2022-02-10 01:27:42 UTC (rev 289514)
+++ trunk/Source/_javascript_Core/runtime/JSString.h 2022-02-10 01:50:04 UTC (rev 289515)
@@ -237,6 +237,8 @@
JS_EXPORT_PRIVATE bool equalSlowCase(JSGlobalObject*, JSString* other) const;
bool isSubstring() const;
+ uintptr_t fiberConcurrently() const { return m_fiber; }
+
mutable uintptr_t m_fiber;
private:
@@ -695,7 +697,7 @@
// is in the middle of converting JSRopeString to JSString.
ALWAYS_INLINE bool JSString::is8Bit() const
{
- uintptr_t pointer = m_fiber;
+ uintptr_t pointer = fiberConcurrently();
if (pointer & isRopeInPointer) {
// Do not load m_fiber twice. We should use the information in pointer.
// Otherwise, JSRopeString may be converted to JSString between the first and second accesses.
@@ -709,7 +711,7 @@
// when we resolve a JSRopeString.
ALWAYS_INLINE unsigned JSString::length() const
{
- uintptr_t pointer = m_fiber;
+ uintptr_t pointer = fiberConcurrently();
if (pointer & isRopeInPointer)
return jsCast<const JSRopeString*>(this)->length();
return bitwise_cast<StringImpl*>(pointer)->length();
@@ -723,7 +725,7 @@
inline const StringImpl* JSString::tryGetValueImpl() const
{
- uintptr_t pointer = m_fiber;
+ uintptr_t pointer = fiberConcurrently();
if (pointer & isRopeInPointer)
return nullptr;
return bitwise_cast<StringImpl*>(pointer);
@@ -1073,7 +1075,7 @@
inline bool JSString::isSubstring() const
{
- return m_fiber & JSRopeString::isSubstringInPointer;
+ return fiberConcurrently() & JSRopeString::isSubstringInPointer;
}
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/WriteBarrier.h (289514 => 289515)
--- trunk/Source/_javascript_Core/runtime/WriteBarrier.h 2022-02-10 01:27:42 UTC (rev 289514)
+++ trunk/Source/_javascript_Core/runtime/WriteBarrier.h 2022-02-10 01:50:04 UTC (rev 289515)
@@ -98,7 +98,7 @@
T* get() const
{
// Copy m_cell to a local to avoid multiple-read issues. (See <http://webkit.org/b/110854>)
- StorageType cell = m_cell;
+ StorageType cell = this->cell();
if (cell)
validateCell(reinterpret_cast<JSCell*>(static_cast<void*>(Traits::unwrap(cell))));
return Traits::unwrap(cell);
@@ -106,7 +106,7 @@
T* operator*() const
{
- StorageType cell = m_cell;
+ StorageType cell = this->cell();
ASSERT(cell);
auto unwrapped = Traits::unwrap(cell);
validateCell<T>(unwrapped);
@@ -115,7 +115,7 @@
T* operator->() const
{
- StorageType cell = m_cell;
+ StorageType cell = this->cell();
ASSERT(cell);
auto unwrapped = Traits::unwrap(cell);
validateCell(unwrapped);
@@ -135,9 +135,9 @@
return SlotHelper<T, Traits>::reinterpret(&m_cell);
}
- explicit operator bool() const { return !!m_cell; }
+ explicit operator bool() const { return !!cell(); }
- bool operator!() const { return !m_cell; }
+ bool operator!() const { return !cell(); }
void setWithoutWriteBarrier(T* value)
{
@@ -147,9 +147,11 @@
Traits::exchange(this->m_cell, value);
}
- T* unvalidatedGet() const { return Traits::unwrap(m_cell); }
+ T* unvalidatedGet() const { return Traits::unwrap(cell()); }
private:
+ StorageType cell() const { return m_cell; }
+
StorageType m_cell;
};
@@ -281,7 +283,7 @@
Structure* get() const
{
// Copy m_structureID to a local to avoid multiple-read issues. (See <http://webkit.org/b/110854>)
- StructureID structureID = m_structureID;
+ StructureID structureID = value();
if (structureID) {
Structure* structure = structureID.decode();
validateCell(reinterpret_cast<JSCell*>(structure));
@@ -292,7 +294,7 @@
Structure* operator*() const
{
- StructureID structureID = m_structureID;
+ StructureID structureID = value();
ASSERT(structureID);
Structure* structure = structureID.decode();
validateCell(reinterpret_cast<JSCell*>(structure));
@@ -301,7 +303,7 @@
Structure* operator->() const
{
- StructureID structureID = m_structureID;
+ StructureID structureID = value();
ASSERT(structureID);
Structure* structure = structureID.decode();
validateCell(reinterpret_cast<JSCell*>(structure));
@@ -315,12 +317,12 @@
explicit operator bool() const
{
- return !!m_structureID;
+ return !!value();
}
bool operator!() const
{
- return !m_structureID;
+ return !value();
}
void setWithoutWriteBarrier(Structure* value)
@@ -337,7 +339,7 @@
Structure* unvalidatedGet() const
{
- StructureID structureID = m_structureID;
+ StructureID structureID = value();
if (structureID)
return structureID.decode();
return nullptr;
