[webkit-reviews] review granted: [Bug 127152] CodeBlockSet should be generational : [Attachment 228216] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 127152: CodeBlockSet should be generational
https://bugs.webkit.org/show_bug.cgi?id=127152

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

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


r=me

> Source/JavaScriptCore/bytecode/CodeBlock.h:1004
> +	   // During EdenCollections all CodeBlocks that are visited are
assumed to be live.

Would be nice to explain our reasoning here: It's probably cheaper to assume
liveness, and we don't want to worry about write barrier subtleties required by
the mutually weak liveness methodology.

> Source/JavaScriptCore/heap/CodeBlockSet.cpp:71
> +void CodeBlockSet::clearAllMarks()
>  {
> -    for (CodeBlock* codeBlock : m_set) {
> +    promoteYoungCodeBlocks();
> +    for (CodeBlock* codeBlock : m_oldCodeBlocks) {
>	   codeBlock->m_mayBeExecuting = false;
>	   codeBlock->m_visitAggregateHasBeenCalled = false;
>      }
>  }

It feels strange for "clearAllMarks" to promote. And might this get us into a
bad state if deleteAllCompiledCode is called at the start of an eden
collection? Similarly strange, though less so, for
deleteUnmarkedAndUnreferenced to promote.

Can we make the caller promote instead? For example,
Heap::deleteAllCompiledCode could run only during full collections, and
Heap::markRoots could eagerly promote code blocks before full collections and
lazily promote code blocks after eden collections. Or, Heap::resetAllocators()
could promote code blocks unconditionally, and Heap::deleteAllCompiledCode
could run after Heap::resetAllocators().