[webkit-reviews] review granted: [Bug 174973] Use one Scavenger thread for all Heaps : [Attachment 322957] the patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 6 09:11:29 PDT 2017


JF Bastien <jfbastien at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 174973: Use one Scavenger thread for all Heaps
https://bugs.webkit.org/show_bug.cgi?id=174973

Attachment 322957: the patch

https://bugs.webkit.org/attachment.cgi?id=322957&action=review




--- Comment #5 from JF Bastien <jfbastien at apple.com> ---
Comment on attachment 322957
  --> https://bugs.webkit.org/attachment.cgi?id=322957
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=322957&action=review

r=me after a few fixes.

> Source/bmalloc/bmalloc/Scavenger.cpp:78
> +    m_isGrowing = true;

Can you call it "m_isProbablyGrowing" or something?

> Source/bmalloc/bmalloc/Scavenger.cpp:87
> +void Scavenger::scheduleIfUnderMemoryPressureHoldingLock(size_t bytes)

Shouldn't you pass in the lock here, to make sure it's actually holding it?
Then the name can be shorter.

> Source/bmalloc/bmalloc/Scavenger.cpp:102
> +    runHoldingLock();

Ditto.

> Source/bmalloc/bmalloc/Scavenger.cpp:139
> +    auto truth = [] { return true; };

Wat?


More information about the webkit-reviews mailing list