Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(726)

Unified Diff: src/profiler/sampler.cc

Issue 1858143003: Get rid of UnsafeCurrent in Sampler (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/profiler/sampler.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/profiler/sampler.cc
diff --git a/src/profiler/sampler.cc b/src/profiler/sampler.cc
index a34042453c902d25a2ced72494a47fe4896e9263..2ff69a4e69942f939988a3e8aeabe322c42d4f2a 100644
--- a/src/profiler/sampler.cc
+++ b/src/profiler/sampler.cc
@@ -12,6 +12,7 @@
#include <pthread.h>
#include <signal.h>
#include <sys/time.h>
+#include <atomic>
#if !V8_OS_QNX && !V8_OS_NACL && !V8_OS_AIX
#include <sys/syscall.h> // NOLINT
@@ -236,6 +237,58 @@ bool IsNoFrameRegion(Address address) {
return false;
}
+typedef List<Sampler*> SamplerList;
+#if defined(USE_SIGNALS)
+class ThreadSamplersPair {
+ public:
+ ThreadSamplersPair(pthread_t thread_id, SamplerList* samplers)
+ : thread_id_(thread_id), samplers_(samplers) {}
+ ~ThreadSamplersPair() {
+ delete samplers_;
+ }
+ pthread_t thread_id() const { return thread_id_; }
+ SamplerList* samplers() const { return samplers_; }
+ bool operator==(const ThreadSamplersPair& rhs) {
+ return thread_id_ == rhs.thread_id();
+ }
+ private:
+ pthread_t thread_id_;
+ SamplerList* samplers_;
+ ThreadSamplersPair() {}
+};
+typedef List<ThreadSamplersPair*> ThreadSamplersList;
+#endif
+
+template <typename T>
+class AtomicGuard {
+ public:
+ explicit AtomicGuard(std::atomic<T>* atomic, T expected, T desired,
+ bool is_block)
fmeawad 2016/04/05 21:25:52 Try to the the v8 base version at https://code.goo
+ : is_success_(false) {
+ do {
+ is_success_ = std::atomic_compare_exchange_strong(atomic,
+ &expected, desired);
+ if (is_success_) break;
+ } while (is_block);
+ atomic_ = atomic;
+ expected_ = expected;
+ desired_ = desired;
+ }
+
+ bool is_success() { return is_success_; }
+
+ ~AtomicGuard() {
+ std::atomic_store(atomic_, expected_);
+ atomic_ = NULL;
+ }
+
+ private:
+ std::atomic<T>* atomic_;
+ T expected_;
+ T desired_;
+ bool is_success_;
+};
+
} // namespace
#if defined(USE_SIGNALS)
@@ -374,6 +427,10 @@ class SignalHandler : public AllStatic {
return signal_handler_installed_;
}
+#if !V8_OS_NACL
+ static void CollectSample(void* context, Sampler* sampler);
+#endif
+
private:
static void Install() {
#if !V8_OS_NACL
@@ -418,23 +475,19 @@ bool SignalHandler::signal_handler_installed_ = false;
// As Native Client does not support signal handling, profiling is disabled.
#if !V8_OS_NACL
-void SignalHandler::HandleProfilerSignal(int signal, siginfo_t* info,
- void* context) {
- USE(info);
- if (signal != SIGPROF) return;
- Isolate* isolate = Isolate::UnsafeCurrent();
- if (isolate == NULL || !isolate->IsInUse()) {
- // We require a fully initialized and entered isolate.
+void SignalHandler::CollectSample(void* context, Sampler* sampler) {
+ if (sampler == NULL || (!sampler->IsProfiling() && !sampler->IsRegistered()))
return;
- }
+ Isolate* isolate = sampler->isolate();
+
+ // We require a fully initialized and entered isolate.
+ if (isolate == NULL || !isolate->IsInUse()) return;
+
if (v8::Locker::IsActive() &&
!isolate->thread_manager()->IsLockedByCurrentThread()) {
return;
}
- Sampler* sampler = isolate->logger()->sampler();
- if (sampler == NULL) return;
-
v8::RegisterState state;
#if defined(USE_SIMULATOR)
@@ -607,19 +660,52 @@ class SamplerThread : public base::Thread {
}
DCHECK(sampler->IsActive());
- DCHECK(!instance_->active_samplers_.Contains(sampler));
DCHECK(instance_->interval_ == sampler->interval());
+
+#if defined(USE_SIGNALS)
+ AddSampler(sampler);
+#else
+ DCHECK(!instance_->active_samplers_.Contains(sampler));
instance_->active_samplers_.Add(sampler);
+#endif // USE_SIGNALS
if (need_to_start) instance_->StartSynchronously();
}
- static void RemoveActiveSampler(Sampler* sampler) {
+ static void RemoveSampler(Sampler* sampler) {
SamplerThread* instance_to_remove = NULL;
{
base::LockGuard<base::Mutex> lock_guard(mutex_);
- DCHECK(sampler->IsActive());
+ DCHECK(sampler->IsActive() || sampler->IsRegistered());
+#if defined(USE_SIGNALS)
+ {
+ AtomicGuard<int> atomic_guard(&sampler_list_access_counter_,
+ 0, 1, true);
+ // Remove sampler from map.
+ pthread_t thread_id = sampler->platform_data()->vm_tid();
+ SamplerList* samplers = NULL;
+ int i = 0;
+ for (; i < thread_id_to_samplers_.length(); ++i) {
+ ThreadSamplersPair* tsp = thread_id_to_samplers_.at(i);
+ if (pthread_equal(tsp->thread_id(), thread_id) != 0) {
+ samplers = tsp->samplers();
+ break;
+ }
+ }
+ if (samplers != NULL) {
+ samplers->RemoveElement(sampler);
+ if (samplers->is_empty()) {
+ ThreadSamplersPair* tsp = thread_id_to_samplers_.Remove(i);
+ delete tsp;
+ }
+ }
+ if (thread_id_to_samplers_.is_empty()) {
+ instance_to_remove = instance_;
+ instance_ = NULL;
+ }
+ }
+#else
bool removed = instance_->active_samplers_.RemoveElement(sampler);
DCHECK(removed);
USE(removed);
@@ -630,6 +716,7 @@ class SamplerThread : public base::Thread {
instance_to_remove = instance_;
instance_ = NULL;
}
+#endif // USE_SIGNALS
}
if (!instance_to_remove) return;
@@ -637,11 +724,29 @@ class SamplerThread : public base::Thread {
delete instance_to_remove;
}
+ // Unlike AddActiveSampler, this method only adds a sampler,
+ // but won't start the sampler thread.
+ static void RegisterSampler(Sampler* sampler) {
+ base::LockGuard<base::Mutex> lock_guard(mutex_);
+#if defined(USE_SIGNALS)
+ AddSampler(sampler);
+#endif // USE_SIGNALS
+ }
+
// Implement Thread::Run().
virtual void Run() {
while (true) {
{
base::LockGuard<base::Mutex> lock_guard(mutex_);
+#if defined(USE_SIGNALS)
+ if (thread_id_to_samplers_.is_empty()) break;
+ if (SignalHandler::Installed()) {
+ for (int i = 0; i < thread_id_to_samplers_.length(); ++i) {
+ pthread_t thread_id = thread_id_to_samplers_.at(i)->thread_id();
+ pthread_kill(thread_id, SIGPROF);
+ }
+ }
+#else
if (active_samplers_.is_empty()) break;
// When CPU profiling is enabled both JavaScript and C++ code is
// profiled. We must not suspend.
@@ -650,6 +755,7 @@ class SamplerThread : public base::Thread {
if (!sampler->IsProfiling()) continue;
sampler->DoSample();
}
+#endif // USE_SIGNALS
}
base::OS::Sleep(base::TimeDelta::FromMilliseconds(interval_));
}
@@ -661,7 +767,36 @@ class SamplerThread : public base::Thread {
static SamplerThread* instance_;
const int interval_;
- List<Sampler*> active_samplers_;
+
+#if defined(USE_SIGNALS)
+ friend class SignalHandler;
+ static ThreadSamplersList thread_id_to_samplers_;
+ static std::atomic<int> sampler_list_access_counter_;
+ static void AddSampler(Sampler* sampler) {
+ AtomicGuard<int> atomic_guard(&SamplerThread::sampler_list_access_counter_,
+ 0, 1, true);
+ // Add sampler into map if needed.
+ pthread_t thread_id = sampler->platform_data()->vm_tid();
+ SamplerList* samplers = NULL;
+ for (int i = 0; i < thread_id_to_samplers_.length(); ++i) {
+ ThreadSamplersPair* tsp = thread_id_to_samplers_.at(i);
+ if (pthread_equal(tsp->thread_id(), thread_id) != 0) {
+ samplers = tsp->samplers();
+ break;
+ }
+ }
+ if (samplers != NULL) {
+ if (!samplers->Contains(sampler))
+ samplers->Add(sampler);
+ } else {
+ samplers = new SamplerList();
+ samplers->Add(sampler);
+ thread_id_to_samplers_.Add(new ThreadSamplersPair(thread_id, samplers));
+ }
+ }
+#else
+ SamplerList active_samplers_;
+#endif // USE_SIGNALS
DISALLOW_COPY_AND_ASSIGN(SamplerThread);
};
@@ -669,6 +804,38 @@ class SamplerThread : public base::Thread {
base::Mutex* SamplerThread::mutex_ = NULL;
SamplerThread* SamplerThread::instance_ = NULL;
+#if defined(USE_SIGNALS)
+ThreadSamplersList SamplerThread::thread_id_to_samplers_;
+std::atomic<int> SamplerThread::sampler_list_access_counter_(0);
+#endif // USE_SIGNALS
+
+// As Native Client does not support signal handling, profiling is disabled.
+#if defined(USE_SIGNALS)
+#if !V8_OS_NACL
+void SignalHandler::HandleProfilerSignal(int signal, siginfo_t* info,
+ void* context) {
+ USE(info);
+ if (signal != SIGPROF) return;
+ AtomicGuard<int> atomic_guard(&SamplerThread::sampler_list_access_counter_,
+ 0, 1, false);
+ if (!atomic_guard.is_success()) return;
+ pthread_t thread_id = pthread_self();
+ SamplerList* samplers = NULL;
+ for (int i = 0; i < SamplerThread::thread_id_to_samplers_.length(); ++i) {
+ ThreadSamplersPair* tsp = SamplerThread::thread_id_to_samplers_.at(i);
+ if (pthread_equal(tsp->thread_id(), thread_id) != 0) {
+ samplers = tsp->samplers();
+ break;
+ }
+ }
+ DCHECK(samplers != NULL);
+ for (int i = 0; i < samplers->length(); ++i) {
+ Sampler* sampler = samplers->at(i);
+ SignalHandler::CollectSample(context, sampler);
+ }
+}
+#endif // !V8_OS_NACL
+#endif // USE_SIGNALs
//
@@ -789,6 +956,7 @@ Sampler::Sampler(Isolate* isolate, int interval)
profiling_(false),
has_processing_thread_(false),
active_(false),
+ registered_(false),
is_counting_samples_(false),
js_sample_count_(0),
external_sample_count_(0) {
@@ -797,6 +965,8 @@ Sampler::Sampler(Isolate* isolate, int interval)
Sampler::~Sampler() {
DCHECK(!IsActive());
+ if (IsRegistered())
+ SamplerThread::RemoveSampler(this);
delete data_;
}
@@ -809,8 +979,9 @@ void Sampler::Start() {
void Sampler::Stop() {
DCHECK(IsActive());
- SamplerThread::RemoveActiveSampler(this);
+ SamplerThread::RemoveSampler(this);
SetActive(false);
+ SetRegistered(false);
}
@@ -850,6 +1021,10 @@ void Sampler::SampleStack(const v8::RegisterState& state) {
void Sampler::DoSample() {
if (!SignalHandler::Installed()) return;
+ if (!IsActive() && !IsRegistered()) {
+ SamplerThread::RegisterSampler(this);
+ SetRegistered(true);
+ }
pthread_kill(platform_data()->vm_tid(), SIGPROF);
}
« no previous file with comments | « src/profiler/sampler.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698