Skip to content

use atomic load/store on curContext_ #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

use atomic load/store on curContext_ #187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 17 additions & 20 deletions bindings/profilers/wall.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ using namespace v8;

namespace dd {

using ContextPtr = std::shared_ptr<Global<Value>>;

// Maximum number of rounds in the GetV8ToEpochOffset
static constexpr int MAX_EPOCH_OFFSET_ATTEMPTS = 20;

Expand Down Expand Up @@ -492,7 +494,6 @@ WallProfiler::WallProfiler(std::chrono::microseconds samplingPeriod,
contexts_.reserve(duration * 2 / samplingPeriod);
}

curContext_.store(&context1_, std::memory_order_relaxed);
collectionMode_.store(CollectionMode::kNoCollect, std::memory_order_relaxed);

auto isolate = v8::Isolate::GetCurrent();
Expand Down Expand Up @@ -952,28 +953,26 @@ v8::CpuProfiler* WallProfiler::CreateV8CpuProfiler() {
}

v8::Local<v8::Value> WallProfiler::GetContext(Isolate* isolate) {
auto context = *curContext_.load(std::memory_order_relaxed);
auto context = GetContextPtr();
if (!context) return v8::Undefined(isolate);
return context->Get(isolate);
}

void WallProfiler::SetContext(Isolate* isolate, Local<Value> value) {
// Need to be careful here, because we might be interrupted by a
// signal handler that will make use of curContext_.
// Update of shared_ptr is not atomic, so instead we use a pointer
// (curContext_) that points on two shared_ptr (context1_ and context2_),
// update the shared_ptr that is not currently in use and then atomically
// update curContext_.
auto newCurContext = curContext_.load(std::memory_order_relaxed) == &context1_
? &context2_
: &context1_;
if (!value->IsNullOrUndefined()) {
*newCurContext = std::make_shared<Global<Value>>(isolate, value);
} else {
newCurContext->reset();
}
std::atomic_signal_fence(std::memory_order_release);
curContext_.store(newCurContext, std::memory_order_relaxed);
std::atomic_store_explicit(
&curContext_,
value->IsNullOrUndefined()
? std::shared_ptr<Global<Value>>()
: std::make_shared<Global<Value>>(isolate, value),
std::memory_order_relaxed);
}

ContextPtr WallProfiler::GetContextPtr() {
auto contextPtr =
atomic_load_explicit(&curContext_, std::memory_order_relaxed);
std::atomic_signal_fence(std::memory_order_acquire);
return contextPtr;
}

NAN_GETTER(WallProfiler::GetContext) {
Expand Down Expand Up @@ -1007,10 +1006,8 @@ void WallProfiler::PushContext(int64_t time_from,
// Be careful this is called in a signal handler context therefore all
// operations must be async signal safe (in particular no allocations).
// Our ring buffer avoids allocations.
auto context = curContext_.load(std::memory_order_relaxed);
std::atomic_signal_fence(std::memory_order_acquire);
if (contexts_.size() < contexts_.capacity()) {
contexts_.push_back({*context, time_from, time_to, cpu_time});
contexts_.push_back({GetContextPtr(), time_from, time_to, cpu_time});
std::atomic_fetch_add_explicit(
reinterpret_cast<std::atomic<uint32_t>*>(&fields_[kSampleCount]),
1U,
Expand Down
7 changes: 2 additions & 5 deletions bindings/profilers/wall.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,7 @@ class WallProfiler : public Nan::ObjectWrap {
// avoid heap allocation. Need to figure out the right move/copy semantics in
// and out of the ring buffer.

// We're using a pair of shared pointers and an atomic pointer-to-current as
// a way to ensure signal safety on update.
ContextPtr context1_;
ContextPtr context2_;
std::atomic<ContextPtr*> curContext_;
ContextPtr curContext_;

std::atomic<CollectionMode> collectionMode_;
std::atomic<uint64_t> noCollectCallCount_;
Expand Down Expand Up @@ -99,6 +95,7 @@ class WallProfiler : public Nan::ObjectWrap {
int64_t startCpuTime);

bool waitForSignal(uint64_t targetCallCount = 0);
ContextPtr GetContextPtr();

public:
/**
Expand Down
Loading