[webkit-reviews] review granted: [Bug 132088] The GC should only resume compiler threads that it suspended in the same GC pass : [Attachment 230022] the patch

[bugzilla-daemon at webkit.org]

Mark Hahnenberg <mhahnenberg at apple.com> has granted Mark Lam
<mark.lam at apple.com>'s request for review:
Bug 132088: The GC should only resume compiler threads that it suspended in the
same GC pass
https://bugs.webkit.org/show_bug.cgi?id=132088

Attachment 230022: the patch
https://bugs.webkit.org/attachment.cgi?id=230022&action=review

------- Additional Comments from Mark Hahnenberg <mhahnenberg at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=230022&action=review


r=me

> Source/JavaScriptCore/heap/Heap.cpp:1025
> +    ASSERT(!m_suspendedCompilerWorklists.size());

I find it's more readable to say "m_suspendedCompilerWorklists.isEmpty()"
instead.

> Source/JavaScriptCore/heap/Heap.cpp:1235
> +    for (auto worklist : m_suspendedCompilerWorklists)
> +	   worklist->resumeAllThreads();
> +    m_suspendedCompilerWorklists.clear();

You should change the other places in the GC to only iterate this list. If the
GC itself didn't suspend a work list then it shouldn't be touching that work
list at all.