[webkit-reviews] review denied: [Bug 45186] Use the Windows thread pool instead of an extra thread for FastMalloc scavenging : [Attachment 78526] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 45186: Use the Windows thread pool instead of an extra thread for
FastMalloc scavenging
https://bugs.webkit.org/show_bug.cgi?id=45186

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

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=78526&action=review

> Source/JavaScriptCore/wtf/FastMalloc.cpp:1566
> +    while (1) {
> +	   if (runScavenge())
> +	       continue;
> +
> +	   pthread_mutex_lock(&m_scavengeMutex);
> +	   m_scavengeThreadActive = false;
> +	   // Block until there are enough free committed pages to release back
to the system.
> +	   pthread_cond_wait(&m_scavengeCondition, &m_scavengeMutex);
> +	   m_scavengeThreadActive = true;
> +	   pthread_mutex_unlock(&m_scavengeMutex);
> +    }

I think it would be a lot better if we followed the libdispatch implementation.
periodicScavenge just performs a single scavenge, then waits for the timer to
fire again (or stops the timer if no more scavenging is needed). That way we
don't tie up a worker thread while we sleep and block.