[webkit-reviews] review denied: [Bug 98084] Block freeing thread should sleep indefinitely when there's no work to do : [Attachment 166562] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 98084: Block freeing thread should sleep indefinitely when there's no work
to do
https://bugs.webkit.org/show_bug.cgi?id=98084

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

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=166562&action=review


This fix looks correct, but I think it can get better.

> Source/JavaScriptCore/heap/BlockAllocator.h:111
> +	   m_numberOfFreeBlocks++;

Can we just use m_numberOfFreeBlocks != 0, instead of a separate
m_blocksNeedFreeing boolean, to tell if we need to free anything?

> Source/JavaScriptCore/heap/BlockAllocator.h:118
> +    MutexLocker mutexLocker(m_freeBlockConditionLock);
> +    if (!m_blocksNeedFreeing) {
> +	   m_blocksNeedFreeing = true;
> +	   m_freeBlockCondition.signal();
> +    }

Rather than doing this unconditionally, I'd prefer to see this done only if
m_numberOfFreeBlocks was 0 when we acquired the lock.