Bug 194187

Summary: [JSC] Decouple JIT related data from CodeBlock
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 193606    
Attachments:
Description Flags
Patch
none
Patch
none
Patch saam: review+

Yusuke Suzuki
Reported 2019-02-01 20:15:36 PST
[JSC] Decouple JIT related data from CodeBlock
Attachments
Patch (36.81 KB, patch)
2019-02-01 20:24 PST, Yusuke Suzuki
no flags
Patch (36.90 KB, patch)
2019-02-01 20:40 PST, Yusuke Suzuki
no flags
Patch (36.91 KB, patch)
2019-02-01 20:50 PST, Yusuke Suzuki
saam: review+
Yusuke Suzuki
Comment 1 2019-02-01 20:24:13 PST
Yusuke Suzuki
Comment 2 2019-02-01 20:40:53 PST
Yusuke Suzuki
Comment 3 2019-02-01 20:50:40 PST
Yusuke Suzuki
Comment 4 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*)
Saam Barati
Comment 5 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
Saam Barati
Comment 6 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
Yusuke Suzuki
Comment 7 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.
Yusuke Suzuki
Comment 8 2019-02-01 23:15:53 PST
Radar WebKit Bug Importer
Comment 9 2019-02-01 23:16:28 PST
Note You need to log in before you can comment on or make changes to this bug.