WebKit Bugzilla
Attachment 341842 Details for
Bug 186102
: [JSC] Remove WeakReferenceHarvester
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-186102-20180602204017.patch (text/plain), 16.22 KB, created by
Yusuke Suzuki
on 2018-06-02 04:40:18 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Yusuke Suzuki
Created:
2018-06-02 04:40:18 PDT
Size:
16.22 KB
patch
obsolete
>Subversion Revision: 232434 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index 92185c85dc0048570d539a674b2cc31dbe8e33ac..8e4de13565e1460c77da24ededc2d69f2d91be42 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,40 @@ >+2018-06-01 Yusuke Suzuki <utatane.tea@gmail.com> >+ >+ [JSC] Remove WeakReferenceHarvester >+ https://bugs.webkit.org/show_bug.cgi?id=186102 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ 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 IsoSubspace::forEachMarkedCell. >+ >+ This patch removes WeakReferenceHarvester. Instead of managing this linked-list, our >+ Weak Reference Harvester constraint set iterates marked JSWeakMap by using Subspace. >+ >+ Attached microbenchmark does not show any regression. >+ >+ * API/JSAPIWrapperObject.h: >+ * CMakeLists.txt: >+ * JavaScriptCore.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::DeadKeyCleaner::target): Deleted. >+ (): Deleted. >+ > 2018-06-01 Wenson Hsieh <wenson_hsieh@apple.com> > > Fix the watchOS build after r232385 >diff --git a/Source/JavaScriptCore/API/JSAPIWrapperObject.h b/Source/JavaScriptCore/API/JSAPIWrapperObject.h >index aae5d418a7a32915ca43ec9728f6e80c2a7e7bbe..305fca4ce0b8fe21d6b8384f9627b367905542d9 100644 >--- a/Source/JavaScriptCore/API/JSAPIWrapperObject.h >+++ b/Source/JavaScriptCore/API/JSAPIWrapperObject.h >@@ -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) >diff --git a/Source/JavaScriptCore/CMakeLists.txt b/Source/JavaScriptCore/CMakeLists.txt >index d87bb0309088c22a28fb4939660709305c0a7471..cdecabd1d6b25ba6ec73ab82a2043e0fc260e54a 100644 >--- a/Source/JavaScriptCore/CMakeLists.txt >+++ b/Source/JavaScriptCore/CMakeLists.txt >@@ -567,7 +567,6 @@ set(JavaScriptCore_PRIVATE_FRAMEWORK_HEADERS > heap/WeakHandleOwner.h > heap/WeakImpl.h > heap/WeakInlines.h >- heap/WeakReferenceHarvester.h > heap/WeakSet.h > heap/WeakSetInlines.h > >diff --git a/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj b/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj >index 13dafbbd1be5c45fd5eaa11bc4139bf8178b40fd..63e5a14f6a358e831dde020878adaab0c98722bd 100644 >--- a/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj >+++ b/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj >@@ -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 */, >diff --git a/Source/JavaScriptCore/heap/Heap.cpp b/Source/JavaScriptCore/heap/Heap.cpp >index 3b1aacfc3eaa6a6a75a521c5810d9dd94c5c41cb..9c66bbc053e73bd66f452157474fc03659ba9a8f 100644 >--- a/Source/JavaScriptCore/heap/Heap.cpp >+++ b/Source/JavaScriptCore/heap/Heap.cpp >@@ -782,7 +782,6 @@ void Heap::endMarking() > }); > > assertMarkStacksEmpty(); >- m_weakReferenceHarvesters.removeAll(); > > RELEASE_ASSERT(m_raceMarkStack->isEmpty()); > >@@ -2716,14 +2715,6 @@ void Heap::addCoreConstraints() > }, > 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) { >@@ -2740,6 +2731,7 @@ void Heap::addCoreConstraints() > }; > > add(vm.executableToCodeBlockEdgesWithConstraints); >+ add(vm.weakMapSpace); > }, > ConstraintVolatility::GreyedByMarking, > ConstraintParallelism::Parallel); >diff --git a/Source/JavaScriptCore/heap/Heap.h b/Source/JavaScriptCore/heap/Heap.h >index e7d99fb2b7817ffb03b692c49e3eac12df3381bb..3efd201871351bf5d1fafcabda112bf8d2746989 100644 >--- a/Source/JavaScriptCore/heap/Heap.h >+++ b/Source/JavaScriptCore/heap/Heap.h >@@ -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 @@ class Heap { > > static const size_t s_blockFragmentLength = 32; > >- ListableHandler<WeakReferenceHarvester>::List m_weakReferenceHarvesters; >- > ParallelHelperClient m_helperClient; > RefPtr<SharedTask<void(SlotVisitor&)>> m_bonusVisitorTask; > >diff --git a/Source/JavaScriptCore/heap/SlotVisitor.cpp b/Source/JavaScriptCore/heap/SlotVisitor.cpp >index 5b0924a846d2ca12344fc537a27fec20590549d4..b89686e545a8e70b289d1b421868e9a82dfd7060 100644 >--- a/Source/JavaScriptCore/heap/SlotVisitor.cpp >+++ b/Source/JavaScriptCore/heap/SlotVisitor.cpp >@@ -753,11 +753,6 @@ void SlotVisitor::donateAndDrain(MonotonicTime timeout) > drain(timeout); > } > >-void SlotVisitor::addWeakReferenceHarvester(WeakReferenceHarvester* weakReferenceHarvester) >-{ >- m_heap.m_weakReferenceHarvesters.addThreadSafe(weakReferenceHarvester); >-} >- > void SlotVisitor::didRace(const VisitRaceKey& race) > { > if (Options::verboseVisitRace()) >diff --git a/Source/JavaScriptCore/heap/SlotVisitor.h b/Source/JavaScriptCore/heap/SlotVisitor.h >index 29fbfacb56234209331bdbf606dfd15c276cc303..e8a27743a6c63b12ad99773abd260d839ad31731 100644 >--- a/Source/JavaScriptCore/heap/SlotVisitor.h >+++ b/Source/JavaScriptCore/heap/SlotVisitor.h >@@ -45,7 +45,6 @@ class MarkedBlock; > 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 @@ class SlotVisitor { > void reportExternalMemoryVisited(size_t); > #endif > >- void addWeakReferenceHarvester(WeakReferenceHarvester*); >- > void dump(PrintStream&) const; > > bool isBuildingHeapSnapshot() const { return !!m_heapSnapshotBuilder; } >diff --git a/Source/JavaScriptCore/heap/WeakReferenceHarvester.h b/Source/JavaScriptCore/heap/WeakReferenceHarvester.h >deleted file mode 100644 >index 4807e06791ea7a87a6c96e9209b65f00317a7e05..0000000000000000000000000000000000000000 >--- a/Source/JavaScriptCore/heap/WeakReferenceHarvester.h >+++ /dev/null >@@ -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 >diff --git a/Source/JavaScriptCore/runtime/WeakMapImpl.cpp b/Source/JavaScriptCore/runtime/WeakMapImpl.cpp >index fc5ebdcc4fa0269223d664478b712232e6c21ab4..c4a360febfcec0474a072bbb68841c7743cdd851 100644 >--- a/Source/JavaScriptCore/runtime/WeakMapImpl.cpp >+++ b/Source/JavaScriptCore/runtime/WeakMapImpl.cpp >@@ -45,9 +45,6 @@ void WeakMapImpl<WeakMapBucket>::visitChildren(JSCell* cell, SlotVisitor& visito > 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,17 @@ size_t WeakMapImpl<WeakMapBucket>::estimatedSize(JSCell* cell) > } > > 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* buffer = thisObject->buffer(); >+ for (uint32_t index = 0; index < thisObject->m_capacity; ++index) { > auto* bucket = buffer + index; > if (bucket->isEmpty() || bucket->isDeleted()) > continue; >diff --git a/Source/JavaScriptCore/runtime/WeakMapImpl.h b/Source/JavaScriptCore/runtime/WeakMapImpl.h >index 1bf6350b08cdd106c7a8b6aee19658e53e879663..e29b329f67c3ddbb6f3979851c0afa11bcfdb9b7 100644 >--- a/Source/JavaScriptCore/runtime/WeakMapImpl.h >+++ b/Source/JavaScriptCore/runtime/WeakMapImpl.h >@@ -309,6 +309,7 @@ class WeakMapImpl : public JSDestructibleObject { > return &vm.weakSetSpace; > } > >+ static void visitOutputConstraints(JSCell*, SlotVisitor&); > void finalizeUnconditionally(VM&); > > private: >@@ -469,27 +470,10 @@ class WeakMapImpl : public JSDestructibleObject { > 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; > }; > > } // namespace JSC >diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog >index e37f8dbde18a69a059aa8c937fa29589ecfd92e4..5d7ed997289376da9f0279a2e8f64d89c69c939d 100644 >--- a/JSTests/ChangeLog >+++ b/JSTests/ChangeLog >@@ -1,3 +1,12 @@ >+2018-06-01 Yusuke Suzuki <utatane.tea@gmail.com> >+ >+ [JSC] Remove WeakReferenceHarvester >+ https://bugs.webkit.org/show_bug.cgi?id=186102 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * microbenchmarks/create-many-weak-map.js: Added. >+ > 2018-05-29 Yusuke Suzuki <utatane.tea@gmail.com> > > [JSC] Add Symbol.prototype.description getter >diff --git a/JSTests/microbenchmarks/create-many-weak-map.js b/JSTests/microbenchmarks/create-many-weak-map.js >new file mode 100644 >index 0000000000000000000000000000000000000000..1d30d123b41b6f67600ec3b17e27821c42f3dc96 >--- /dev/null >+++ b/JSTests/microbenchmarks/create-many-weak-map.js >@@ -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); >+ } >+}
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 186102
:
341582
|
341583
|
341842
|
341844