Diff
Modified: trunk/JSTests/ChangeLog (232442 => 232443)
--- trunk/JSTests/ChangeLog 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/JSTests/ChangeLog 2018-06-02 21:11:19 UTC (rev 232443)
@@ -1,5 +1,14 @@
2018-06-02 Yusuke Suzuki <[email protected]>
+ [JSC] Remove WeakReferenceHarvester
+ https://bugs.webkit.org/show_bug.cgi?id=186102
+
+ Reviewed by Filip Pizlo.
+
+ * microbenchmarks/create-many-weak-map.js: Added.
+
+2018-06-02 Yusuke Suzuki <[email protected]>
+
[JSC] Object.create should have intrinsic
https://bugs.webkit.org/show_bug.cgi?id=186200
Added: trunk/JSTests/microbenchmarks/create-many-weak-map.js (0 => 232443)
--- trunk/JSTests/microbenchmarks/create-many-weak-map.js (rev 0)
+++ trunk/JSTests/microbenchmarks/create-many-weak-map.js 2018-06-02 21:11:19 UTC (rev 232443)
@@ -0,0 +1,20 @@
+var root = [];
+
+function create()
+{
+ var weakMap = new WeakMap();
+ for (var i = 0; i < 1e2; ++i) {
+ var key = {};
+ var value = {};
+ weakMap.set(key, value);
+ if (i % 10 === 0)
+ root.push(key);
+ }
+}
+
+for (var i = 0; i < 1e2; ++i) {
+ for (var j = 0; j < 1e2; ++j) {
+ var map = create();
+ root.push(map);
+ }
+}
Modified: trunk/Source/_javascript_Core/API/JSAPIWrapperObject.h (232442 => 232443)
--- trunk/Source/_javascript_Core/API/JSAPIWrapperObject.h 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/API/JSAPIWrapperObject.h 2018-06-02 21:11:19 UTC (rev 232443)
@@ -29,7 +29,6 @@
#include "JSBase.h"
#include "JSCPoison.h"
#include "JSDestructibleObject.h"
-#include "WeakReferenceHarvester.h"
#include <wtf/Poisoned.h>
#if JSC_OBJC_API_ENABLED || defined(JSC_GLIB_API_ENABLED)
Modified: trunk/Source/_javascript_Core/CMakeLists.txt (232442 => 232443)
--- trunk/Source/_javascript_Core/CMakeLists.txt 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/CMakeLists.txt 2018-06-02 21:11:19 UTC (rev 232443)
@@ -567,7 +567,6 @@
heap/WeakHandleOwner.h
heap/WeakImpl.h
heap/WeakInlines.h
- heap/WeakReferenceHarvester.h
heap/WeakSet.h
heap/WeakSetInlines.h
Modified: trunk/Source/_javascript_Core/ChangeLog (232442 => 232443)
--- trunk/Source/_javascript_Core/ChangeLog 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-06-02 21:11:19 UTC (rev 232443)
@@ -1,5 +1,47 @@
2018-06-02 Yusuke Suzuki <[email protected]>
+ [JSC] Remove WeakReferenceHarvester
+ https://bugs.webkit.org/show_bug.cgi?id=186102
+
+ Reviewed by Filip Pizlo.
+
+ After several cleanups, now JSWeakMap becomes the last user of WeakReferenceHarvester.
+ Since JSWeakMap is already managed in IsoSubspace, we can iterate marked JSWeakMap
+ by using output constraints & Subspace iteration.
+
+ This patch removes WeakReferenceHarvester. Instead of managing this linked-list, our
+ output constraint set iterates marked JSWeakMap by using Subspace.
+
+ And we also add locking for JSWeakMap's rehash and output constraint visiting.
+
+ Attached microbenchmark does not show any regression.
+
+ * API/JSAPIWrapperObject.h:
+ * CMakeLists.txt:
+ * _javascript_Core.xcodeproj/project.pbxproj:
+ * heap/Heap.cpp:
+ (JSC::Heap::endMarking):
+ (JSC::Heap::addCoreConstraints):
+ * heap/Heap.h:
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::addWeakReferenceHarvester): Deleted.
+ * heap/SlotVisitor.h:
+ * heap/WeakReferenceHarvester.h: Removed.
+ * runtime/WeakMapImpl.cpp:
+ (JSC::WeakMapImpl<WeakMapBucket>::visitChildren):
+ (JSC::WeakMapImpl<WeakMapBucket<WeakMapBucketDataKey>>::visitOutputConstraints):
+ (JSC::WeakMapImpl<WeakMapBucket<WeakMapBucketDataKeyValue>>::visitOutputConstraints):
+ (JSC::WeakMapImpl<WeakMapBucket<WeakMapBucketDataKey>>::visitWeakReferences): Deleted.
+ (JSC::WeakMapImpl<WeakMapBucket<WeakMapBucketDataKeyValue>>::visitWeakReferences): Deleted.
+ * runtime/WeakMapImpl.h:
+ (JSC::WeakMapImpl::WeakMapImpl):
+ (JSC::WeakMapImpl::finishCreation):
+ (JSC::WeakMapImpl::rehash):
+ (JSC::WeakMapImpl::makeAndSetNewBuffer):
+ (JSC::WeakMapImpl::DeadKeyCleaner::target): Deleted.
+
+2018-06-02 Yusuke Suzuki <[email protected]>
+
[JSC] Object.create should have intrinsic
https://bugs.webkit.org/show_bug.cgi?id=186200
Modified: trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj (232442 => 232443)
--- trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2018-06-02 21:11:19 UTC (rev 232443)
@@ -150,7 +150,6 @@
0F235BE217178E1C00690C7F /* FTLThunks.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F235BCC17178E1C00690C7F /* FTLThunks.h */; settings = {ATTRIBUTES = (Private, ); }; };
0F235BEC17178E7300690C7F /* DFGOSRExitBase.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F235BE817178E7300690C7F /* DFGOSRExitBase.h */; };
0F235BEE17178E7300690C7F /* DFGOSRExitPreparation.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F235BEA17178E7300690C7F /* DFGOSRExitPreparation.h */; };
- 0F242DA713F3B1E8007ADD4C /* WeakReferenceHarvester.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F242DA513F3B1BB007ADD4C /* WeakReferenceHarvester.h */; settings = {ATTRIBUTES = (Private, ); }; };
0F24E54117EA9F5900ABB217 /* AssemblyHelpers.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F24E53C17EA9F5900ABB217 /* AssemblyHelpers.h */; settings = {ATTRIBUTES = (Private, ); }; };
0F24E54217EA9F5900ABB217 /* CCallHelpers.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F24E53D17EA9F5900ABB217 /* CCallHelpers.h */; settings = {ATTRIBUTES = (Private, ); }; };
0F24E54317EA9F5900ABB217 /* FPRInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F24E53E17EA9F5900ABB217 /* FPRInfo.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -2063,7 +2062,6 @@
0F235BE817178E7300690C7F /* DFGOSRExitBase.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGOSRExitBase.h; path = dfg/DFGOSRExitBase.h; sourceTree = "<group>"; };
0F235BE917178E7300690C7F /* DFGOSRExitPreparation.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = DFGOSRExitPreparation.cpp; path = dfg/DFGOSRExitPreparation.cpp; sourceTree = "<group>"; };
0F235BEA17178E7300690C7F /* DFGOSRExitPreparation.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGOSRExitPreparation.h; path = dfg/DFGOSRExitPreparation.h; sourceTree = "<group>"; };
- 0F242DA513F3B1BB007ADD4C /* WeakReferenceHarvester.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WeakReferenceHarvester.h; sourceTree = "<group>"; };
0F24E53B17EA9F5900ABB217 /* AssemblyHelpers.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = AssemblyHelpers.cpp; sourceTree = "<group>"; };
0F24E53C17EA9F5900ABB217 /* AssemblyHelpers.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AssemblyHelpers.h; sourceTree = "<group>"; };
0F24E53D17EA9F5900ABB217 /* CCallHelpers.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CCallHelpers.h; sourceTree = "<group>"; };
@@ -5821,7 +5819,6 @@
14F7256414EE265E00B1652B /* WeakHandleOwner.h */,
14E84F9D14EE1ACC00D6D5D4 /* WeakImpl.h */,
14BE7D3217135CF400D1807A /* WeakInlines.h */,
- 0F242DA513F3B1BB007ADD4C /* WeakReferenceHarvester.h */,
14E84F9B14EE1ACC00D6D5D4 /* WeakSet.cpp */,
14E84F9C14EE1ACC00D6D5D4 /* WeakSet.h */,
14150132154BB13F005D8C98 /* WeakSetInlines.h */,
@@ -9627,7 +9624,6 @@
E3A32BC71FC83147007D7E76 /* WeakMapImpl.h in Headers */,
E393ADD81FE702D00022D681 /* WeakMapImplInlines.h in Headers */,
A7CA3AE617DA41AE006538AF /* WeakMapPrototype.h in Headers */,
- 0F242DA713F3B1E8007ADD4C /* WeakReferenceHarvester.h in Headers */,
14E84FA114EE1ACC00D6D5D4 /* WeakSet.h in Headers */,
709FB86A1AE335C60039D069 /* WeakSetConstructor.h in Headers */,
14150133154BB13F005D8C98 /* WeakSetInlines.h in Headers */,
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (232442 => 232443)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2018-06-02 21:11:19 UTC (rev 232443)
@@ -782,7 +782,6 @@
});
assertMarkStacksEmpty();
- m_weakReferenceHarvesters.removeAll();
RELEASE_ASSERT(m_raceMarkStack->isEmpty());
@@ -2717,14 +2716,6 @@
ConstraintVolatility::GreyedByMarking);
m_constraintSet->add(
- "Wrh", "Weak Reference Harvesters",
- [this] (SlotVisitor& slotVisitor) {
- for (WeakReferenceHarvester* current = m_weakReferenceHarvesters.head(); current; current = current->next())
- current->visitWeakReferences(slotVisitor);
- },
- ConstraintVolatility::GreyedByMarking);
-
- m_constraintSet->add(
"O", "Output",
[] (SlotVisitor& slotVisitor) {
VM& vm = slotVisitor.vm();
@@ -2740,6 +2731,7 @@
};
add(vm.executableToCodeBlockEdgesWithConstraints);
+ add(vm.weakMapSpace);
},
ConstraintVolatility::GreyedByMarking,
ConstraintParallelism::Parallel);
Modified: trunk/Source/_javascript_Core/heap/Heap.h (232442 => 232443)
--- trunk/Source/_javascript_Core/heap/Heap.h 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/heap/Heap.h 2018-06-02 21:11:19 UTC (rev 232443)
@@ -40,7 +40,6 @@
#include "StructureIDTable.h"
#include "Synchronousness.h"
#include "WeakHandleOwner.h"
-#include "WeakReferenceHarvester.h"
#include <wtf/AutomaticThread.h>
#include <wtf/ConcurrentPtrHashSet.h>
#include <wtf/Deque.h>
@@ -674,8 +673,6 @@
static const size_t s_blockFragmentLength = 32;
- ListableHandler<WeakReferenceHarvester>::List m_weakReferenceHarvesters;
-
ParallelHelperClient m_helperClient;
RefPtr<SharedTask<void(SlotVisitor&)>> m_bonusVisitorTask;
Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.cpp (232442 => 232443)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2018-06-02 21:11:19 UTC (rev 232443)
@@ -753,11 +753,6 @@
drain(timeout);
}
-void SlotVisitor::addWeakReferenceHarvester(WeakReferenceHarvester* weakReferenceHarvester)
-{
- m_heap.m_weakReferenceHarvesters.addThreadSafe(weakReferenceHarvester);
-}
-
void SlotVisitor::didRace(const VisitRaceKey& race)
{
if (Options::verboseVisitRace())
Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.h (232442 => 232443)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.h 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.h 2018-06-02 21:11:19 UTC (rev 232443)
@@ -45,7 +45,6 @@
class MarkingConstraint;
class MarkingConstraintSolver;
template<typename T> class Weak;
-class WeakReferenceHarvester;
template<typename T, typename Traits> class WriteBarrierBase;
typedef uint32_t HeapVersion;
@@ -140,8 +139,6 @@
void reportExternalMemoryVisited(size_t);
#endif
- void addWeakReferenceHarvester(WeakReferenceHarvester*);
-
void dump(PrintStream&) const;
bool isBuildingHeapSnapshot() const { return !!m_heapSnapshotBuilder; }
Deleted: trunk/Source/_javascript_Core/heap/WeakReferenceHarvester.h (232442 => 232443)
--- trunk/Source/_javascript_Core/heap/WeakReferenceHarvester.h 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/heap/WeakReferenceHarvester.h 2018-06-02 21:11:19 UTC (rev 232443)
@@ -1,40 +0,0 @@
-/*
- * Copyright (C) 2011 Apple Inc. All rights reserved.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- *
- */
-
-#pragma once
-
-#include "ListableHandler.h"
-
-namespace JSC {
-
-class SlotVisitor;
-
-class WeakReferenceHarvester : public ListableHandler<WeakReferenceHarvester> {
-public:
- virtual void visitWeakReferences(SlotVisitor&) = 0;
-
-protected:
- WeakReferenceHarvester()
- {
- }
-
- virtual ~WeakReferenceHarvester() { }
-};
-
-} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/WeakMapImpl.cpp (232442 => 232443)
--- trunk/Source/_javascript_Core/runtime/WeakMapImpl.cpp 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/runtime/WeakMapImpl.cpp 2018-06-02 21:11:19 UTC (rev 232443)
@@ -45,9 +45,6 @@
WeakMapImpl* thisObject = jsCast<WeakMapImpl*>(cell);
ASSERT_GC_OBJECT_INHERITS(thisObject, info());
Base::visitChildren(thisObject, visitor);
-
- if (isWeakMap())
- visitor.addWeakReferenceHarvester(&thisObject->m_deadKeyCleaner);
visitor.reportExtraMemoryVisited(thisObject->m_capacity * sizeof(WeakMapBucket));
}
@@ -59,16 +56,18 @@
}
template <>
-void WeakMapImpl<WeakMapBucket<WeakMapBucketDataKey>>::visitWeakReferences(SlotVisitor&)
+void WeakMapImpl<WeakMapBucket<WeakMapBucketDataKey>>::visitOutputConstraints(JSCell*, SlotVisitor&)
{
// Only JSWeakMap needs to harvest value references
}
template <>
-void WeakMapImpl<WeakMapBucket<WeakMapBucketDataKeyValue>>::visitWeakReferences(SlotVisitor& visitor)
+void WeakMapImpl<WeakMapBucket<WeakMapBucketDataKeyValue>>::visitOutputConstraints(JSCell* cell, SlotVisitor& visitor)
{
- auto* buffer = this->buffer();
- for (uint32_t index = 0; index < m_capacity; ++index) {
+ auto* thisObject = jsCast<WeakMapImpl*>(cell);
+ auto locker = holdLock(thisObject->cellLock());
+ auto* buffer = thisObject->buffer();
+ for (uint32_t index = 0; index < thisObject->m_capacity; ++index) {
auto* bucket = buffer + index;
if (bucket->isEmpty() || bucket->isDeleted())
continue;
Modified: trunk/Source/_javascript_Core/runtime/WeakMapImpl.h (232442 => 232443)
--- trunk/Source/_javascript_Core/runtime/WeakMapImpl.h 2018-06-02 21:08:10 UTC (rev 232442)
+++ trunk/Source/_javascript_Core/runtime/WeakMapImpl.h 2018-06-02 21:11:19 UTC (rev 232443)
@@ -207,18 +207,19 @@
WeakMapImpl(VM& vm, Structure* structure)
: Base(vm, structure)
- , m_keyCount(0)
- , m_deleteCount(0)
- , m_capacity(4)
{
}
+ static constexpr uint32_t initialCapacity = 4;
+
void finishCreation(VM& vm)
{
ASSERT_WITH_MESSAGE(WeakMapBucket<WeakMapBucketDataKey>::offsetOfKey() == WeakMapBucket<WeakMapBucketDataKeyValue>::offsetOfKey(), "We assume this to be true in the DFG and FTL JIT.");
Base::finishCreation(vm);
- makeAndSetNewBuffer();
+
+ auto locker = holdLock(cellLock());
+ makeAndSetNewBuffer(locker, initialCapacity);
}
// WeakMap operations must not cause GC. We model operations in DFG based on this guarantee.
@@ -309,6 +310,7 @@
return &vm.weakSetSpace;
}
+ static void visitOutputConstraints(JSCell*, SlotVisitor&);
void finalizeUnconditionally(VM&);
private:
@@ -403,15 +405,20 @@
// function must not touch any GC related features. This is why we do not allocate WeakMapBuffer
// in auxiliary buffer.
+ // This rehash modifies m_buffer which is not GC-managed buffer. But m_buffer can be touched in
+ // visitOutputConstraints. Thus, we should guard it with cellLock.
+ auto locker = holdLock(cellLock());
+
uint32_t oldCapacity = m_capacity;
MallocPtr<WeakMapBufferType, JSValueMalloc> oldBuffer = WTFMove(m_buffer);
+ uint32_t capacity = m_capacity;
if (mode == RehashMode::RemoveBatching) {
ASSERT(shouldShrink());
- m_capacity = nextCapacityAfterBatchRemoval(m_capacity, m_keyCount);
+ capacity = nextCapacityAfterBatchRemoval(capacity, m_keyCount);
} else
- m_capacity = nextCapacity(m_capacity, m_keyCount);
- makeAndSetNewBuffer();
+ capacity = nextCapacity(capacity, m_keyCount);
+ makeAndSetNewBuffer(locker, capacity);
auto* buffer = this->buffer();
const uint32_t mask = m_capacity - 1;
@@ -449,11 +456,12 @@
}
}
- void makeAndSetNewBuffer()
+ void makeAndSetNewBuffer(const AbstractLocker&, uint32_t capacity)
{
- ASSERT(!(m_capacity & (m_capacity - 1)));
+ ASSERT(!(capacity & (capacity - 1)));
- m_buffer = WeakMapBufferType::create(m_capacity);
+ m_buffer = WeakMapBufferType::create(capacity);
+ m_capacity = capacity;
ASSERT(m_buffer);
assertBufferIsEmpty();
}
@@ -469,27 +477,10 @@
template<typename Appender>
void takeSnapshotInternal(unsigned limit, Appender);
- class DeadKeyCleaner : public WeakReferenceHarvester {
- public:
- WeakMapImpl* target()
- {
- return bitwise_cast<WeakMapImpl*>(bitwise_cast<char*>(this) - OBJECT_OFFSETOF(WeakMapImpl, m_deadKeyCleaner));
- }
-
- private:
- void visitWeakReferences(SlotVisitor& visitor) override
- {
- target()->visitWeakReferences(visitor);
- }
- };
-
- void visitWeakReferences(SlotVisitor&);
-
MallocPtr<WeakMapBufferType, JSValueMalloc> m_buffer;
- uint32_t m_keyCount;
- uint32_t m_deleteCount;
- uint32_t m_capacity;
- DeadKeyCleaner m_deadKeyCleaner;
+ uint32_t m_capacity { 0 };
+ uint32_t m_keyCount { 0 };
+ uint32_t m_deleteCount { 0 };
};
} // namespace JSC
