[webkit-reviews] review denied: [Bug 85429] Heap should sweep incrementally : [Attachment 144651] 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 85429: Heap should sweep incrementally
https://bugs.webkit.org/show_bug.cgi?id=85429

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

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


> Source/JavaScriptCore/heap/Heap.cpp:711
> +    m_objectSpace.didFinishMarking();
> +    m_sweeper->didFinishMarking();

I'd prefer to see a more explicit interface:
m_sweeper->startSweeping(m_objectSpace.copyToVector()).

> Source/JavaScriptCore/heap/Heap.cpp:794
> +    m_bytesAbandoned = 0;

I tend to think that any GC should recent m_bytesAbandoned to 0, no?

> Source/JavaScriptCore/heap/MarkedSpace.cpp:239
> +    unsigned blocksSwept = 0;
> +    for (; m_currentBlockToSweepIndex < m_blocksToSweep.size();
m_currentBlockToSweepIndex++) {

I think it would make more sense to put this state and functionality into the
incremental sweeper.