[webkit-reviews] review granted: [Bug 202503] Throw away baseline code if there is an optimized replacement : [Attachment 387554] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 13 14:57:13 PST 2020
Yusuke Suzuki <ysuzuki at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 202503: Throw away baseline code if there is an optimized replacement
https://bugs.webkit.org/show_bug.cgi?id=202503
Attachment 387554: patch
https://bugs.webkit.org/attachment.cgi?id=387554&action=review
--- Comment #9 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 387554
--> https://bugs.webkit.org/attachment.cgi?id=387554
patch
View in context: https://bugs.webkit.org/attachment.cgi?id=387554&action=review
r=me
> Source/JavaScriptCore/ChangeLog:12
> + This patch's goal is to help us save JIT executable memory by
throwing
> + away baseline code when it has an optimized replacement. To make it
> + easy to reason about, we do this when finalizing a GC, and when the
> + CodeBlock is not on the stack. When we do this, we throw away all
JIT
> + data and unlink all incoming calls.
Nice.
> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1389
> + if (!Options::forceBaseline() && jitType() == JITType::BaselineJIT &&
!m_vm->heap.codeBlockSet().isCurrentlyExecuting(this)) {
Adding some comment about what it is doing would be nice, something like,
// If BaselineJIT code is not executed and optimized replacement exists, we
attempt to make BaselineJIT CodeBlock back to LLInt CodeBlock to purge unused
JIT code.
> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1390
> + if (CodeBlock* optimizedCodeBlock = optimizedReplacement()) {
Is it safe to call this in CodeBlock::finalizeUnconditionally while
ExecutableToCodeBlockEdge::finalizeUnconditionally is not called yet?
Should we replace the order of finalizeUnconditionally between CodeBlock and
ExecutableToCodeBlockEdge?
> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1391
> + if (!optimizedCodeBlock->m_osrExitCounter) {
This is heuristic like, "If DFG / FTL OSR exit happens, we could recompile it
so we keep Baseline JIT code", right?
> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1622
> + m_jitData = nullptr;
Nice. We can now purge JITData for CodeBlock by changing BaselineJIT -> LLInt.
> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:-2005
> - auto nextBytecode = codeBlock->jitCodeMap().find(nextBytecodeIndex);
I wonder if we can remove jitCodeMap completely, or shrink size of it by only
recording some part of it (only recording op_loop_hint place). Can you file a
bug and put FIXME to JITCodeMap.h?
More information about the webkit-reviews
mailing list