Bug 194187 - [JSC] Decouple JIT related data from CodeBlock
Summary: [JSC] Decouple JIT related data from CodeBlock
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks: 193606
  Show dependency treegraph
 
Reported: 2019-02-01 20:15 PST by Yusuke Suzuki
Modified: 2019-02-04 17:34 PST (History)
6 users (show)

See Also:


Attachments
Patch (36.81 KB, patch)
2019-02-01 20:24 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (36.90 KB, patch)
2019-02-01 20:40 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (36.91 KB, patch)
2019-02-01 20:50 PST, Yusuke Suzuki
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-02-01 20:15:36 PST
[JSC] Decouple JIT related data from CodeBlock
Comment 1 Yusuke Suzuki 2019-02-01 20:24:13 PST
Created attachment 360954 [details]
Patch
Comment 2 Yusuke Suzuki 2019-02-01 20:40:53 PST
Created attachment 360956 [details]
Patch
Comment 3 Yusuke Suzuki 2019-02-01 20:50:40 PST
Created attachment 360957 [details]
Patch
Comment 4 Yusuke Suzuki 2019-02-01 21:06:11 PST
FYI, there are list of functions which create JITData in RAMification. We can ensure that JITData is only created by JIT compilers, or after CodeBlock is JIT compiled.

JSC::DFG::SpeculativeJIT::compileValueAdd(JSC::DFG::Node*)
JSC::DFG::SpeculativeJIT::compileValueMul(JSC::DFG::Node*)
JSC::DFG::SpeculativeJIT::compileValueNegate(JSC::DFG::Node*)
JSC::DFG::SpeculativeJIT::compileValueSub(JSC::DFG::Node*)
JSC::DFG::SpeculativeJIT::emitCall(JSC::DFG::Node*)
JSC::JIT::emit_op_add(JSC::Instruction const*)
JSC::JIT::emit_op_get_by_val(JSC::Instruction const*)
JSC::JIT::emit_op_has_indexed_property(JSC::Instruction const*)
JSC::JIT::emit_op_mul(JSC::Instruction const*)
JSC::JIT::emit_op_negate(JSC::Instruction const*)
JSC::JIT::emit_op_sub(JSC::Instruction const*)
JSC::JIT::privateCompileSlowCases()
JSC::JITGetByIdGenerator::JITGetByIdGenerator(JSC::CodeBlock*, JSC::CodeOrigin, JSC::CallSiteIndex, JSC::RegisterSet const&, WTF::UniquedStringImpl*, JSC::JSValueRegs, JSC::JSValueRegs, JSC::AccessType)
JSC::JITGetByIdWithThisGenerator::JITGetByIdWithThisGenerator(JSC::CodeBlock*, JSC::CodeOrigin, JSC::CallSiteIndex, JSC::RegisterSet const&, WTF::UniquedStringImpl*, JSC::JSValueRegs, JSC::JSValueRegs, JSC::JSValueRegs, JSC::AccessType)
JSC::JITInByIdGenerator::JITInByIdGenerator(JSC::CodeBlock*, JSC::CodeOrigin, JSC::CallSiteIndex, JSC::RegisterSet const&, WTF::UniquedStringImpl*, JSC::JSValueRegs, JSC::JSValueRegs)
JSC::JITInstanceOfGenerator::JITInstanceOfGenerator(JSC::CodeBlock*, JSC::CodeOrigin, JSC::CallSiteIndex, JSC::RegisterSet const&, JSC::X86Registers::RegisterID, JSC::X86Registers::RegisterID, JSC::X86Registers::RegisterID, JSC::X86Registers::RegisterID, JSC::X86Registers::RegisterID, bool)
JSC::JITPutByIdGenerator::JITPutByIdGenerator(JSC::CodeBlock*, JSC::CodeOrigin, JSC::CallSiteIndex, JSC::RegisterSet const&, JSC::JSValueRegs, JSC::JSValueRegs, JSC::X86Registers::RegisterID, JSC::ECMAMode, JSC::PutKind)
JSC::PolymorphicCallStubRoutine::PolymorphicCallStubRoutine(JSC::MacroAssemblerCodeRef<(WTF::PtrTag)49594> const&, JSC::VM&, JSC::JSCell const*, JSC::ExecState*, JSC::CallLinkInfo&, WTF::Vector<JSC::PolymorphicCallCase, 0ul, WTF::CrashOnOverflow, 16ul> const&, std::__1::unique_ptr<unsigned int [], WTF::FastFree<unsigned int []> >&&)
JSC::linkDirectFor(JSC::ExecState*, JSC::CallLinkInfo&, JSC::CodeBlock*, JSC::MacroAssemblerCodePtr<(WTF::PtrTag)357>)
JSC::linkFor(JSC::ExecState*, JSC::CallLinkInfo&, JSC::CodeBlock*, JSC::JSObject*, JSC::MacroAssemblerCodePtr<(WTF::PtrTag)357>)
WTF::SharedTaskFunctor<void (JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&), JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileCallOrConstruct()::'lambda'(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)>::run(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)
WTF::SharedTaskFunctor<void (JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&), JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileCallOrConstructVarargs()::'lambda'(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)>::run(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)
WTF::SharedTaskFunctor<void (JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&), JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileCallOrConstructVarargsSpread()::'lambda'(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)>::run(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)
WTF::SharedTaskFunctor<void (JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&), JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileDirectCallOrConstruct()::'lambda'(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)>::run(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)
WTF::SharedTaskFunctor<void (JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&), JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileTailCall()::'lambda'(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)>::run(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)
WTF::SharedTaskFunctor<void (JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&), void JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileBinaryMathIC<JSC::JITAddGenerator, long long (*)(JSC::ExecState*, long long, long long, JSC::JITBinaryMathIC<JSC::JITAddGenerator>*), long long (*)(JSC::ExecState*, long long, long long), void>(JSC::ArithProfile*, JSC::Instruction const*, long long (*)(JSC::ExecState*, long long, long long, JSC::JITBinaryMathIC<JSC::JITAddGenerator>*
), long long (*)(JSC::ExecState*, long long, long long))::'lambda'(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)>::run(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)
WTF::SharedTaskFunctor<void (JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&), void JSC::FTL::(anonymous namespace)::LowerDFGToB3::compileBinaryMathIC<JSC::JITSubGenerator, long long (*)(JSC::ExecState*, long long, long long, JSC::JITBinaryMathIC<JSC::JITSubGenerator>*), long long (*)(JSC::ExecState*, long long, long long), void>(JSC::ArithProfile*, JSC::Instruction const*, long long (*)(JSC::ExecState*, long long, long long, JSC::JITBinaryMathIC<JSC::JITSubGenerator>*
), long long (*)(JSC::ExecState*, long long, long long))::'lambda'(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)>::run(JSC::CCallHelpers&, JSC::B3::StackmapGenerationParams const&)
void JSC::JIT::compileOpCall<JSC::OpCall>(JSC::Instruction const*, unsigned int)
void JSC::JIT::compileOpCall<JSC::OpCallVarargs>(JSC::Instruction const*, unsigned int)
void JSC::JIT::compileOpCall<JSC::OpConstruct>(JSC::Instruction const*, unsigned int)
void JSC::JIT::compileOpCall<JSC::OpTailCall>(JSC::Instruction const*, unsigned int)
void JSC::JIT::emit_op_put_by_val<JSC::OpPutByVal>(JSC::Instruction const*)
Comment 5 Saam Barati 2019-02-01 21:53:59 PST
Comment on attachment 360957 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=360957&action=review

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1364
>  void CodeBlock::finalizeBaselineJITInlineCaches()

Don’t we want to be holding the lock while iterating these data structures?

> Source/JavaScriptCore/bytecode/CodeBlock.h:989
> +    std::unique_ptr<JITData> m_jitData;

Why not just put this on the base class for JITCode for baseline and above? It would save us one pointer stored in CodeBlock
Comment 6 Saam Barati 2019-02-01 21:55:12 PST
Comment on attachment 360957 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=360957&action=review

> Source/JavaScriptCore/ChangeLog:23
> +        The size of CodeBlock is reduced from 512 to 352.

We should make sure we have a 352 byte size class if we don’t already
Comment 7 Yusuke Suzuki 2019-02-01 23:12:37 PST
Comment on attachment 360957 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=360957&action=review

Thanks!

>> Source/JavaScriptCore/ChangeLog:23
>> +        The size of CodeBlock is reduced from 512 to 352.
> 
> We should make sure we have a 352 byte size class if we don’t already

Since CodeBlock is in IsoSubspace, we use 352 as its size.

>> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1364
>>  void CodeBlock::finalizeBaselineJITInlineCaches()
> 
> Don’t we want to be holding the lock while iterating these data structures?

Since this is finalizer in GC, concurrent compiler threads already stop.

>> Source/JavaScriptCore/bytecode/CodeBlock.h:989
>> +    std::unique_ptr<JITData> m_jitData;
> 
> Why not just put this on the base class for JITCode for baseline and above? It would save us one pointer stored in CodeBlock

Currently, m_jitCode is accessed from JIT code through jitCodeOffset(). Maybe, we can do that, but I would like to do that in a separate patch.
And we also have a chance to put baseline counter, and OSR counters in this data. But currently, they are also touched from JIT code. That's why I'm now postponing them.
Comment 8 Yusuke Suzuki 2019-02-01 23:15:53 PST
Committed r240893: <https://trac.webkit.org/changeset/240893>
Comment 9 Radar WebKit Bug Importer 2019-02-01 23:16:28 PST
<rdar://problem/47760033>