[JSC] Number of SlotVisitors can increase after setting up m_visitCounters
Created attachment 329573 [details] Patch
Comment on attachment 329573 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329573&action=review > Source/JavaScriptCore/heap/HeapInlines.h:274 > + auto locker = holdLock(m_parallelSlotVisitorLock); In this patch, we keep this lock and the current mechanism. But I think we should create all SlotVisitors in Heap's constructor based on HeapHelperPool's # of threads. The size of m_parallelSlotVisitors is capped with this value, and it should not be so large. At that time, we can remove this m_parallelSlotVisitorLock in this function and forEachSlotVisitor function. I opened a bug for this, which is separated from this bug. https://bugs.webkit.org/show_bug.cgi?id=180907
Comment on attachment 329573 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329573&action=review Nice! >> Source/JavaScriptCore/heap/HeapInlines.h:274 >> + auto locker = holdLock(m_parallelSlotVisitorLock); > > In this patch, we keep this lock and the current mechanism. > But I think we should create all SlotVisitors in Heap's constructor based on HeapHelperPool's # of threads. > The size of m_parallelSlotVisitors is capped with this value, and it should not be so large. > At that time, we can remove this m_parallelSlotVisitorLock in this function and forEachSlotVisitor function. > I opened a bug for this, which is separated from this bug. > https://bugs.webkit.org/show_bug.cgi?id=180907 I think it's safe to assume that Vector::size() doesn't need locking. On the other hand, it's also OK to be paranoid. Up to you!
Comment on attachment 329573 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=329573&action=review Thank you for your review! >>> Source/JavaScriptCore/heap/HeapInlines.h:274 >>> + auto locker = holdLock(m_parallelSlotVisitorLock); >> >> In this patch, we keep this lock and the current mechanism. >> But I think we should create all SlotVisitors in Heap's constructor based on HeapHelperPool's # of threads. >> The size of m_parallelSlotVisitors is capped with this value, and it should not be so large. >> At that time, we can remove this m_parallelSlotVisitorLock in this function and forEachSlotVisitor function. >> I opened a bug for this, which is separated from this bug. >> https://bugs.webkit.org/show_bug.cgi?id=180907 > > I think it's safe to assume that Vector::size() doesn't need locking. On the other hand, it's also OK to be paranoid. Up to you! I would like to keep this for a while. Once bug 180907 is fixed, we can remove this checking :)
Committed r226010: <https://trac.webkit.org/changeset/226010>
<rdar://problem/36094575>