RESOLVED FIXED 212765
Add a Thread Specific Cache for LinkBuffer::CompactAndLinkCode()
https://bugs.webkit.org/show_bug.cgi?id=212765
Summary Add a Thread Specific Cache for LinkBuffer::CompactAndLinkCode()
Michael Saboff
Reported 2020-06-04 11:15:32 PDT
Similar to https://bugs.webkit.org/show_bug.cgi?id=212562, we should add a Thread Local Buffer for LinkBuffer::copyCompactAndLinkCode(). When compacting JIT code, for most CPU types we need a second data buffer. We should make this buffer a thread local just like we did for the AssemblerBuffer in 212562.
Attachments
Patch (4.28 KB, patch)
2020-06-04 11:23 PDT, Michael Saboff
saam: review+
Patch for landing (4.21 KB, patch)
2020-06-04 13:09 PDT, Michael Saboff
msaboff: review+
Michael Saboff
Comment 1 2020-06-04 11:17:32 PDT
Michael Saboff
Comment 2 2020-06-04 11:23:00 PDT
Saam Barati
Comment 3 2020-06-04 12:15:51 PDT
Comment on attachment 401051 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401051&action=review r=me > Source/JavaScriptCore/assembler/LinkBuffer.cpp:125 > +typedef ThreadSpecific<BranchCompactionLinkBuffer, WTF::CanBeGCThread::True> ThreadSpecificBranchCompactionLinkBuffer; nit: "using" instead of "typedef" > Source/JavaScriptCore/assembler/LinkBuffer.cpp:201 > + uint8_t* m_data; > + size_t m_size; nit: you can give these fields default values so you don't have to initialize them in 2 different ctors > Source/JavaScriptCore/assembler/LinkBuffer.cpp:232 > + BranchCompactionLinkBuffer outBuffer(m_size); can we do the FAST_JIT_PERMISSIONS&&arm64e path above in a runtime supported way for other devices too? Seems purely better than this, since it doesn't require a side buffer. There are a few arm64 devices that at runtime support APRR Maybe as a follow-up in a separate patch
Michael Saboff
Comment 4 2020-06-04 13:09:07 PDT
Created attachment 401067 [details] Patch for landing > > Source/JavaScriptCore/assembler/LinkBuffer.cpp:125 > > +typedef ThreadSpecific<BranchCompactionLinkBuffer, WTF::CanBeGCThread::True> ThreadSpecificBranchCompactionLinkBuffer; > > nit: "using" instead of "typedef" Done. > > Source/JavaScriptCore/assembler/LinkBuffer.cpp:201 > > + uint8_t* m_data; > > + size_t m_size; > > nit: you can give these fields default values so you don't have to > initialize them in 2 different ctors Done.
Michael Saboff
Comment 5 2020-06-04 19:41:01 PDT
Note You need to log in before you can comment on or make changes to this bug.