- Revision
- 164018
- Author
- [email protected]
- Date
- 2014-02-12 22:50:09 -0800 (Wed, 12 Feb 2014)
Log Message
No need to save reservedZoneSize when dropping the JSLock.
<https://webkit.org/b/128719>
Reviewed by Geoffrey Garen.
Source/_javascript_Core:
The reservedZoneSize does not change due to the VM being run on a different
thread. Hence, there is no need to save and restore its value. Instead of
calling updateReservedZoneSize() to update the stack limit, we now call
setStackPointerAtVMEntry() to do the job. setStackPointerAtVMEntry()
will update the stackPointerAtVMEntry and delegate to updateStackLimit() to
update the stack limit based on the new stackPointerAtVMEntry.
* runtime/ErrorHandlingScope.cpp:
(JSC::ErrorHandlingScope::ErrorHandlingScope):
(JSC::ErrorHandlingScope::~ErrorHandlingScope):
- Previously, we initialize stackPointerAtVMEntry in VMEntryScope. This
means that the stackPointerAtVMEntry may not be initialize when we
instantiate the ErrorHandlingScope. And so, we needed to initialize the
stackPointerAtVMEntry in the ErrorHandlingScope constructor if it's not
already initialized.
Now that we initialize the stackPointerAtVMEntry when we lock the VM JSLock,
we are guaranteed that it will be initialized by the time we instantiate
the ErrorHandlingScope. Hence, we can change the ErrorHandlingScope code
to just assert that the stackPointerAtVMEntry is initialized instead.
* runtime/InitializeThreading.cpp:
(JSC::initializeThreading):
- We no longer need to save the reservedZoneSize. Remove the related code.
* runtime/JSLock.cpp:
(JSC::JSLock::lock):
- When we grab the JSLock mutex for the first time, there is no reason why
the stackPointerAtVMEntry should be initialized. By definition, grabbing
the lock for the first time equates to entering the VM for the first time.
Hence, we can just assert that stackPointerAtVMEntry is uninitialized,
and initialize it unconditionally.
The only exception to this is if we're locking to regrab the JSLock in
grabAllLocks(), but grabAllLocks() will take care of restoring the
stackPointerAtVMEntry in that case after lock() returns. stackPointerAtVMEntry
should still be 0 when we've just locked the JSLock. So, the above assertion
always holds true.
Note: VM::setStackPointerAtVMEntry() will take care of calling
VM::updateStackLimit() based on the new stackPointerAtVMEntry.
- There is no need to save the reservedZoneSize. The reservedZoneSize is
set to Options::reservedZoneSize() when the VM is initialized. Thereafter,
the ErrorHandlingScope will change it to Options::errorModeReservedZoneSize()
when we're handling an error, and it will restore it afterwards. There is
no other reason we should be changing the reservedZoneSize. Hence, we can
remove the unnecessary code to save it here.
(JSC::JSLock::unlock):
- Similarly, when the lockCount reaches 0 in unlock(), it is synonymous with
exiting the VM. Hence, we should just clear the stackPointerAtVMEntry and
update the stackLimit. Exiting the VM should have no effect on the VM
reservedZoneSize. Hence, we can remove the unnecessary code to "restore" it.
(JSC::JSLock::dropAllLocks):
- When dropping locks, we do not need to save the reservedZoneSize because
the reservedZoneSize should remain the same regardless of which thread
we are executing JS on. Hence, we can remove the unnecessary code to save
the reservedZoneSize here.
(JSC::JSLock::grabAllLocks):
- When re-grabbing locks, restoring the stackPointerAtVMEntry via
VM::setStackPointerAtVMEntry() will take care of updating the stack limit.
As explained above, there's no need to save the reservedZoneSize. Hence,
there's no need to "restore" it here.
* runtime/VM.cpp:
(JSC::VM::VM):
(JSC::VM::setStackPointerAtVMEntry):
- Sets the stackPointerAtVMEntry and delegates to updateStackLimit() to update
the stack limit based on the new stackPointerAtVMEntry.
(JSC::VM::updateStackLimit):
* runtime/VM.h:
(JSC::VM::stackPointerAtVMEntry):
- Renamed stackPointerAtVMEntry to m_stackPointerAtVMEntry and made it private.
Added a stackPointerAtVMEntry() function to read the value.
Source/WTF:
* wtf/WTFThreadData.cpp:
(WTF::WTFThreadData::WTFThreadData):
* wtf/WTFThreadData.h:
- removed unnneeded m_savedReservedZoneSize.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (164017 => 164018)
--- trunk/Source/_javascript_Core/ChangeLog 2014-02-13 06:40:35 UTC (rev 164017)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-02-13 06:50:09 UTC (rev 164018)
@@ -1,3 +1,88 @@
+2014-02-12 Mark Lam <[email protected]>
+
+ No need to save reservedZoneSize when dropping the JSLock.
+ <https://webkit.org/b/128719>
+
+ Reviewed by Geoffrey Garen.
+
+ The reservedZoneSize does not change due to the VM being run on a different
+ thread. Hence, there is no need to save and restore its value. Instead of
+ calling updateReservedZoneSize() to update the stack limit, we now call
+ setStackPointerAtVMEntry() to do the job. setStackPointerAtVMEntry()
+ will update the stackPointerAtVMEntry and delegate to updateStackLimit() to
+ update the stack limit based on the new stackPointerAtVMEntry.
+
+ * runtime/ErrorHandlingScope.cpp:
+ (JSC::ErrorHandlingScope::ErrorHandlingScope):
+ (JSC::ErrorHandlingScope::~ErrorHandlingScope):
+ - Previously, we initialize stackPointerAtVMEntry in VMEntryScope. This
+ means that the stackPointerAtVMEntry may not be initialize when we
+ instantiate the ErrorHandlingScope. And so, we needed to initialize the
+ stackPointerAtVMEntry in the ErrorHandlingScope constructor if it's not
+ already initialized.
+
+ Now that we initialize the stackPointerAtVMEntry when we lock the VM JSLock,
+ we are guaranteed that it will be initialized by the time we instantiate
+ the ErrorHandlingScope. Hence, we can change the ErrorHandlingScope code
+ to just assert that the stackPointerAtVMEntry is initialized instead.
+
+ * runtime/InitializeThreading.cpp:
+ (JSC::initializeThreading):
+ - We no longer need to save the reservedZoneSize. Remove the related code.
+
+ * runtime/JSLock.cpp:
+ (JSC::JSLock::lock):
+ - When we grab the JSLock mutex for the first time, there is no reason why
+ the stackPointerAtVMEntry should be initialized. By definition, grabbing
+ the lock for the first time equates to entering the VM for the first time.
+ Hence, we can just assert that stackPointerAtVMEntry is uninitialized,
+ and initialize it unconditionally.
+
+ The only exception to this is if we're locking to regrab the JSLock in
+ grabAllLocks(), but grabAllLocks() will take care of restoring the
+ stackPointerAtVMEntry in that case after lock() returns. stackPointerAtVMEntry
+ should still be 0 when we've just locked the JSLock. So, the above assertion
+ always holds true.
+
+ Note: VM::setStackPointerAtVMEntry() will take care of calling
+ VM::updateStackLimit() based on the new stackPointerAtVMEntry.
+
+ - There is no need to save the reservedZoneSize. The reservedZoneSize is
+ set to Options::reservedZoneSize() when the VM is initialized. Thereafter,
+ the ErrorHandlingScope will change it to Options::errorModeReservedZoneSize()
+ when we're handling an error, and it will restore it afterwards. There is
+ no other reason we should be changing the reservedZoneSize. Hence, we can
+ remove the unnecessary code to save it here.
+
+ (JSC::JSLock::unlock):
+ - Similarly, when the lockCount reaches 0 in unlock(), it is synonymous with
+ exiting the VM. Hence, we should just clear the stackPointerAtVMEntry and
+ update the stackLimit. Exiting the VM should have no effect on the VM
+ reservedZoneSize. Hence, we can remove the unnecessary code to "restore" it.
+
+ (JSC::JSLock::dropAllLocks):
+ - When dropping locks, we do not need to save the reservedZoneSize because
+ the reservedZoneSize should remain the same regardless of which thread
+ we are executing JS on. Hence, we can remove the unnecessary code to save
+ the reservedZoneSize here.
+
+ (JSC::JSLock::grabAllLocks):
+ - When re-grabbing locks, restoring the stackPointerAtVMEntry via
+ VM::setStackPointerAtVMEntry() will take care of updating the stack limit.
+ As explained above, there's no need to save the reservedZoneSize. Hence,
+ there's no need to "restore" it here.
+
+ * runtime/VM.cpp:
+ (JSC::VM::VM):
+ (JSC::VM::setStackPointerAtVMEntry):
+ - Sets the stackPointerAtVMEntry and delegates to updateStackLimit() to update
+ the stack limit based on the new stackPointerAtVMEntry.
+ (JSC::VM::updateStackLimit):
+ * runtime/VM.h:
+ (JSC::VM::stackPointerAtVMEntry):
+ - Renamed stackPointerAtVMEntry to m_stackPointerAtVMEntry and made it private.
+ Added a stackPointerAtVMEntry() function to read the value.
+
2014-02-12 Mark Hahnenberg <[email protected]>
DelayedReleaseScope in MarkedAllocator::tryAllocateHelper is wrong
Modified: trunk/Source/_javascript_Core/runtime/ErrorHandlingScope.cpp (164017 => 164018)
--- trunk/Source/_javascript_Core/runtime/ErrorHandlingScope.cpp 2014-02-13 06:40:35 UTC (rev 164017)
+++ trunk/Source/_javascript_Core/runtime/ErrorHandlingScope.cpp 2014-02-13 06:50:09 UTC (rev 164018)
@@ -35,8 +35,7 @@
ErrorHandlingScope::ErrorHandlingScope(VM& vm)
: m_vm(vm)
{
- if (!m_vm.stackPointerAtVMEntry)
- m_vm.stackPointerAtVMEntry = this;
+ RELEASE_ASSERT(m_vm.stackPointerAtVMEntry());
size_t newReservedZoneSize = Options::errorModeReservedZoneSize();
m_savedReservedZoneSize = m_vm.updateReservedZoneSize(newReservedZoneSize);
#if ENABLE(LLINT_C_LOOP)
@@ -46,8 +45,7 @@
ErrorHandlingScope::~ErrorHandlingScope()
{
- if (m_vm.stackPointerAtVMEntry == this)
- m_vm.stackPointerAtVMEntry = nullptr;
+ RELEASE_ASSERT(m_vm.stackPointerAtVMEntry());
m_vm.updateReservedZoneSize(m_savedReservedZoneSize);
#if ENABLE(LLINT_C_LOOP)
m_vm.interpreter->stack().setReservedZoneSize(m_savedReservedZoneSize);
Modified: trunk/Source/_javascript_Core/runtime/InitializeThreading.cpp (164017 => 164018)
--- trunk/Source/_javascript_Core/runtime/InitializeThreading.cpp 2014-02-13 06:40:35 UTC (rev 164017)
+++ trunk/Source/_javascript_Core/runtime/InitializeThreading.cpp 2014-02-13 06:50:09 UTC (rev 164018)
@@ -73,9 +73,7 @@
DisallowGC::initialize();
#endif
WTFThreadData& threadData = wtfThreadData();
-
threadData.setSavedLastStackTop(threadData.stack().origin());
- threadData.setSavedReservedZoneSize(Options::reservedZoneSize());
});
}
Modified: trunk/Source/_javascript_Core/runtime/JSLock.cpp (164017 => 164018)
--- trunk/Source/_javascript_Core/runtime/JSLock.cpp 2014-02-13 06:40:35 UTC (rev 164017)
+++ trunk/Source/_javascript_Core/runtime/JSLock.cpp 2014-02-13 06:50:09 UTC (rev 164018)
@@ -123,11 +123,9 @@
WTFThreadData& threadData = wtfThreadData();
- if (!m_vm->stackPointerAtVMEntry) {
- void* p = &p;
- m_vm->stackPointerAtVMEntry = p; // A proxy for the current stack pointer.
- threadData.setSavedReservedZoneSize(m_vm->updateReservedZoneSize(Options::reservedZoneSize()));
- }
+ RELEASE_ASSERT(!m_vm->stackPointerAtVMEntry());
+ void* p = &p; // A proxy for the current stack pointer.
+ m_vm->setStackPointerAtVMEntry(p);
m_vm->setLastStackTop(threadData.savedLastStackTop());
}
@@ -145,10 +143,8 @@
m_lockCount -= unlockCount;
if (!m_lockCount) {
- if (m_vm) {
- m_vm->stackPointerAtVMEntry = nullptr;
- m_vm->updateReservedZoneSize(wtfThreadData().savedReservedZoneSize());
- }
+ if (m_vm)
+ m_vm->setStackPointerAtVMEntry(nullptr);
setOwnerThread(0);
m_lock.unlock();
}
@@ -185,9 +181,8 @@
#endif
WTFThreadData& threadData = wtfThreadData();
- threadData.setSavedStackPointerAtVMEntry(m_vm->stackPointerAtVMEntry);
+ threadData.setSavedStackPointerAtVMEntry(m_vm->stackPointerAtVMEntry());
threadData.setSavedLastStackTop(m_vm->lastStackTop());
- threadData.setSavedReservedZoneSize(m_vm->reservedZoneSize());
unsigned droppedLockCount = m_lockCount;
unlock(droppedLockCount);
@@ -216,9 +211,8 @@
--m_lockDropDepth;
WTFThreadData& threadData = wtfThreadData();
- m_vm->stackPointerAtVMEntry = threadData.savedStackPointerAtVMEntry();
+ m_vm->setStackPointerAtVMEntry(threadData.savedStackPointerAtVMEntry());
m_vm->setLastStackTop(threadData.savedLastStackTop());
- m_vm->updateReservedZoneSize(threadData.savedReservedZoneSize());
}
JSLock::DropAllLocks::DropAllLocks(ExecState* exec)
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (164017 => 164018)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2014-02-13 06:40:35 UTC (rev 164017)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2014-02-13 06:50:09 UTC (rev 164018)
@@ -173,7 +173,6 @@
, vmType(vmType)
, clientData(0)
, topCallFrame(CallFrame::noCaller())
- , stackPointerAtVMEntry(0)
, arrayConstructorTable(adoptPtr(new HashTable(JSC::arrayConstructorTable)))
, arrayPrototypeTable(adoptPtr(new HashTable(JSC::arrayPrototypeTable)))
, booleanPrototypeTable(adoptPtr(new HashTable(JSC::booleanPrototypeTable)))
@@ -223,6 +222,7 @@
#if ENABLE(GC_VALIDATION)
, m_initializingObjectClass(0)
#endif
+ , m_stackPointerAtVMEntry(0)
, m_stackLimit(0)
#if ENABLE(LLINT_C_LOOP)
, m_jsStackLimit(0)
@@ -732,6 +732,12 @@
m_exceptionStack = RefCountedArray<StackFrame>();
}
+void VM::setStackPointerAtVMEntry(void* sp)
+{
+ m_stackPointerAtVMEntry = sp;
+ updateStackLimit();
+}
+
size_t VM::updateReservedZoneSize(size_t reservedZoneSize)
{
size_t oldReservedZoneSize = m_reservedZoneSize;
@@ -744,9 +750,9 @@
inline void VM::updateStackLimit()
{
- if (stackPointerAtVMEntry) {
+ if (m_stackPointerAtVMEntry) {
ASSERT(wtfThreadData().stack().isGrowingDownward());
- char* startOfStack = reinterpret_cast<char*>(stackPointerAtVMEntry);
+ char* startOfStack = reinterpret_cast<char*>(m_stackPointerAtVMEntry);
#if ENABLE(FTL_JIT)
m_stackLimit = wtfThreadData().stack().recursionLimit(startOfStack, Options::maxPerThreadStackUsage(), m_reservedZoneSize + m_largestFTLStackSize);
m_ftlStackLimit = wtfThreadData().stack().recursionLimit(startOfStack, Options::maxPerThreadStackUsage(), m_reservedZoneSize + 2 * m_largestFTLStackSize);
Modified: trunk/Source/_javascript_Core/runtime/VM.h (164017 => 164018)
--- trunk/Source/_javascript_Core/runtime/VM.h 2014-02-13 06:40:35 UTC (rev 164017)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2014-02-13 06:50:09 UTC (rev 164018)
@@ -230,7 +230,6 @@
VMType vmType;
ClientData* clientData;
ExecState* topCallFrame;
- void* stackPointerAtVMEntry;
Watchdog watchdog;
const OwnPtr<const HashTable> arrayConstructorTable;
@@ -379,6 +378,9 @@
JS_EXPORT_PRIVATE JSValue throwException(ExecState*, JSValue);
JS_EXPORT_PRIVATE JSObject* throwException(ExecState*, JSObject*);
+ void* stackPointerAtVMEntry() const { return m_stackPointerAtVMEntry; }
+ void setStackPointerAtVMEntry(void*);
+
size_t reservedZoneSize() const { return m_reservedZoneSize; }
size_t updateReservedZoneSize(size_t reservedZoneSize);
@@ -529,6 +531,7 @@
#if ENABLE(GC_VALIDATION)
const ClassInfo* m_initializingObjectClass;
#endif
+ void* m_stackPointerAtVMEntry;
size_t m_reservedZoneSize;
#if ENABLE(LLINT_C_LOOP)
struct {
Modified: trunk/Source/WTF/ChangeLog (164017 => 164018)
--- trunk/Source/WTF/ChangeLog 2014-02-13 06:40:35 UTC (rev 164017)
+++ trunk/Source/WTF/ChangeLog 2014-02-13 06:50:09 UTC (rev 164018)
@@ -1,3 +1,15 @@
+2014-02-12 Mark Lam <[email protected]>
+
+ No need to save reservedZoneSize when dropping the JSLock.
+ <https://webkit.org/b/128719>
+
+ Reviewed by Geoffrey Garen.
+
+ * wtf/WTFThreadData.cpp:
+ (WTF::WTFThreadData::WTFThreadData):
+ * wtf/WTFThreadData.h:
+ - removed unnneeded m_savedReservedZoneSize.
+
2014-02-11 Mark Hahnenberg <[email protected]>
32-bit LLInt writeBarrierOnGlobalObject is wrong
Modified: trunk/Source/WTF/wtf/WTFThreadData.cpp (164017 => 164018)
--- trunk/Source/WTF/wtf/WTFThreadData.cpp 2014-02-13 06:40:35 UTC (rev 164017)
+++ trunk/Source/WTF/wtf/WTFThreadData.cpp 2014-02-13 06:50:09 UTC (rev 164018)
@@ -51,7 +51,6 @@
#endif
, m_savedStackPointerAtVMEntry(0)
, m_savedLastStackTop(stack().origin())
- , m_savedReservedZoneSize(0)
{
#if USE(WEB_THREAD)
static JSC::IdentifierTable* sharedIdentifierTable = new JSC::IdentifierTable();
Modified: trunk/Source/WTF/wtf/WTFThreadData.h (164017 => 164018)
--- trunk/Source/WTF/wtf/WTFThreadData.h 2014-02-13 06:40:35 UTC (rev 164017)
+++ trunk/Source/WTF/wtf/WTFThreadData.h 2014-02-13 06:50:09 UTC (rev 164018)
@@ -126,16 +126,6 @@
m_savedLastStackTop = lastStackTop;
}
- size_t savedReservedZoneSize()
- {
- return m_savedReservedZoneSize;
- }
-
- void setSavedReservedZoneSize(size_t reservedZoneSize)
- {
- m_savedReservedZoneSize = reservedZoneSize;
- }
-
void* m_apiData;
private:
@@ -150,7 +140,6 @@
#endif
void* m_savedStackPointerAtVMEntry;
void* m_savedLastStackTop;
- size_t m_savedReservedZoneSize;
static WTF_EXPORTDATA ThreadSpecific<WTFThreadData>* staticData;
friend WTFThreadData& wtfThreadData();
