Commit 48feba60 authored by Peter Marshall's avatar Peter Marshall Committed by Commit Bot

Reland "[cpu-profiler] Add more logging to find flaky failure"

This is a reland of 138bcfc3

Fixed all the data races and ran TSAN locally to confirm.

Original change's description:
> [cpu-profiler] Add more logging to find flaky failure
>
> There is a flaky 5x failure in the tree which I can't reproduce locally.
> This extra logging will help flush out what the problem is.
>
> Bug: v8:8649
>
> Change-Id: If36d2ce0f4feb398d7d746d69b417bb55a714422
> Reviewed-on: https://chromium-review.googlesource.com/c/1402787
> Commit-Queue: Peter Marshall <petermarshall@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#58796}

Bug: v8:8649
Change-Id: I53e293ef85a54d4b2b39aa3b980832031201aa0c
Reviewed-on: https://chromium-review.googlesource.com/c/1411633
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58833}
parent aaee6958
include_rules = [ include_rules = [
"+include", "+include",
"-src", "-src",
"+src/flags.h",
"+src/base", "+src/base",
"+src/libsampler", "+src/libsampler",
] ]
\ No newline at end of file
...@@ -63,6 +63,8 @@ typedef zx_arm64_general_regs_t zx_thread_state_general_regs_t; ...@@ -63,6 +63,8 @@ typedef zx_arm64_general_regs_t zx_thread_state_general_regs_t;
#include "src/base/atomic-utils.h" #include "src/base/atomic-utils.h"
#include "src/base/hashmap.h" #include "src/base/hashmap.h"
#include "src/base/platform/platform.h" #include "src/base/platform/platform.h"
// TODO(petermarshall): Remove when cpu profiler logging is no longer needed
#include "src/flags.h"
#if V8_OS_ANDROID && !defined(__BIONIC_HAVE_UCONTEXT_T) #if V8_OS_ANDROID && !defined(__BIONIC_HAVE_UCONTEXT_T)
...@@ -265,6 +267,10 @@ class SamplerManager { ...@@ -265,6 +267,10 @@ class SamplerManager {
void RemoveSampler(Sampler* sampler) { void RemoveSampler(Sampler* sampler) {
AtomicGuard atomic_guard(&samplers_access_counter_); AtomicGuard atomic_guard(&samplers_access_counter_);
DCHECK(sampler->IsActive() || sampler->IsRegistered()); DCHECK(sampler->IsActive() || sampler->IsRegistered());
if (i::FLAG_cpu_profiler_logging) {
printf("SamplerManager RemoveSampler: signals: %d\n", signals_);
}
// Remove sampler from map. // Remove sampler from map.
pthread_t thread_id = sampler->platform_data()->vm_tid(); pthread_t thread_id = sampler->platform_data()->vm_tid();
void* thread_key = ThreadKey(thread_id); void* thread_key = ThreadKey(thread_id);
...@@ -289,6 +295,7 @@ class SamplerManager { ...@@ -289,6 +295,7 @@ class SamplerManager {
void DoSample(const v8::RegisterState& state) { void DoSample(const v8::RegisterState& state) {
AtomicGuard atomic_guard(&SamplerManager::samplers_access_counter_, false); AtomicGuard atomic_guard(&SamplerManager::samplers_access_counter_, false);
if (!atomic_guard.is_success()) return; if (!atomic_guard.is_success()) return;
signals_++;
pthread_t thread_id = pthread_self(); pthread_t thread_id = pthread_self();
base::HashMap::Entry* entry = base::HashMap::Entry* entry =
sampler_map_.Lookup(ThreadKey(thread_id), ThreadHash(thread_id)); sampler_map_.Lookup(ThreadKey(thread_id), ThreadHash(thread_id));
...@@ -314,6 +321,7 @@ class SamplerManager { ...@@ -314,6 +321,7 @@ class SamplerManager {
private: private:
base::HashMap sampler_map_; base::HashMap sampler_map_;
static AtomicMutex samplers_access_counter_; static AtomicMutex samplers_access_counter_;
int signals_ = 0;
}; };
AtomicMutex SamplerManager::samplers_access_counter_; AtomicMutex SamplerManager::samplers_access_counter_;
...@@ -435,7 +443,6 @@ int SignalHandler::client_count_ = 0; ...@@ -435,7 +443,6 @@ int SignalHandler::client_count_ = 0;
struct sigaction SignalHandler::old_signal_handler_; struct sigaction SignalHandler::old_signal_handler_;
bool SignalHandler::signal_handler_installed_ = false; bool SignalHandler::signal_handler_installed_ = false;
void SignalHandler::HandleProfilerSignal(int signal, siginfo_t* info, void SignalHandler::HandleProfilerSignal(int signal, siginfo_t* info,
void* context) { void* context) {
USE(info); USE(info);
...@@ -627,6 +634,9 @@ void Sampler::UnregisterIfRegistered() { ...@@ -627,6 +634,9 @@ void Sampler::UnregisterIfRegistered() {
Sampler::~Sampler() { Sampler::~Sampler() {
DCHECK(!IsActive()); DCHECK(!IsActive());
DCHECK(!IsRegistered()); DCHECK(!IsRegistered());
if (i::FLAG_cpu_profiler_logging && samples_ != 0) {
printf("~Sampler: samples = %d\n", samples_);
}
delete data_; delete data_;
} }
...@@ -668,6 +678,7 @@ void Sampler::DecreaseProfilingDepth() { ...@@ -668,6 +678,7 @@ void Sampler::DecreaseProfilingDepth() {
#if defined(USE_SIGNALS) #if defined(USE_SIGNALS)
void Sampler::DoSample() { void Sampler::DoSample() {
samples_++;
if (!SignalHandler::Installed()) return; if (!SignalHandler::Installed()) return;
if (!IsActive() && !IsRegistered()) { if (!IsActive() && !IsRegistered()) {
SamplerManager::instance()->AddSampler(this); SamplerManager::instance()->AddSampler(this);
......
...@@ -100,6 +100,7 @@ class Sampler { ...@@ -100,6 +100,7 @@ class Sampler {
base::Atomic32 active_; base::Atomic32 active_;
base::Atomic32 registered_; base::Atomic32 registered_;
PlatformData* data_; // Platform specific data. PlatformData* data_; // Platform specific data.
int samples_ = 0;
DISALLOW_IMPLICIT_CONSTRUCTORS(Sampler); DISALLOW_IMPLICIT_CONSTRUCTORS(Sampler);
}; };
......
...@@ -28,10 +28,16 @@ class CpuSampler : public sampler::Sampler { ...@@ -28,10 +28,16 @@ class CpuSampler : public sampler::Sampler {
CpuSampler(Isolate* isolate, SamplingEventsProcessor* processor) CpuSampler(Isolate* isolate, SamplingEventsProcessor* processor)
: sampler::Sampler(reinterpret_cast<v8::Isolate*>(isolate)), : sampler::Sampler(reinterpret_cast<v8::Isolate*>(isolate)),
processor_(processor) {} processor_(processor) {}
~CpuSampler() {
if (i::FLAG_cpu_profiler_logging) {
printf("~CpuSampler: samples_: %d\n", samples_);
}
}
void SampleStack(const v8::RegisterState& regs) override { void SampleStack(const v8::RegisterState& regs) override {
TickSample* sample = processor_->StartTickSample(); TickSample* sample = processor_->StartTickSample();
if (sample == nullptr) return; if (sample == nullptr) return;
samples_++;
Isolate* isolate = reinterpret_cast<Isolate*>(this->isolate()); Isolate* isolate = reinterpret_cast<Isolate*>(this->isolate());
sample->Init(isolate, regs, TickSample::kIncludeCEntryFrame, true); sample->Init(isolate, regs, TickSample::kIncludeCEntryFrame, true);
if (is_counting_samples_ && !sample->timestamp.IsNull()) { if (is_counting_samples_ && !sample->timestamp.IsNull()) {
...@@ -43,6 +49,7 @@ class CpuSampler : public sampler::Sampler { ...@@ -43,6 +49,7 @@ class CpuSampler : public sampler::Sampler {
private: private:
SamplingEventsProcessor* processor_; SamplingEventsProcessor* processor_;
int samples_ = 0;
}; };
ProfilerEventsProcessor::ProfilerEventsProcessor(Isolate* isolate, ProfilerEventsProcessor::ProfilerEventsProcessor(Isolate* isolate,
...@@ -407,6 +414,9 @@ void CpuProfiler::StartProcessorIfNotStarted() { ...@@ -407,6 +414,9 @@ void CpuProfiler::StartProcessorIfNotStarted() {
CpuProfile* CpuProfiler::StopProfiling(const char* title) { CpuProfile* CpuProfiler::StopProfiling(const char* title) {
if (!is_profiling_) return nullptr; if (!is_profiling_) return nullptr;
StopProcessorIfLastProfile(title); StopProcessorIfLastProfile(title);
if (i::FLAG_cpu_profiler_logging) {
PrintF("StopProfiling: samples = %d\n", generator_->samples_);
}
return profiles_->StopProfiling(title); return profiles_->StopProfiling(title);
} }
......
...@@ -783,6 +783,7 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) { ...@@ -783,6 +783,7 @@ void ProfileGenerator::RecordTickSample(const TickSample& sample) {
if (FLAG_cpu_profiler_logging) { if (FLAG_cpu_profiler_logging) {
PrintF("RecordTickSample: "); PrintF("RecordTickSample: ");
sample.print(); sample.print();
samples_++;
} }
if (sample.pc != nullptr) { if (sample.pc != nullptr) {
......
...@@ -463,10 +463,16 @@ class CpuProfilesCollection { ...@@ -463,10 +463,16 @@ class CpuProfilesCollection {
class ProfileGenerator { class ProfileGenerator {
public: public:
explicit ProfileGenerator(CpuProfilesCollection* profiles); explicit ProfileGenerator(CpuProfilesCollection* profiles);
~ProfileGenerator() {
if (FLAG_cpu_profiler_logging) {
PrintF("~ProfileGenerator: samples = %d\n", samples_);
}
}
void RecordTickSample(const TickSample& sample); void RecordTickSample(const TickSample& sample);
CodeMap* code_map() { return &code_map_; } CodeMap* code_map() { return &code_map_; }
int samples_ = 0;
private: private:
CodeEntry* FindEntry(Address address); CodeEntry* FindEntry(Address address);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment