Skip to content

Commit

Permalink
[tools] Add a VMState for Atomics.wait
Browse files Browse the repository at this point in the history
We will use this state in devtools via the inspector to indicate
whether a thread is currently stuck polling in atomics.wait.

VMState already distinguishes the important states we care about which
are idle vs. running JS. We also want to know the state for
atomics.wait(), which is commonly used in WebWorkers to poll the main
page for work to do.

This CL just adds and maintains the state and adds assertions in
atomics tests. Another CL will emit inspector notifications when the
VMState changes in a way that the inspector cares about.

Re-flow comments as a drive-by cleanup.

Bug: chromium:1025490
Change-Id: I961051bfb846aa20454a56214310370ea8e47d1c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2033168
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66071}
  • Loading branch information
psmarshall authored and Commit Bot committed Jan 31, 2020
1 parent a6cf73e commit e8ba569
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 10 deletions.
1 change: 1 addition & 0 deletions include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -2199,6 +2199,7 @@ enum StateTag {
COMPILER,
OTHER,
EXTERNAL,
ATOMICS_WAIT,
IDLE
};

Expand Down
2 changes: 2 additions & 0 deletions src/execution/futex-emulation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "src/base/macros.h"
#include "src/base/platform/time.h"
#include "src/execution/isolate.h"
#include "src/execution/vm-state-inl.h"
#include "src/handles/handles-inl.h"
#include "src/numbers/conversions.h"
#include "src/objects/bigint.h"
Expand Down Expand Up @@ -132,6 +133,7 @@ template <typename T>
Object FutexEmulation::Wait(Isolate* isolate,
Handle<JSArrayBuffer> array_buffer, size_t addr,
T value, double rel_timeout_ms) {
VMState<ATOMICS_WAIT> state(isolate);
DCHECK_LT(addr, array_buffer->byte_length());

bool use_timeout = rel_timeout_ms != V8_INFINITY;
Expand Down
10 changes: 5 additions & 5 deletions src/execution/vm-state-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,9 @@
namespace v8 {
namespace internal {

//
// VMState class implementation. A simple stack of VM states held by the
// logger and partially threaded through the call stack. States are pushed by
// VMState construction and popped by destruction.
//
// VMState class implementation. A simple stack of VM states held by the logger
// and partially threaded through the call stack. States are pushed by VMState
// construction and popped by destruction.
inline const char* StateToString(StateTag state) {
switch (state) {
case JS:
Expand All @@ -35,6 +33,8 @@ inline const char* StateToString(StateTag state) {
return "OTHER";
case EXTERNAL:
return "EXTERNAL";
case ATOMICS_WAIT:
return "ATOMICS_WAIT";
case IDLE:
return "IDLE";
}
Expand Down
9 changes: 4 additions & 5 deletions src/execution/vm-state.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@
namespace v8 {
namespace internal {

// Logging and profiling. A StateTag represents a possible state of
// the VM. The logger maintains a stack of these. Creating a VMState
// object enters a state by pushing on the stack, and destroying a
// VMState object leaves a state by popping the current state from the
// stack.
// Logging and profiling. A StateTag represents a possible state of the VM. The
// logger maintains a stack of these. Creating a VMState object enters a state
// by pushing on the stack, and destroying a VMState object leaves a state by
// popping the current state from the stack.
template <StateTag Tag>
class VMState {
public:
Expand Down
1 change: 1 addition & 0 deletions src/profiler/profile-generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,7 @@ CodeEntry* ProfileGenerator::EntryForVMState(StateTag tag) {
case PARSER:
case COMPILER:
case BYTECODE_COMPILER:
case ATOMICS_WAIT:
// DOM events handlers are reported as OTHER / EXTERNAL entries.
// To avoid confusing people, let's put all these entries into
// one bucket.
Expand Down
3 changes: 3 additions & 0 deletions src/profiler/sampling-heap-profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ SamplingHeapProfiler::AllocationNode* SamplingHeapProfiler::AddStack() {
case IDLE:
name = "(IDLE)";
break;
// Treat atomics wait as a normal JS event; we don't care about the
// difference for allocations.
case ATOMICS_WAIT:
case JS:
name = "(JS)";
break;
Expand Down
2 changes: 2 additions & 0 deletions test/cctest/test-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25979,6 +25979,8 @@ void AtomicsWaitCallbackForTesting(
CHECK_EQ(timeout_in_ms, info->expected_timeout);
CHECK_EQ(value, info->expected_value);
CHECK_EQ(offset_in_bytes, info->expected_offset);
CHECK_EQ(v8::StateTag::ATOMICS_WAIT,
reinterpret_cast<i::Isolate*>(info->isolate)->current_vm_state());

auto ThrowSomething = [&]() {
info->isolate->ThrowException(v8::Integer::New(info->isolate, 42));
Expand Down

0 comments on commit e8ba569

Please sign in to comment.