Bug 194187 – [JSC] Decouple JIT related data from CodeBlock

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Decouple JIT related data from CodeBlock

[Yusuke Suzuki]

Created attachment 360954 [details]
Patch

[Yusuke Suzuki]

Created attachment 360956 [details]
Patch

[Yusuke Suzuki]

Created attachment 360957 [details]
Patch

[Yusuke Suzuki]

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*)

[Saam Barati]

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

[Saam Barati]

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

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Committed r240893: <https://trac.webkit.org/changeset/240893>

[Radar WebKit Bug Importer]

<rdar://problem/47760033>