[webkit-reviews] review granted: [Bug 98792] Copying collection shouldn't require O(live bytes) memory overhead : [Attachment 168285] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 98792: Copying collection shouldn't require O(live bytes) memory overhead
https://bugs.webkit.org/show_bug.cgi?id=98792

Attachment 168285: Patch
https://bugs.webkit.org/attachment.cgi?id=168285&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=168285&action=review


> Source/JavaScriptCore/heap/GCThread.cpp:88
> +    {
> +	   MutexLocker locker(m_shared.m_markingLock);
> +    }

Add a comment to say why this trick works.

> Source/JavaScriptCore/heap/GCThread.cpp:94
> +		   m_slotVisitor->drainFromShared(SlotVisitor::SlaveDrain);

One sentence comment about termination condition for marking, please.

> Source/JavaScriptCore/heap/GCThread.cpp:98
> +		   m_copyVisitor->copyFromShared();

One sentence comment about termination condition for copying, please.

> Source/JavaScriptCore/heap/Heap.h:267
> +    struct CopyFunctor : public MarkedBlock::VoidFunctor {

Up to you, but you could rename it to something like
MarkedBlockSnapshotFunctor.