Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 874696af9d4376277356d27dfd2b7637244ef405
https://github.com/WebKit/WebKit/commit/874696af9d4376277356d27dfd2b7637244ef405
Author: Mark Lam <mark....@apple.com>
Date: 2025-07-30 (Wed, 30 Jul 2025)
Changed paths:
M Source/JavaScriptCore/interpreter/FrameTracers.h
M Source/JavaScriptCore/interpreter/Interpreter.cpp
M Source/JavaScriptCore/runtime/VM.cpp
M Source/JavaScriptCore/runtime/VM.h
M Source/JavaScriptCore/runtime/VMTraps.cpp
M Source/JavaScriptCore/runtime/VMTraps.h
M Source/JavaScriptCore/runtime/VMTrapsInlines.h
Log Message:
-----------
Misc VMTraps improvements.
https://bugs.webkit.org/show_bug.cgi?id=296224
rdar://156193538
Reviewed by Yijia Huang.
1. There are clearTrapBit(), setTrapBit(), and fireTrap() methods. fireTrap()
is conceptually like
setTrapBit(), except that it can be called asynchronously from other
threads. clearTrapBit()
and setTrapBit() are only meant to be called from the mutator. It is not
clear from the names
that this is the nature of their roles, and it's easy for client code to use
the wrong method.
We can improve this by changing them into a more intuitive symmetrical pair
of functions:
clearTrap() and fireTrap(). We can also make them more robust and
thread-safe, while still
keeping them performant. More on that below.
2. Drop "Bit" from the name because it communicates an implementation detail
and not the purpose
of the method. This is an unnecessary, and can be distracting.
3. The portion of fireTraps() that need to be thread-safe is the part which
actually requests
thread stopping. We can factor this part out into a
requestThreadStopIfNeeded() method.
fireTrap() will only need to call requestThreadStopIfNeeded() if the trap
event is an async event.
For non-async events, we can skip this. In all cases where fireTrap() needs
to be fast, the trap
event is constant and non-async. Hence, the isAsyncEvent() check can be
constant folded, and the
call to requestThreadStopIfNeeded() will be completely elided.
Also introduce a cancelThreadStopIfNeeded() method to keep the symmetry, and
the logic easier to
understand. Currently, cancelThreadStopIfNeeded() doesn't really do
anything that interesting.
However, in the future, when we add other thread stopping mechanisms, their
hooks or
implementations can go into these 2 methods to request and cancel stoppage.
Similarly, in all cases where clearTrap() needs to be fast, the trap event
is also constant and
non-async. Hence, the analogous call to cancelThreadStopIfNeeded() will be
elided,
4. takeTopPriorityTrap() is only called from handleTraps(). So, move it in
there as a lambda to
communicate this fact. takeTopPriorityTrap() will clear bits in
VMTraps::m_trapBits without
calling cancelThreadStopIfNeeded().
5. Add a call to cancelThreadStopIfNeeded() on exit from handleTraps(). This
ensures that we can
cancel thread stoppage if takeTopPriorityTrap() has cleared every event bit
for async events i.e.
if there are no more async events to service.
6. Also fix a typo in the VMTraps:: m_needToInvalidateCodeBlocks field name.
7. Remove unnecessary VMTraps checks in Interpreter::executeProgram(),
Interpreter::executeCallImpl,
Interpreter::executeConstruct(), Interpreter::executeEval(), and
Interpreter::executeModuleProgram().
These checks are for NonDebuggerAsyncEvents, which rarely ever manifest.
There's no urgency to
check for these traps here.
8. Simplify the implementation of DeterTraps to just set and clear a
VMTraps::m_trapsDeferred flag.
VMTraps::handleTraps() returns early if m_trapsDeferred is set. For
performance reasons, the
previous approach aims to avoid triggering calls into VMTraps::handleTraps()
when a DeferTraps
scope is in effect. However, this is misguided. VMTraps are extremely
rare. Hence, it will
not hurt performance with any significance to just allow
VMTraps::handleTraps() to be called,
and have it return early. Again, this will only happen under the rare
circumstance when we
both are in a DeferTraps scope and have an VMTraps fired at the same time.
This new implementation removes the DeferTrapHandling bit, and makes it
possible to enter and
exit a DeferTraps scope without having to carefully manipulate
VMTraps::m_trapBits. Hence,
this keeps the implementation of fireTraps() and clearTraps() simpler
without needing to deal
with the complications of deferring and restoring traps.
* Source/JavaScriptCore/interpreter/FrameTracers.h:
(JSC::SuspendExceptionScope::SuspendExceptionScope):
(JSC::SuspendExceptionScope::~SuspendExceptionScope):
* Source/JavaScriptCore/interpreter/Interpreter.cpp:
(JSC::Interpreter::executeProgram):
(JSC::Interpreter::executeCallImpl):
(JSC::Interpreter::executeConstruct):
(JSC::Interpreter::executeEval):
(JSC::Interpreter::executeModuleProgram):
* Source/JavaScriptCore/runtime/VM.cpp:
(JSC::VM::setException):
* Source/JavaScriptCore/runtime/VM.h:
(JSC::VM::clearException):
* Source/JavaScriptCore/runtime/VMTraps.cpp:
(JSC::VMTraps::invalidateCodeBlocksOnStack):
(JSC::VMTraps::cancelThreadStopIfNeeded):
(JSC::VMTraps::requestThreadStopIfNeeded):
(JSC::VMTraps::handleTraps):
(JSC::VMTraps::deferTerminationSlow):
(JSC::VMTraps::undoDeferTerminationSlow):
(JSC::VMTraps::fireTrap): Deleted.
(JSC::VMTraps::takeTopPriorityTrap): Deleted.
* Source/JavaScriptCore/runtime/VMTraps.h:
(JSC::VMTraps::isAsyncEvent):
(JSC::VMTraps::needHandling const):
(JSC::VMTraps::clearTrap):
(JSC::VMTraps::fireTrap):
(JSC::VMTraps::clearTrapWithoutCancellingThreadStop):
(JSC::VMTraps::clearTrapBit): Deleted.
(JSC::VMTraps::setTrapBit): Deleted.
* Source/JavaScriptCore/runtime/VMTrapsInlines.h:
(JSC::VMTraps::deferTermination):
(JSC::DeferTraps::DeferTraps):
(JSC::DeferTraps::~DeferTraps):
Canonical link: https://commits.webkit.org/298039@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
