[webkit-reviews] review denied: [Bug 45186] Use the Windows thread pool instead of an extra thread for FastMalloc scavenging : [Attachment 78983] 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 78983: Patch
https://bugs.webkit.org/attachment.cgi?id=78983&action=review

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

> Source/JavaScriptCore/wtf/FastMalloc.cpp:1566
> +    // We need to use WT_EXECUTEONLYONCE here and reschedule the timer,
because
> +    // Windows will fire the timer event even when the function is already
running.

Passing 0 for the repeat duration should be enough to make the timer fire only
once. If you pass WT_EXECUTEDEFAULT, are you then able to use
ChangeTimerQueueTimer to reuse the same timer? If that still doesn't work, I
think it's worth adding a comment in rescheduleScavenger and/or
suspendScavenger that explains why we have to delete and recreate the timer.

> Source/JavaScriptCore/wtf/FastMalloc.cpp:2463
> +    if (shouldScavenge()) {
> +	   suspendScavenger();
> +	   return;
> +    }

I think you're missing a ! in there.