Title: [277475] trunk/Source/_javascript_Core
- Revision
- 277475
- Author
- [email protected]
- Date
- 2021-05-13 19:03:43 -0700 (Thu, 13 May 2021)
Log Message
m_calleeSaveRegisters should not be a pointer to a pointer
https://bugs.webkit.org/show_bug.cgi?id=225787
Reviewed by Keith Miller.
Ben found this through memory stress testing.
RegisterAtOffsetList is effectively just a pointer. unique_ptr<RegisterAtOffsetList>
is a pointer to a pointer. RegisterAtOffsetList is long-lived, so it
creates heap page fragmentation.
Worth 3MB on Ben's test.
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::setCalleeSaveRegisters):
(JSC::CodeBlock::calleeSaveRegisters const): Use a fence before setting
m_hasCalleeSaveRegisters to ensure that all writes have completed before
the struct becomes visible.
* bytecode/CodeBlock.h: Use RegisterAtOffsetList directly instead of
unique_ptr<RegisterAtOffsetList> to avoid a long-lived lonely 8 byte
allocation.
* ftl/FTLCompile.cpp:
(JSC::FTL::compile): Updated for type change.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (277474 => 277475)
--- trunk/Source/_javascript_Core/ChangeLog 2021-05-14 01:48:25 UTC (rev 277474)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-05-14 02:03:43 UTC (rev 277475)
@@ -1,3 +1,31 @@
+2021-05-13 Geoffrey Garen <[email protected]>
+
+ m_calleeSaveRegisters should not be a pointer to a pointer
+ https://bugs.webkit.org/show_bug.cgi?id=225787
+
+ Reviewed by Keith Miller.
+
+ Ben found this through memory stress testing.
+
+ RegisterAtOffsetList is effectively just a pointer. unique_ptr<RegisterAtOffsetList>
+ is a pointer to a pointer. RegisterAtOffsetList is long-lived, so it
+ creates heap page fragmentation.
+
+ Worth 3MB on Ben's test.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::setCalleeSaveRegisters):
+ (JSC::CodeBlock::calleeSaveRegisters const): Use a fence before setting
+ m_hasCalleeSaveRegisters to ensure that all writes have completed before
+ the struct becomes visible.
+
+ * bytecode/CodeBlock.h: Use RegisterAtOffsetList directly instead of
+ unique_ptr<RegisterAtOffsetList> to avoid a long-lived lonely 8 byte
+ allocation.
+
+ * ftl/FTLCompile.cpp:
+ (JSC::FTL::compile): Updated for type change.
+
2021-05-13 Chris Dumez <[email protected]>
Rename FileSystem::directoryName() to FileSystem::parentPath()
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (277474 => 277475)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2021-05-14 01:48:25 UTC (rev 277474)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2021-05-14 02:03:43 UTC (rev 277475)
@@ -1790,16 +1790,24 @@
return 0;
}
-void CodeBlock::setCalleeSaveRegisters(RegisterSet calleeSaveRegisters)
+void CodeBlock::setCalleeSaveRegisters(RegisterSet registerSet)
{
+ auto calleeSaveRegisters = RegisterAtOffsetList(registerSet);
+
ConcurrentJSLocker locker(m_lock);
- ensureJITData(locker).m_calleeSaveRegisters = makeUnique<RegisterAtOffsetList>(calleeSaveRegisters);
+ auto& jitData = ensureJITData(locker);
+ jitData.m_calleeSaveRegisters = WTFMove(calleeSaveRegisters);
+ WTF::storeStoreFence();
+ jitData.m_hasCalleeSaveRegisters = true;
}
-void CodeBlock::setCalleeSaveRegisters(std::unique_ptr<RegisterAtOffsetList> registerAtOffsetList)
+void CodeBlock::setCalleeSaveRegisters(RegisterAtOffsetList&& registerAtOffsetList)
{
ConcurrentJSLocker locker(m_lock);
- ensureJITData(locker).m_calleeSaveRegisters = WTFMove(registerAtOffsetList);
+ auto& jitData = ensureJITData(locker);
+ jitData.m_calleeSaveRegisters = WTFMove(registerAtOffsetList);
+ WTF::storeStoreFence();
+ jitData.m_hasCalleeSaveRegisters = true;
}
void CodeBlock::resetJITData()
@@ -2492,8 +2500,8 @@
{
#if ENABLE(JIT)
if (auto* jitData = m_jitData.get()) {
- if (const RegisterAtOffsetList* registers = jitData->m_calleeSaveRegisters.get())
- return registers;
+ if (jitData->m_hasCalleeSaveRegisters)
+ return &jitData->m_calleeSaveRegisters;
}
#endif
return &RegisterAtOffsetList::llintBaselineCalleeSaveRegisters();
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (277474 => 277475)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2021-05-14 01:48:25 UTC (rev 277474)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2021-05-14 02:03:43 UTC (rev 277475)
@@ -64,6 +64,7 @@
#include "ProfilerJettisonReason.h"
#include "ProgramExecutable.h"
#include "PutPropertySlot.h"
+#include "RegisterAtOffsetList.h"
#include "ValueProfile.h"
#include "VirtualRegister.h"
#include "Watchpoint.h"
@@ -283,7 +284,8 @@
FixedVector<SimpleJumpTable> m_switchJumpTables;
FixedVector<StringJumpTable> m_stringSwitchJumpTables;
std::unique_ptr<PCToCodeOriginMap> m_pcToCodeOriginMap;
- std::unique_ptr<RegisterAtOffsetList> m_calleeSaveRegisters;
+ bool m_hasCalleeSaveRegisters { false };
+ RegisterAtOffsetList m_calleeSaveRegisters;
JITCodeMap m_jitCodeMap;
};
@@ -343,7 +345,7 @@
Optional<CodeOrigin> findPC(void* pc);
void setCalleeSaveRegisters(RegisterSet);
- void setCalleeSaveRegisters(std::unique_ptr<RegisterAtOffsetList>);
+ void setCalleeSaveRegisters(RegisterAtOffsetList&&);
void setRareCaseProfiles(FixedVector<RareCaseProfile>&&);
RareCaseProfile* rareCaseProfileForBytecodeIndex(const ConcurrentJSLocker&, BytecodeIndex);
Modified: trunk/Source/_javascript_Core/ftl/FTLCompile.cpp (277474 => 277475)
--- trunk/Source/_javascript_Core/ftl/FTLCompile.cpp 2021-05-14 01:48:25 UTC (rev 277474)
+++ trunk/Source/_javascript_Core/ftl/FTLCompile.cpp 2021-05-14 02:03:43 UTC (rev 277475)
@@ -71,10 +71,9 @@
if (state.allocationFailed)
return;
- std::unique_ptr<RegisterAtOffsetList> registerOffsets =
- makeUnique<RegisterAtOffsetList>(state.proc->calleeSaveRegisterAtOffsetList());
+ RegisterAtOffsetList registerOffsets = state.proc->calleeSaveRegisterAtOffsetList();
if (shouldDumpDisassembly())
- dataLog(tierName, "Unwind info for ", CodeBlockWithJITType(codeBlock, JITType::FTLJIT), ": ", *registerOffsets, "\n");
+ dataLog(tierName, "Unwind info for ", CodeBlockWithJITType(codeBlock, JITType::FTLJIT), ": ", registerOffsets, "\n");
codeBlock->setCalleeSaveRegisters(WTFMove(registerOffsets));
ASSERT(!(state.proc->frameSize() % sizeof(EncodedJSValue)));
state.jitCode->common.frameRegisterCount = state.proc->frameSize() / sizeof(EncodedJSValue);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
