Skip to content

Commit

Permalink
use new lock (#1423)
Browse files Browse the repository at this point in the history
Summary: Pull Request resolved: #1423

Differential Revision: D58193617
  • Loading branch information
Matt Blagden authored and facebook-github-bot committed Jun 5, 2024
1 parent 58d9209 commit fd22381
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 9 deletions.
2 changes: 2 additions & 0 deletions include/hermes/VM/Profiler/SamplingProfiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ class SamplingProfiler {
/// domains, and thread associated with the runtime.
std::mutex runtimeDataLock_;

std::mutex threadIdLock_;

protected:
/// Sampled stack traces overtime. Protected by runtimeDataLock_.
std::vector<StackTrace> sampledStacks_;
Expand Down
11 changes: 6 additions & 5 deletions lib/VM/Profiler/SamplingProfilerPosix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ struct SamplingProfilerPosix : SamplingProfiler {

/// Thread that this profiler instance represents. This can be updated as the
/// runtime is invoked on different threads. Must only be accessed while
/// holding the runtimeDataLock_.
/// holding the threadIdLock_.
pthread_t currentThread_;

#if defined(HERMESVM_ENABLE_LOOM) && defined(__ANDROID__)
Expand Down Expand Up @@ -315,9 +315,10 @@ bool Sampler::platformSuspendVMAndWalkStack(SamplingProfiler *profiler) {
// acquired the updates to domains_.
self->profilerForSig_.store(profiler, std::memory_order_release);

// Signal target runtime thread to sample stack. The runtimeDataLock is
// held by the caller, ensuring the runtime won't start to be used on
// Signal target runtime thread to sample stack. The threadIdLock_ is
// held ensuring the runtime won't start to be used on
// another thread before sampling begins.
std::lock_guard<std::mutex> lockGuard(posixProfiler->threadIdLock_);
pthread_kill(posixProfiler->currentThread_, SIGPROF);

// Threading: samplingDoneSem_ will synchronise this thread with the
Expand Down Expand Up @@ -473,13 +474,13 @@ std::unique_ptr<SamplingProfiler> SamplingProfiler::create(Runtime &rt) {

bool SamplingProfiler::belongsToCurrentThread() {
auto profiler = static_cast<sampling_profiler::SamplingProfilerPosix *>(this);
std::lock_guard<std::mutex> lock(profiler->runtimeDataLock_);
std::lock_guard<std::mutex> lock(profiler->threadIdLock_);
return profiler->currentThread_ == pthread_self();
}

void SamplingProfiler::setRuntimeThread() {
auto profiler = static_cast<sampling_profiler::SamplingProfilerPosix *>(this);
std::lock_guard<std::mutex> lock(profiler->runtimeDataLock_);
std::lock_guard<std::mutex> lock(profiler->threadIdLock_);
profiler->currentThread_ = pthread_self();
}

Expand Down
7 changes: 4 additions & 3 deletions lib/VM/Profiler/SamplingProfilerWindows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,10 @@ void Sampler::platformPostSampleStack(SamplingProfiler *localProfiler) {
bool Sampler::platformSuspendVMAndWalkStack(SamplingProfiler *profiler) {
auto *winProfiler = static_cast<SamplingProfilerWindows *>(profiler);

// Suspend the JS thread. The runtimeDataLock is held by the caller, ensuring
// Suspend the JS thread. The threadIdLock_ is held by the caller, ensuring
// the runtime won't start to be used on another thread before sampling
// begins.
std::lock_guard<std::mutex> lockGuard(winProfiler->threadIdLock_);
DWORD prevSuspendCount = SuspendThread(winProfiler->currentThread_);
if (prevSuspendCount == static_cast<DWORD>(-1)) {
return true;
Expand Down Expand Up @@ -212,14 +213,14 @@ std::unique_ptr<SamplingProfiler> SamplingProfiler::create(Runtime &rt) {
bool SamplingProfiler::belongsToCurrentThread() {
auto profiler =
static_cast<sampling_profiler::SamplingProfilerWindows *>(this);
std::lock_guard<std::mutex> lock(profiler->runtimeDataLock_);
std::lock_guard<std::mutex> lock(profiler->threadIdLock_);
return GetThreadId(profiler->currentThread_) == GetCurrentThreadId();
}

void SamplingProfiler::setRuntimeThread() {
auto profiler =
static_cast<sampling_profiler::SamplingProfilerWindows *>(this);
std::lock_guard<std::mutex> lock(profiler->runtimeDataLock_);
std::lock_guard<std::mutex> lock(profiler->threadIdLock_);
CloseHandle(profiler->currentThread_);
profiler->currentThread_ = openCurrentThread();
}
Expand Down
2 changes: 1 addition & 1 deletion unittests/VMRuntime/SamplingProfilerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ TEST(SamplingProfilerTest, MultipleProfilers) {
#endif

TEST(SamplingProfilerTest, MultipleThreads) {
constexpr uint32_t kThreadCount = 3;
constexpr uint32_t kThreadCount = 10;

auto rt = makeRuntime(withSamplingProfilerEnabled);
rt->samplingProfiler->enable();
Expand Down

0 comments on commit fd22381

Please sign in to comment.