This is a big overhead during EdenCollections. We should think about how we can generationalize our CodeBlock lifetimes.
Created attachment 222707 [details] Patch
Comment on attachment 222707 [details] Patch r=me
(In reply to comment #1) > Created an attachment (id=222707) [details] > Patch Hmm, I think this patch has issues. The correctness of the weakReferenceHarvester stuff in which CodeBlocks participate is dependent on visiting all marked CodeBlocks during a collection. Otherwise we won't propagate liveness for all the things that hang off of CodeBlock that need it. Another alternative is to just ignore all new CodeBlocks (i.e. auto-promote, pretenure, etc.). We'd still have to visit them to LUB value profiles.
Created attachment 228214 [details] Patch
Created attachment 228216 [details] Patch
Comment on attachment 228216 [details] Patch 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().
Comment on attachment 228216 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228216&action=review > Source/JavaScriptCore/heap/CodeBlockSet.cpp:135 > + m_oldCodeBlocks.remove(codeBlock); Needs early return.
Created attachment 228428 [details] Final patch
Comment on attachment 228428 [details] Final patch View in context: https://bugs.webkit.org/attachment.cgi?id=228428&action=review > Source/JavaScriptCore/bytecode/CodeBlock.h:151 > + template <typename Functor> void forEachRelatedCodeBlock(Functor&& functor) > + { > + Functor f(std::forward<Functor>(functor)); > + Vector<CodeBlock*, 4> codeBlocks; > + codeBlocks.append(this); > + > + while (!codeBlocks.isEmpty()) { > + CodeBlock* currentCodeBlock = codeBlocks.takeLast(); > + f(currentCodeBlock); > + > + if (CodeBlock* alternative = currentCodeBlock->alternative()) > + codeBlocks.append(alternative); > + if (CodeBlock* osrEntryBlock = currentCodeBlock->specialOSREntryBlockOrNull()) > + codeBlocks.append(osrEntryBlock); > + } > + } I had to add handling of the OSR entry block along with the alternative blocks. To make it more general, I turned it into a DFS of a DAG. There's no defense against cycles.
Comment on attachment 228428 [details] Final patch r=me
Comment on attachment 228428 [details] Final patch Clearing flags on attachment: 228428 Committed r166678: <http://trac.webkit.org/changeset/166678>
All reviewed patches have been landed. Closing bug.