From 98234525b4aab16459b10ef51c5fe8ffbcf83861 Mon Sep 17 00:00:00 2001 From: dthomson Date: Wed, 15 Jan 2020 13:55:07 -0800 Subject: [PATCH 01/11] Add library mapping information to profiles. PiperOrigin-RevId: 289930597 --- third_party/javaprofiler/profile_proto_builder.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index 00774a503..1138ce587 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -91,12 +91,18 @@ void ProfileProtoBuilder::UnsampleMetrics() { unique_ptr ProfileProtoBuilder::CreateSampledProto() { +#ifndef STANDALONE_BUILD + builder_.AddCurrentMappings(); +#endif builder_.Finalize(); return unique_ptr(builder_.Consume()); } unique_ptr ProfileProtoBuilder::CreateUnsampledProto() { +#ifndef STANDALONE_BUILD + builder_.AddCurrentMappings(); +#endif UnsampleMetrics(); builder_.Finalize(); return unique_ptr(builder_.Consume()); From 44b9bccf70aef30e99aad293317497c6c1070fec Mon Sep 17 00:00:00 2001 From: dthomson Date: Thu, 23 Jan 2020 13:13:25 -0800 Subject: [PATCH 02/11] Clean shutdown of the thread. PiperOrigin-RevId: 291229124 --- third_party/javaprofiler/heap_sampler.cc | 19 +++++++++++-------- third_party/javaprofiler/heap_sampler.h | 21 ++++++++++++++++----- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index efb499bde..4be70d837 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -310,7 +310,7 @@ void HeapMonitor::Disable() { jvmti_.store(nullptr); // Notify the agent thread that we are done. - google::javaprofiler::HeapMonitor::GetInstance()->NotifyGCWaitingThread(); + google::javaprofiler::HeapMonitor::GetInstance()->ShutdownGCWaitingThread(); #else // Do nothing: we never enabled ourselves. @@ -349,18 +349,21 @@ std::unique_ptr HeapMonitor::EmptyHeapProfile( ->CreateProto(); } -void HeapMonitor::NotifyGCWaitingThreadInternal() { +void HeapMonitor::NotifyGCWaitingThreadInternal(GcEvent event) { std::unique_lock lock(gc_waiting_mutex_); - gc_notified_ = true; + gc_notify_events_.push_back(event); gc_waiting_cv_.notify_all(); } -void HeapMonitor::WaitForGC() { +HeapMonitor::GcEvent HeapMonitor::WaitForGC() { std::unique_lock lock(gc_waiting_mutex_); - gc_notified_ = false; // If we are woken up without having been notified, just go back to sleep. - gc_waiting_cv_.wait(lock, [this] { return gc_notified_; } ); + gc_waiting_cv_.wait(lock, [this] { return !gc_notify_events_.empty(); } ); + + GcEvent result = gc_notify_events_.front(); + gc_notify_events_.pop_front(); + return result; } void HeapMonitor::GCWaitingThread(jvmtiEnv* jvmti_env, JNIEnv* jni_env, @@ -370,10 +373,10 @@ void HeapMonitor::GCWaitingThread(jvmtiEnv* jvmti_env, JNIEnv* jni_env, void HeapMonitor::GCWaitingThreadRun(JNIEnv* jni_env) { while (true) { - WaitForGC(); + GcEvent event = WaitForGC(); // Was the heap monitor disabled? - if (!Enabled()) { + if (event == GcEvent::SHUTDOWN) { break; } diff --git a/third_party/javaprofiler/heap_sampler.h b/third_party/javaprofiler/heap_sampler.h index b1b757530..c65220135 100644 --- a/third_party/javaprofiler/heap_sampler.h +++ b/third_party/javaprofiler/heap_sampler.h @@ -21,6 +21,7 @@ #include #include // NOLINT +#include #include #include // NOLINT #include @@ -169,7 +170,11 @@ class HeapMonitor { static void AddCallback(jvmtiEventCallbacks *callbacks); static void NotifyGCWaitingThread() { - GetInstance()->NotifyGCWaitingThreadInternal(); + GetInstance()->NotifyGCWaitingThreadInternal(GcEvent::GC_FINISHED); + } + + static void ShutdownGCWaitingThread() { + GetInstance()->NotifyGCWaitingThreadInternal(GcEvent::SHUTDOWN); } // Not copyable or movable. @@ -177,7 +182,7 @@ class HeapMonitor { HeapMonitor &operator=(const HeapMonitor &) = delete; private: - HeapMonitor() : gc_notified_(false), storage_(jvmti_.load()) {} + HeapMonitor() : storage_(jvmti_.load()) {} // We construct the heap_monitor at the first call to GetInstance, so ensure // Enable was called at least once before to initialize jvmti_. @@ -188,11 +193,17 @@ class HeapMonitor { static bool Supported(jvmtiEnv* jvmti); + enum class GcEvent { + NO_EVENT, + GC_FINISHED, + SHUTDOWN + }; + bool CreateGCWaitingThread(jvmtiEnv* jvmti, JNIEnv* jni); static void GCWaitingThread(jvmtiEnv *jvmti_env, JNIEnv *jni_env, void *arg); void GCWaitingThreadRun(JNIEnv* jni_env); - void WaitForGC(); - void NotifyGCWaitingThreadInternal(); + GcEvent WaitForGC(); + void NotifyGCWaitingThreadInternal(GcEvent event); void CompactData(JNIEnv* jni_env); @@ -202,7 +213,7 @@ class HeapMonitor { static std::atomic jvmti_; static std::atomic sampling_interval_; - bool gc_notified_; + std::list gc_notify_events_; std::condition_variable gc_waiting_cv_; std::mutex gc_waiting_mutex_; HeapEventStorage storage_; From 3c2f868dac6a680c60f94b2a6b01ed7b73ba779d Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Sun, 23 Feb 2020 21:07:26 -0800 Subject: [PATCH 03/11] LSC: Add std:: qualifications to all references to std::string and std::basic_string. Adding these qualifications will make google3 C++ more portable, allow the global using std::string declarations in the google3 copy of the C++ standard library to be deleted, and bring google3 C++ in line with how the rest of the world uses C++. This completes the work started in go/lsc-add-std and go/std-type-qualification. LSC documentation: go/lsc-std-string Tested: tap_presubmit: http://test/OCL:295966075:BASE:296180783:1582224434355:a04fbb44 Some tests failed; test failures are believed to be unrelated to this CL PiperOrigin-RevId: 296808824 --- .../javaprofiler/async_ref_counted_string.cc | 16 +++--- .../javaprofiler/async_ref_counted_string.h | 8 +-- third_party/javaprofiler/display.cc | 33 ++++++------ third_party/javaprofiler/display.h | 12 ++--- third_party/javaprofiler/method_info.h | 27 ++++------ third_party/javaprofiler/native.cc | 4 +- third_party/javaprofiler/native.h | 6 +-- .../javaprofiler/profile_proto_builder.cc | 26 +++++----- .../javaprofiler/profile_proto_builder.h | 27 +++++----- third_party/javaprofiler/profile_test_lib.cc | 3 +- third_party/javaprofiler/profile_test_lib.h | 2 +- third_party/javaprofiler/stacktrace_fixer.cc | 51 +++++++++---------- third_party/javaprofiler/stacktrace_fixer.h | 16 +++--- third_party/javaprofiler/stacktraces.cc | 4 +- third_party/javaprofiler/stacktraces.h | 12 ++--- third_party/javaprofiler/tags.cc | 24 +++++---- third_party/javaprofiler/tags.h | 8 +-- 17 files changed, 137 insertions(+), 142 deletions(-) diff --git a/third_party/javaprofiler/async_ref_counted_string.cc b/third_party/javaprofiler/async_ref_counted_string.cc index 9704f6a78..31b387225 100644 --- a/third_party/javaprofiler/async_ref_counted_string.cc +++ b/third_party/javaprofiler/async_ref_counted_string.cc @@ -23,11 +23,12 @@ namespace google { namespace javaprofiler { namespace { -using StringRefCountTable = std::unordered_map>; +using StringRefCountTable = + std::unordered_map>; using StringRefCount = StringRefCountTable::value_type; // Maps string to its reference count. -std::unordered_map> *string_table = nullptr; +std::unordered_map> *string_table = nullptr; // All accesses to string_table should be protected by string_table_mutex. std::mutex string_table_mutex; @@ -47,7 +48,7 @@ std::mutex string_table_mutex; // Resolves the StringRefCount of a given string, increments the reference count // and returns the pointer of StringRefCount. It returns nullptr if the internal // string table is not initialized. -StringRefCount *AcquireByString(const string &str) { +StringRefCount *AcquireByString(const std::string &str) { std::lock_guard lock(string_table_mutex); if (string_table == nullptr) { return nullptr; @@ -114,7 +115,7 @@ void Release(StringRefCount *str_ref_cnt) { } // namespace -AsyncRefCountedString::AsyncRefCountedString(const string &str) +AsyncRefCountedString::AsyncRefCountedString(const std::string &str) : AsyncRefCountedString() { Release(ptr_.exchange(AcquireByString(str))); } @@ -128,7 +129,8 @@ AsyncRefCountedString::~AsyncRefCountedString() { Release(ptr_.exchange(nullptr)); } -AsyncRefCountedString &AsyncRefCountedString::operator=(const string &str) { +AsyncRefCountedString &AsyncRefCountedString::operator=( + const std::string &str) { Release(ptr_.exchange(AcquireByString(str))); return *this; } @@ -160,7 +162,7 @@ void AsyncRefCountedString::AsyncSafeReset() { assert(AsyncSafeRelease(ptr_.exchange(nullptr))); } -const string *AsyncRefCountedString::Get() const { +const std::string *AsyncRefCountedString::Get() const { StringRefCount *str_ref_cnt = ptr_.load(); if (str_ref_cnt == nullptr) { return nullptr; @@ -172,7 +174,7 @@ const string *AsyncRefCountedString::Get() const { bool AsyncRefCountedString::Init() { std::lock_guard lock(string_table_mutex); if (string_table == nullptr) { - string_table = new std::unordered_map>(); + string_table = new std::unordered_map>(); return true; } return false; diff --git a/third_party/javaprofiler/async_ref_counted_string.h b/third_party/javaprofiler/async_ref_counted_string.h index 276ccb88a..10560fede 100644 --- a/third_party/javaprofiler/async_ref_counted_string.h +++ b/third_party/javaprofiler/async_ref_counted_string.h @@ -46,12 +46,12 @@ namespace javaprofiler { class AsyncRefCountedString { public: AsyncRefCountedString() : ptr_(nullptr) {} - explicit AsyncRefCountedString(const string &str); + explicit AsyncRefCountedString(const std::string &str); AsyncRefCountedString(const AsyncRefCountedString &other); ~AsyncRefCountedString(); - AsyncRefCountedString &operator=(const string &str); + AsyncRefCountedString &operator=(const std::string &str); AsyncRefCountedString &operator=(const AsyncRefCountedString &other); AsyncRefCountedString &operator=(AsyncRefCountedString &&other); // Async-signal-safe version of operator=(const AsyncRefCountedString &other). @@ -82,7 +82,7 @@ class AsyncRefCountedString { // Returns nullptr when it does not refer to any string. // The returned string pointer is valid as long as the AsyncRefCountedString // instance stay unchanged and it is async-signal-safe. - const string *Get() const; + const std::string *Get() const; // Returns the hash value. As a specific string only has one internal copy, // the stored string address is directly used as the hash value. @@ -103,7 +103,7 @@ class AsyncRefCountedString { private: // The value of ptr_ requires to be changed atomically so that // its value will not get corrupted upon interrupts. - std::atomic> *> ptr_; + std::atomic> *> ptr_; }; } // namespace javaprofiler diff --git a/third_party/javaprofiler/display.cc b/third_party/javaprofiler/display.cc index f4d940ef5..86b867880 100644 --- a/third_party/javaprofiler/display.cc +++ b/third_party/javaprofiler/display.cc @@ -50,8 +50,8 @@ const char kMethodIDUnknown[] = "UnknownMethodID"; // Since the method is unknown, associated signature is empty. const char kSignatureUnknown[] = ""; -void GetMethodName(jvmtiEnv *jvmti, jmethodID method_id, string *method_name, - string *signature) { +void GetMethodName(jvmtiEnv *jvmti, jmethodID method_id, + std::string *method_name, std::string *signature) { jint error; JvmtiScopedPtr signature_ptr(jvmti); JvmtiScopedPtr name_ptr(jvmti); @@ -89,8 +89,8 @@ void GetMethodName(jvmtiEnv *jvmti, jmethodID method_id, string *method_name, } void GetClassAndFileName(jvmtiEnv *jvmti, jmethodID method_id, - jclass declaring_class, string *file_name, - string *class_name) { + jclass declaring_class, std::string *file_name, + std::string *class_name) { JvmtiScopedPtr source_name_ptr(jvmti); if (JVMTI_ERROR_NONE != jvmti->GetSourceFileName(declaring_class, source_name_ptr.GetRef())) { @@ -117,9 +117,9 @@ void GetClassAndFileName(jvmtiEnv *jvmti, jmethodID method_id, } } -void FillFieldsWithUnknown(string *file_name, string *class_name, - string *method_name, string *signature, - int *line_number) { +void FillFieldsWithUnknown(std::string *file_name, std::string *class_name, + std::string *method_name, std::string *signature, + int *line_number) { *file_name = kFileUnknown; *class_name = kClassUnknown; *method_name = kMethodUnknown; @@ -129,10 +129,9 @@ void FillFieldsWithUnknown(string *file_name, string *class_name, } } -void FillMethodSignatureAndLine(jvmtiEnv *jvmti, - const JVMPI_CallFrame &frame, - string *method_name, string *signature, - int *line_number) { +void FillMethodSignatureAndLine(jvmtiEnv *jvmti, const JVMPI_CallFrame &frame, + std::string *method_name, + std::string *signature, int *line_number) { GetMethodName(jvmti, frame.method_id, method_name, signature); // frame.lineno is actually a bci if it is a Java method; Asgct is piggy @@ -196,9 +195,9 @@ jint GetLineNumber(jvmtiEnv *jvmti, jmethodID method, jlocation location) { } bool GetStackFrameElements(JNIEnv *jni, jvmtiEnv *jvmti, - const JVMPI_CallFrame &frame, string *file_name, - string *class_name, string *method_name, - string *signature, int *line_number) { + const JVMPI_CallFrame &frame, std::string *file_name, + std::string *class_name, std::string *method_name, + std::string *signature, int *line_number) { if (!jvmti) { FillFieldsWithUnknown(file_name, class_name, method_name, signature, line_number); @@ -221,9 +220,9 @@ bool GetStackFrameElements(JNIEnv *jni, jvmtiEnv *jvmti, } bool GetStackFrameElements(jvmtiEnv *jvmti, const JVMPI_CallFrame &frame, - jclass declaring_class, string *file_name, - string *class_name, string *method_name, - string *signature, int *line_number) { + jclass declaring_class, std::string *file_name, + std::string *class_name, std::string *method_name, + std::string *signature, int *line_number) { if (!jvmti) { FillFieldsWithUnknown(file_name, class_name, method_name, signature, line_number); diff --git a/third_party/javaprofiler/display.h b/third_party/javaprofiler/display.h index ee897759c..ffaccb87a 100644 --- a/third_party/javaprofiler/display.h +++ b/third_party/javaprofiler/display.h @@ -38,9 +38,9 @@ jint GetLineNumber(jvmtiEnv *jvmti, jmethodID method, jlocation location); // When unknown, it fills the parameters with: UnknownFile, UnknownClass, // UnknownMethod, "", and -1. bool GetStackFrameElements(JNIEnv *jni, jvmtiEnv *jvmti, - const JVMPI_CallFrame &frame, string *file_name, - string *class_name, string *method_name, - string *signature, int *line_number); + const JVMPI_CallFrame &frame, std::string *file_name, + std::string *class_name, std::string *method_name, + std::string *signature, int *line_number); // Fill the file_name, class_name, method_name, and line_number parameters using // the information provided by the frame and using the JVMTI environment. @@ -49,9 +49,9 @@ bool GetStackFrameElements(JNIEnv *jni, jvmtiEnv *jvmti, // When unknown, it fills the parameters with: UnknownFile, UnknownClass, // UnknownMethod, "", and -1. bool GetStackFrameElements(jvmtiEnv *jvmti, const JVMPI_CallFrame &frame, - jclass declaring_class, string *file_name, - string *class_name, string *method_name, - string *signature, int *line_number); + jclass declaring_class, std::string *file_name, + std::string *class_name, std::string *method_name, + std::string *signature, int *line_number); } // namespace javaprofiler } // namespace google diff --git a/third_party/javaprofiler/method_info.h b/third_party/javaprofiler/method_info.h index 7b6fd205b..a66f81a5b 100644 --- a/third_party/javaprofiler/method_info.h +++ b/third_party/javaprofiler/method_info.h @@ -33,10 +33,11 @@ namespace javaprofiler { class MethodInfo { public: // Constructor providing all the information regarding a method. - MethodInfo(const string &method_name, - const string &class_name, - const string &file_name) : - method_name_(method_name), class_name_(class_name), file_name_(file_name) {} + MethodInfo(const std::string &method_name, const std::string &class_name, + const std::string &file_name) + : method_name_(method_name), + class_name_(class_name), + file_name_(file_name) {} // An invalid location Id. static const int64 kInvalidLocationId = 0; @@ -50,22 +51,16 @@ class MethodInfo { locations_[bci] = location_id; } - const string &MethodName() const { - return method_name_; - } + const std::string &MethodName() const { return method_name_; } - const string &ClassName() const { - return class_name_; - } + const std::string &ClassName() const { return class_name_; } - const string &FileName() const { - return file_name_; - } + const std::string &FileName() const { return file_name_; } private: - string method_name_; - string class_name_; - string file_name_; + std::string method_name_; + std::string class_name_; + std::string file_name_; // Cache of jlocation results. std::unordered_map locations_; diff --git a/third_party/javaprofiler/native.cc b/third_party/javaprofiler/native.cc index a6d684f13..3a27655bb 100644 --- a/third_party/javaprofiler/native.cc +++ b/third_party/javaprofiler/native.cc @@ -30,7 +30,7 @@ namespace google { namespace javaprofiler { -NativeProcessInfo::NativeProcessInfo(const string &procmaps_filename) +NativeProcessInfo::NativeProcessInfo(const std::string &procmaps_filename) : procmaps_filename_(procmaps_filename) { Refresh(); } @@ -77,7 +77,7 @@ void NativeProcessInfo::Refresh() { const char *filename = &line[filename_index]; size_t filename_len = strcspn(filename, " \t\n"); mappings_.emplace_back( - Mapping{start, limit, string(filename, filename_len)}); + Mapping{start, limit, std::string(filename, filename_len)}); } fclose(f); } diff --git a/third_party/javaprofiler/native.h b/third_party/javaprofiler/native.h index 58acab8ce..29a69e458 100644 --- a/third_party/javaprofiler/native.h +++ b/third_party/javaprofiler/native.h @@ -31,18 +31,18 @@ class NativeProcessInfo { public: // procmaps_filename contains the path of a file describing the memory // regions in the current process, in /proc//maps format. - explicit NativeProcessInfo(const string &procmaps_filename); + explicit NativeProcessInfo(const std::string &procmaps_filename); struct Mapping { uint64 start, limit; - string name; + std::string name; }; void Refresh(); const std::vector &Mappings() const { return mappings_; } private: - const string procmaps_filename_; + const std::string procmaps_filename_; std::vector mappings_; DISALLOW_COPY_AND_ASSIGN(NativeProcessInfo); }; diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index 1138ce587..a270e4e7f 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -62,8 +62,8 @@ void ProfileProtoBuilder::AddTraces(const ProfileStackTrace *traces, } } -void ProfileProtoBuilder::AddArtificialTrace(const string& name, int count, - int sampling_rate) { +void ProfileProtoBuilder::AddArtificialTrace(const std::string &name, int count, + int sampling_rate) { perftools::profiles::Location *location = location_builder_.LocationFor( name, name, "", -1); @@ -216,10 +216,10 @@ MethodInfo *ProfileProtoBuilder::Method(jmethodID method_id) { return it->second.get(); } - string file_name; - string class_name; - string method_name; - string signature; + std::string file_name; + std::string class_name; + std::string method_name; + std::string signature; // Ignore lineno since we pass nullptr anyway. JVMPI_CallFrame jvm_frame = { 0, method_id }; @@ -227,7 +227,7 @@ MethodInfo *ProfileProtoBuilder::Method(jmethodID method_id) { &class_name, &method_name, &signature, nullptr); FixMethodParameters(&signature); - string full_method_name = class_name + "." + method_name + signature; + std::string full_method_name = class_name + "." + method_name + signature; std::unique_ptr unique_method( new MethodInfo(full_method_name, class_name, file_name)); @@ -248,7 +248,7 @@ void ProfileProtoBuilder::AddNativeInfo(const JVMPI_CallFrame &jvm_frame, return; } - string function_name = native_cache_->GetFunctionName(jvm_frame); + std::string function_name = native_cache_->GetFunctionName(jvm_frame); perftools::profiles::Location *location = native_cache_->GetLocation(jvm_frame, &location_builder_); @@ -282,7 +282,7 @@ size_t LocationBuilder::LocationInfoHash::operator()( unsigned int h = 1; - hash hash_string; + hash hash_string; hash hash_int; h = 31U * h + hash_string(info.class_name); @@ -302,10 +302,8 @@ bool LocationBuilder::LocationInfoEquals::operator()( } perftools::profiles::Location *LocationBuilder::LocationFor( - const string &class_name, - const string &function_name, - const string &file_name, - int line_number) { + const std::string &class_name, const std::string &function_name, + const std::string &file_name, int line_number) { auto profile = builder_->mutable_profile(); LocationInfo info{ class_name, function_name, file_name, line_number }; @@ -322,7 +320,7 @@ perftools::profiles::Location *LocationBuilder::LocationFor( auto line = location->add_line(); - string simplified_name = function_name; + std::string simplified_name = function_name; SimplifyFunctionName(&simplified_name); auto function_id = builder_->FunctionId( simplified_name.c_str(), function_name.c_str(), file_name.c_str(), 0); diff --git a/third_party/javaprofiler/profile_proto_builder.h b/third_party/javaprofiler/profile_proto_builder.h index 967ba53c6..56dd29410 100644 --- a/third_party/javaprofiler/profile_proto_builder.h +++ b/third_party/javaprofiler/profile_proto_builder.h @@ -68,16 +68,16 @@ class LocationBuilder { // Return an existing or new location matching the given parameters, // modifying the profile as needed to add new function and location // information. - perftools::profiles::Location *LocationFor(const string &class_name, - const string &function_name, - const string &file_name, + perftools::profiles::Location *LocationFor(const std::string &class_name, + const std::string &function_name, + const std::string &file_name, int line_number); private: struct LocationInfo { - string class_name; - string function_name; - string file_name; + std::string class_name; + std::string function_name; + std::string file_name; int line_number; }; @@ -105,7 +105,7 @@ class ProfileFrameCache { virtual perftools::profiles::Location *GetLocation( const JVMPI_CallFrame &jvm_frame, LocationBuilder *location_builder) = 0; - virtual string GetFunctionName(const JVMPI_CallFrame &jvm_frame) = 0; + virtual std::string GetFunctionName(const JVMPI_CallFrame &jvm_frame) = 0; virtual ~ProfileFrameCache() {} }; @@ -128,7 +128,8 @@ class ProfileProtoBuilder { // Add a "fake" trace with a single frame. Used to represent JVM // tasks such as JIT compilation and GC. - void AddArtificialTrace(const string& name, int count, int sampling_rate); + void AddArtificialTrace(const std::string &name, int count, + int sampling_rate); // Build the proto. Calling any other method on the class after calling // this has undefined behavior. @@ -161,11 +162,11 @@ class ProfileProtoBuilder { protected: struct SampleType { - SampleType(const string &type_in, const string &unit_in) + SampleType(const std::string &type_in, const std::string &unit_in) : type(type_in), unit(unit_in) {} - string type; - string unit; + std::string type; + std::string unit; }; // Create the profile proto builder, if native_cache is nullptr, then no @@ -204,7 +205,7 @@ class ProfileProtoBuilder { } // Notify the state that we are visiting a native frame. - void NativeFrame(const string &function_name) { + void NativeFrame(const std::string &function_name) { if (StartsWith(function_name, "JavaCalls::call_helper")) { in_jni_helpers_ = true; } @@ -223,7 +224,7 @@ class ProfileProtoBuilder { // TODO: Support a "complete detail" mode to override this. bool in_jni_helpers_ = false; - static bool StartsWith(const string &s, const string &prefix) { + static bool StartsWith(const std::string &s, const std::string &prefix) { return s.find(prefix) == 0; } }; diff --git a/third_party/javaprofiler/profile_test_lib.cc b/third_party/javaprofiler/profile_test_lib.cc index e4873c36d..701dc69ed 100644 --- a/third_party/javaprofiler/profile_test_lib.cc +++ b/third_party/javaprofiler/profile_test_lib.cc @@ -35,8 +35,7 @@ static jvmtiError Deallocate(jvmtiEnv* jvmti, unsigned char* mem) { return JVMTI_ERROR_NONE; } -static void CreateJvmtiString(jvmtiEnv* jvmti, - const string& name, +static void CreateJvmtiString(jvmtiEnv* jvmti, const std::string& name, char** name_str) { unsigned char* name_str_u; jvmti->Allocate(name.size() + 1, &name_str_u); diff --git a/third_party/javaprofiler/profile_test_lib.h b/third_party/javaprofiler/profile_test_lib.h index 91a521b82..1f0655700 100644 --- a/third_party/javaprofiler/profile_test_lib.h +++ b/third_party/javaprofiler/profile_test_lib.h @@ -35,7 +35,7 @@ class TestProfileFrameCache : public ProfileFrameCache { return &nop_; } - string GetFunctionName(const JVMPI_CallFrame &jvm_frame) { return ""; } + std::string GetFunctionName(const JVMPI_CallFrame &jvm_frame) { return ""; } private: perftools::profiles::Location nop_; diff --git a/third_party/javaprofiler/stacktrace_fixer.cc b/third_party/javaprofiler/stacktrace_fixer.cc index f63652edc..417fe6741 100644 --- a/third_party/javaprofiler/stacktrace_fixer.cc +++ b/third_party/javaprofiler/stacktrace_fixer.cc @@ -27,7 +27,7 @@ namespace { // followed by specified suffix chars and removing those. For example, // calling the function with ("foo123bar", "foo", "321") returns "foobar". template -void SimplifySuffixedName(string *name, const char (&trigger)[M], +void SimplifySuffixedName(std::string *name, const char (&trigger)[M], const char (&suffix_chars)[N]) { size_t first = 0; while ((first = name->find(trigger, first)) != std::string::npos) { @@ -45,7 +45,7 @@ void SimplifySuffixedName(string *name, const char (&trigger)[M], // or '$$EnhancedBy*$$' in its name) to make it more human readable, and group // related functions under a single name. This could be done with a regexp // replacement, but including RE2 increases the size of the agent. -void SimplifyDynamicClassName(string *name) { +void SimplifyDynamicClassName(std::string *name) { // Replace $$[0-9a-f]+ by $$ to remove unique values, for example in // $FastClassByCGLIB$$fd6bdf6d.invoke. SimplifySuffixedName(name, "$$", "0123456789abcdef"); @@ -54,7 +54,7 @@ void SimplifyDynamicClassName(string *name) { // Simplifies the name of a lambda method to replace $$Lambda$[0-9]+\.[0-9]+ by // $$Lambda$ to remove unique values, for example in // com.google.something.Something$$Lambda$197.1849072452.run. -void SimplifyLambdaName(string *name) { +void SimplifyLambdaName(std::string *name) { constexpr char trigger[] = "$$Lambda$"; constexpr char digits[] = "0123456789"; const size_t trigger_length = strlen(trigger); @@ -89,7 +89,7 @@ constexpr char digits[] = "0123456789"; // Simplifies the name of a method generated by the runtime as a reflection // stub. See the test file for examples, or generateName() in // jdk/internal/reflect/MethodAccessorGenerator.java. -void SimplifyInternalReflectionMethodName(string *name) { +void SimplifyInternalReflectionMethodName(std::string *name) { SimplifySuffixedName( name, "jdk.internal.reflect.GeneratedConstructorAccessor", @@ -109,7 +109,7 @@ void SimplifyInternalReflectionMethodName(string *name) { // Simplifies the name of a method generated by the runtime as a reflection // stub. See the test file for examples, or generateName() in // sun/reflect/MethodAccessorGenerator.java. -void SimplifyReflectionMethodName(string *name) { +void SimplifyReflectionMethodName(std::string *name) { SimplifySuffixedName( name, "sun.reflect.GeneratedConstructorAccessor", @@ -126,13 +126,13 @@ void SimplifyReflectionMethodName(string *name) { digits); } -string ParseMethodTypeSignatureWithReturn(const char *buffer, int buffer_size, - int *pos); +std::string ParseMethodTypeSignatureWithReturn(const char *buffer, + int buffer_size, int *pos); // JVM type signature parser and pretty printer. It returns the pretty-printed // string; it also modifies the position pointer in the input buffer. // TODO: Move ParseFieldType to replace the string directly. -string ParseFieldType(const char *buffer, int buffer_size, int *pos) { +std::string ParseFieldType(const char *buffer, int buffer_size, int *pos) { if (*pos >= buffer_size) { return ""; } @@ -174,7 +174,7 @@ string ParseFieldType(const char *buffer, int buffer_size, int *pos) { for (const char *cur = begin; cur < end; cur++) { if (*cur == ';') { *pos += (cur - begin) + 1; - return string(begin, cur); + return std::string(begin, cur); } } *pos = buffer_size; @@ -182,7 +182,7 @@ string ParseFieldType(const char *buffer, int buffer_size, int *pos) { } case '[': { // Recursively parse the array type. - string s = ParseFieldType(buffer, buffer_size, pos); + std::string s = ParseFieldType(buffer, buffer_size, pos); return s + "[]"; } case '(': { @@ -201,8 +201,8 @@ inline bool AtSignatureEnd(const char* buffer, int buffer_size, int *pos) { return *pos >= buffer_size || buffer[*pos] == ')'; } -string ParseMethodTypeSignatureInternal(const char *buffer, int buffer_size, - int *pos) { +std::string ParseMethodTypeSignatureInternal(const char *buffer, + int buffer_size, int *pos) { if (buffer == nullptr || pos == nullptr || *pos >= buffer_size || buffer[*pos] != '(') { return ""; @@ -211,7 +211,7 @@ string ParseMethodTypeSignatureInternal(const char *buffer, int buffer_size, // Skip the '('. (*pos)++; - string buf("("); + std::string buf("("); while (!AtSignatureEnd(buffer, buffer_size, pos)) { buf.append(ParseFieldType(buffer, buffer_size, pos)); if (!AtSignatureEnd(buffer, buffer_size, pos)) { @@ -231,14 +231,15 @@ string ParseMethodTypeSignatureInternal(const char *buffer, int buffer_size, // JVM method type signature parser and pretty printer. It returns the // pretty-printed string; it also modifies the position pointer in the input // buffer. -string ParseMethodTypeSignature(const char *buffer, int buffer_size, int *pos) { +std::string ParseMethodTypeSignature(const char *buffer, int buffer_size, + int *pos) { return ParseMethodTypeSignatureInternal(buffer, buffer_size, pos); } -string ParseMethodTypeSignatureWithReturn(const char *buffer, int buffer_size, - int *pos) { - string argument_string = ParseMethodTypeSignatureInternal(buffer, buffer_size, - pos); +std::string ParseMethodTypeSignatureWithReturn(const char *buffer, + int buffer_size, int *pos) { + std::string argument_string = + ParseMethodTypeSignatureInternal(buffer, buffer_size, pos); if (argument_string.empty()) { return ""; } @@ -248,31 +249,29 @@ string ParseMethodTypeSignatureWithReturn(const char *buffer, int buffer_size, return argument_string; } - string return_string = ParseFieldType(buffer, buffer_size, pos); + std::string return_string = ParseFieldType(buffer, buffer_size, pos); return return_string + " " + argument_string; } } // namespace -void SimplifyFunctionName(string* name) { +void SimplifyFunctionName(std::string *name) { SimplifyDynamicClassName(name); SimplifyLambdaName(name); SimplifyReflectionMethodName(name); SimplifyInternalReflectionMethodName(name); } -void FixPath(string *s) { - std::replace(s->begin(), s->end(), '/', '.'); -} +void FixPath(std::string *s) { std::replace(s->begin(), s->end(), '/', '.'); } -void PrettyPrintSignature(string *s) { +void PrettyPrintSignature(std::string *s) { int pos = 0; - string result = ParseFieldType(s->data(), s->length(), &pos); + std::string result = ParseFieldType(s->data(), s->length(), &pos); FixPath(&result); *s = result; } -void FixMethodParameters(string *signature) { +void FixMethodParameters(std::string *signature) { if (signature == nullptr || signature->empty() || signature->at(0) != '(') { return; diff --git a/third_party/javaprofiler/stacktrace_fixer.h b/third_party/javaprofiler/stacktrace_fixer.h index 84fa732d0..ed15fd9f7 100644 --- a/third_party/javaprofiler/stacktrace_fixer.h +++ b/third_party/javaprofiler/stacktrace_fixer.h @@ -26,17 +26,17 @@ namespace google { namespace javaprofiler { // Simplifies the name of a function to make it more human readable, and group // related functions under a single name. - void SimplifyFunctionName(string *name); +void SimplifyFunctionName(std::string *name); - // Fix the parameter signature from a JVM type signature to a pretty-print - // one. - void FixMethodParameters(string *signature); +// Fix the parameter signature from a JVM type signature to a pretty-print +// one. +void FixMethodParameters(std::string *signature); - // Convert class name format from "pkg/name/class" to "pkg.name.class". - void FixPath(string *s); +// Convert class name format from "pkg/name/class" to "pkg.name.class". +void FixPath(std::string *s); - // Pretty-prints a JVM type signature. - void PrettyPrintSignature(string *s); +// Pretty-prints a JVM type signature. +void PrettyPrintSignature(std::string *s); } // namespace javaprofiler } // namespace google diff --git a/third_party/javaprofiler/stacktraces.cc b/third_party/javaprofiler/stacktraces.cc index 48d7cc450..3a3232a74 100644 --- a/third_party/javaprofiler/stacktraces.cc +++ b/third_party/javaprofiler/stacktraces.cc @@ -20,8 +20,8 @@ namespace javaprofiler { ASGCTType Asgct::asgct_; std::mutex *AttributeTable::mutex_; -std::unordered_map *AttributeTable::string_map_; -std::vector *AttributeTable::strings_; +std::unordered_map *AttributeTable::string_map_; +std::vector *AttributeTable::strings_; bool AsyncSafeTraceMultiset::Add(int attr, JVMPI_CallTrace *trace) { uint64 hash_val = CalculateHash(attr, trace->num_frames, &trace->frames[0]); diff --git a/third_party/javaprofiler/stacktraces.h b/third_party/javaprofiler/stacktraces.h index 9adbbec2e..1ceb50109 100644 --- a/third_party/javaprofiler/stacktraces.h +++ b/third_party/javaprofiler/stacktraces.h @@ -59,8 +59,8 @@ class AttributeTable { public: static void Init() { mutex_ = new (std::mutex); - string_map_ = new (std::unordered_map); - strings_ = new (std::vector); + string_map_ = new (std::unordered_map); + strings_ = new (std::vector); strings_->push_back(""); } @@ -81,9 +81,9 @@ class AttributeTable { return ret; } - static std::vector GetStrings() { + static std::vector GetStrings() { if (mutex_ == nullptr) { - return std::vector(); + return std::vector(); } std::lock_guard lock(*mutex_); return *strings_; @@ -91,8 +91,8 @@ class AttributeTable { private: static std::mutex *mutex_; - static std::unordered_map *string_map_; - static std::vector *strings_; + static std::unordered_map *string_map_; + static std::vector *strings_; DISALLOW_IMPLICIT_CONSTRUCTORS(AttributeTable); }; diff --git a/third_party/javaprofiler/tags.cc b/third_party/javaprofiler/tags.cc index 65d8aaff8..e81801055 100644 --- a/third_party/javaprofiler/tags.cc +++ b/third_party/javaprofiler/tags.cc @@ -31,15 +31,15 @@ std::mutex mutex; const Tags *empty_tags = nullptr; const AsyncRefCountedString *empty_async_string = nullptr; // Stores all the keys. -std::vector *keys = nullptr; +std::vector *keys = nullptr; // Maps key to its id. The index of a key stored in keys (called id or key_id) // is used to retrieve a key or value quickly as it is very friendly to vectors. -std::unordered_map *key_to_id = nullptr; +std::unordered_map *key_to_id = nullptr; // Returns the id of the key if the key has been registered. Otherwise, it tries // to add the key and returns its id upon success or returns -1 if there is no // extra space. -int32_t RegisterKey(const string &key) { +int32_t RegisterKey(const std::string &key) { std::lock_guard lock(mutex); const auto &target = key_to_id->find(key); if (target != key_to_id->end()) { @@ -65,7 +65,7 @@ const char kAttrKey[] = "attr"; // Returns the id of the key if the key has been registered; otherwise, returns // -1. -int32_t Tags::GetIdByKey(const string &key) { +int32_t Tags::GetIdByKey(const std::string &key) { std::lock_guard lock(mutex); auto target = key_to_id->find(key); if (target == key_to_id->end()) { @@ -80,7 +80,7 @@ void Tags::AsyncSafeCopy(const Tags &from) { } } -bool Tags::Set(const string &key, const AsyncRefCountedString &value) { +bool Tags::Set(const std::string &key, const AsyncRefCountedString &value) { // All values are stored in the vector "values_" and "key_id" (resolved by // "key") is used to locate the right position. int32_t key_id = RegisterKey(key); @@ -122,7 +122,7 @@ uint64 Tags::Hash() const { return h; } -const AsyncRefCountedString &Tags::Get(const string &key) const { +const AsyncRefCountedString &Tags::Get(const std::string &key) const { int32_t key_id = GetIdByKey(key); if (key_id >= 0) { return values_[key_id]; @@ -130,9 +130,11 @@ const AsyncRefCountedString &Tags::Get(const string &key) const { return *empty_async_string; } -std::vector> Tags::GetAll() const { +std::vector> Tags::GetAll() + const { std::lock_guard lock(mutex); - std::vector> all_pairs(keys->size()); + std::vector> all_pairs( + keys->size()); for (int i = 0; i < keys->size(); i++) { all_pairs[i].first = (*keys)[i]; all_pairs[i].second = values_[i]; @@ -147,7 +149,7 @@ void Tags::SetAttribute(int64 value) { } int64 Tags::GetAttribute() const { - const string *value = Get(kAttrKey).Get(); + const std::string *value = Get(kAttrKey).Get(); if (value == nullptr) { return 0; } @@ -164,8 +166,8 @@ bool Tags::Init() { return false; } empty_async_string = new AsyncRefCountedString(); - keys = new std::vector(); - key_to_id = new std::unordered_map(); + keys = new std::vector(); + key_to_id = new std::unordered_map(); empty_tags = new Tags(); } // Register the key "Accessors::kAttrKey" to support Accessors::SetAttribute() diff --git a/third_party/javaprofiler/tags.h b/third_party/javaprofiler/tags.h index 0523c83a1..918af1bc1 100644 --- a/third_party/javaprofiler/tags.h +++ b/third_party/javaprofiler/tags.h @@ -34,16 +34,16 @@ class Tags { // Returns the value (in AsyncRefCountedString reference) indexed by "key". Be // cautious that the referenced AsyncRefCountedString may change if modifiers // (e.g., Set(), Clear()) are invoked later. - const AsyncRefCountedString &Get(const string &key) const; + const AsyncRefCountedString &Get(const std::string &key) const; // Sets the value indexed by "key" to "value". It returns true on success; // otherwise returns false. The failure usually comes from that "key" does not // exist in the key list while there is no space to accommodate this new // "key". - bool Set(const string &key, const AsyncRefCountedString &value); + bool Set(const std::string &key, const AsyncRefCountedString &value); // Resets all values to nullptr. void ClearAll(); // Returns all the key-value pairs stored in this Tags. - std::vector> GetAll() const; + std::vector> GetAll() const; // Legacy interfaces. The attribute value is stored internally indexed by a // special key (kAttrKey). @@ -80,7 +80,7 @@ class Tags { private: // // Returns the id of "key" if "key" is present; otherwise, returns -1. - static int32_t GetIdByKey(const string &key); + static int32_t GetIdByKey(const std::string &key); AsyncRefCountedString values_[kMaxNumTags]; From 1ee2efe3241f134bd6ee53a9c5037c7533339ac5 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Mon, 2 Mar 2020 16:03:25 -0800 Subject: [PATCH 04/11] Delete dead code PiperOrigin-RevId: 298470814 --- third_party/javaprofiler/profile_proto_builder.cc | 5 ----- third_party/javaprofiler/profile_proto_builder.h | 1 - 2 files changed, 6 deletions(-) diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index a270e4e7f..250c5e20d 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -126,11 +126,6 @@ void ProfileProtoBuilder::UpdateSampleValues( sample->set_value(kMetric, sample->value(kMetric) + size); } -void ProfileProtoBuilder::InitSampleValues( - perftools::profiles::Sample *sample, int64 metric) { - InitSampleValues(sample, 1, metric); -} - void ProfileProtoBuilder::InitSampleValues( perftools::profiles::Sample *sample, int64 count, int64 metric) { sample->add_value(count); diff --git a/third_party/javaprofiler/profile_proto_builder.h b/third_party/javaprofiler/profile_proto_builder.h index 56dd29410..5678cb92e 100644 --- a/third_party/javaprofiler/profile_proto_builder.h +++ b/third_party/javaprofiler/profile_proto_builder.h @@ -231,7 +231,6 @@ class ProfileProtoBuilder { void AddSampleType(const SampleType &sample_type); void SetPeriodType(const SampleType &metric_type); - void InitSampleValues(perftools::profiles::Sample *sample, int64 metric); void InitSampleValues(perftools::profiles::Sample *sample, int64 count, int64 metric); void UpdateSampleValues(perftools::profiles::Sample *sample, int64 count, From 2412d00b5c8493ca701b75dd9d0d7a7ee29d531c Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 5 Mar 2020 16:49:31 -0800 Subject: [PATCH 05/11] Add support for "bytes" tag in heap sampler. PiperOrigin-RevId: 299228617 --- third_party/javaprofiler/heap_sampler.cc | 3 +- .../javaprofiler/profile_proto_builder.cc | 95 +++++++++++++++---- .../javaprofiler/profile_proto_builder.h | 58 ++++++++--- 3 files changed, 120 insertions(+), 36 deletions(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index 4be70d837..d0d8ad2b4 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -147,7 +147,8 @@ std::unique_ptr HeapEventStorage::ConvertToProto( *trace_target = {call_target, object->Size()}; } - builder->AddTraces(stack_trace_data.get(), objects_size); + builder->AddTracesAppendingMetricAsLabel(stack_trace_data.get(), + objects_size); return builder->CreateProto(); } diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index 250c5e20d..3b6753a51 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -24,16 +24,17 @@ namespace javaprofiler { const int kCount = 0; const int kMetric = 1; -ProfileProtoBuilder::ProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, - ProfileFrameCache *native_cache, - int64 sampling_rate, - const SampleType &count_type, - const SampleType &metric_type) +ProfileProtoBuilder::ProfileProtoBuilder( + JNIEnv *jni_env, jvmtiEnv *jvmti_env, + ProfileFrameCache *native_cache, int64 sampling_rate, + const SampleType &count_type, const SampleType &metric_type, + const std::list &label_types) : sampling_rate_(sampling_rate), jni_env_(jni_env), jvmti_env_(jvmti_env), native_cache_(native_cache), - location_builder_(&builder_) { + location_builder_(&builder_), + label_types_(label_types) { AddSampleType(count_type); AddSampleType(metric_type); SetPeriodType(metric_type); @@ -46,7 +47,7 @@ void ProfileProtoBuilder::AddTraces(const ProfileStackTrace *traces, } for (int i = 0; i < num_traces; ++i) { - AddTrace(traces[i], 1); + AddTrace(traces[i], 1, false); } } @@ -58,7 +59,29 @@ void ProfileProtoBuilder::AddTraces(const ProfileStackTrace *traces, } for (int i = 0; i < num_traces; ++i) { - AddTrace(traces[i], counts[i]); + AddTrace(traces[i], counts[i], false); + } +} + +void ProfileProtoBuilder::AddTracesAppendingMetricAsLabel( + const ProfileStackTrace *traces, int num_traces) { + if (native_cache_) { + native_cache_->ProcessTraces(traces, num_traces); + } + + for (int i = 0; i < num_traces; ++i) { + AddTrace(traces[i], 1, true); + } +} + +void ProfileProtoBuilder::AddTracesAppendingMetricAsLabel( + const ProfileStackTrace *traces, const int32 *counts, int num_traces) { + if (native_cache_) { + native_cache_->ProcessTraces(traces, num_traces); + } + + for (int i = 0; i < num_traces; ++i) { + AddTrace(traces[i], counts[i], true); } } @@ -71,7 +94,8 @@ void ProfileProtoBuilder::AddArtificialTrace(const std::string &name, int count, auto sample = profile->add_sample(); sample->add_location_id(location->id()); // Move count * sampling rate to 64-bit. - InitSampleValues(sample, count, static_cast(count) * sampling_rate); + InitSampleValues(sample, count, static_cast(count) * sampling_rate, + std::list()); } void ProfileProtoBuilder::UnsampleMetrics() { @@ -127,14 +151,32 @@ void ProfileProtoBuilder::UpdateSampleValues( } void ProfileProtoBuilder::InitSampleValues( - perftools::profiles::Sample *sample, int64 count, int64 metric) { + perftools::profiles::Sample *sample, int64 count, int64 metric, + const std::list &label_values) { sample->add_value(count); sample->add_value(metric); + + DCHECK (label_values.size() == label_types_.size()) + << "Number of label values != number of label types"; + auto it_value = label_values.begin(); + auto it_type = label_types_.begin(); + for (; it_value != label_values.end() && it_type != label_types_.end(); + ++it_value, ++it_type) { + auto label = sample->add_label(); + label->set_key(builder_.StringId(it_type->type.c_str())); + label->set_num(*it_value); + label->set_num_unit(builder_.StringId(it_type->unit.c_str())); + } } void ProfileProtoBuilder::AddTrace(const ProfileStackTrace &trace, - int32 count) { - auto sample = trace_samples_.SampleFor(*trace.trace); + int32 count, + bool append_metric_value_as_label) { + std::list label_values; + if (append_metric_value_as_label) { + label_values.push_back(trace.metric_value); + } + auto sample = trace_samples_.SampleFor(trace.trace, label_values); if (sample != nullptr) { UpdateSampleValues(sample, count, trace.metric_value); @@ -144,9 +186,9 @@ void ProfileProtoBuilder::AddTrace(const ProfileStackTrace &trace, auto profile = builder_.mutable_profile(); sample = profile->add_sample(); - trace_samples_.Add(*trace.trace, sample); + trace_samples_.Add(trace.trace, label_values, sample); - InitSampleValues(sample, count, trace.metric_value); + InitSampleValues(sample, count, trace.metric_value, label_values); int first_frame = SkipTopNativeFrames(*trace.trace); @@ -326,8 +368,10 @@ perftools::profiles::Location *LocationBuilder::LocationFor( return location; } -size_t TraceSamples::TraceHash::operator()(const JVMPI_CallTrace *trace) const { +size_t TraceSamples::TraceHash::operator()(const TraceAndValues &trace_values) + const { unsigned int h = 1; + const JVMPI_CallTrace *trace = trace_values.trace; for (int f = 0; f < trace->num_frames; f++) { { int len = sizeof(jint); @@ -344,11 +388,19 @@ size_t TraceSamples::TraceHash::operator()(const JVMPI_CallTrace *trace) const { } } } + for (auto value : trace_values.label_values) { + h = 31U * h + static_cast(value); + } return h; } -bool TraceSamples::TraceEquals::operator()( - const JVMPI_CallTrace *trace1, const JVMPI_CallTrace *trace2) const { +bool TraceSamples::TraceEquals::operator()(const TraceAndValues &trace_values1, + const TraceAndValues &trace_values2) const { + if (trace_values1.label_values != trace_values2.label_values) { + return false; + } + const JVMPI_CallTrace *trace1 = trace_values1.trace; + const JVMPI_CallTrace *trace2 = trace_values2.trace; if (trace1->num_frames != trace2->num_frames) { return false; } @@ -367,8 +419,8 @@ bool TraceSamples::TraceEquals::operator()( } perftools::profiles::Sample *TraceSamples::SampleFor( - const JVMPI_CallTrace &trace) const { - auto found = traces_.find(&trace); + const JVMPI_CallTrace *trace, const std::list &label_values) const { + auto found = traces_.find({trace, label_values}); if (found == traces_.end()) { return nullptr; } @@ -376,9 +428,10 @@ perftools::profiles::Sample *TraceSamples::SampleFor( return found->second; } -void TraceSamples::Add(const JVMPI_CallTrace &trace, +void TraceSamples::Add(const JVMPI_CallTrace *trace, + const std::list &label_values, perftools::profiles::Sample *sample) { - traces_[&trace] = sample; + traces_[{trace, label_values}] = sample; } double CalculateSamplingRatio(int64 rate, int64 count, int64 metric_value) { diff --git a/third_party/javaprofiler/profile_proto_builder.h b/third_party/javaprofiler/profile_proto_builder.h index 5678cb92e..eac0b324d 100644 --- a/third_party/javaprofiler/profile_proto_builder.h +++ b/third_party/javaprofiler/profile_proto_builder.h @@ -21,10 +21,10 @@ #include #include +#include #include #include #include -#include #include "perftools/profiles/proto/builder.h" #include "third_party/javaprofiler/method_info.h" @@ -38,23 +38,31 @@ struct ProfileStackTrace { jint metric_value; }; -// Store proto sample objects for specific stack traces. +// Store proto sample objects for specific stack traces and label values. class TraceSamples { public: - perftools::profiles::Sample *SampleFor(const JVMPI_CallTrace &trace) const; - void Add(const JVMPI_CallTrace &trace, perftools::profiles::Sample *sample); + perftools::profiles::Sample *SampleFor( + const JVMPI_CallTrace *trace, const std::list &label_values) const; + void Add(const JVMPI_CallTrace *trace, const std::list &label_values, + perftools::profiles::Sample *sample); private: + struct TraceAndValues { + const JVMPI_CallTrace *trace; + // Values for each label type in ProfileProtoBuilder::label_types_. + const std::list label_values; + }; + struct TraceHash { - size_t operator()(const JVMPI_CallTrace *trace) const; + size_t operator()(const TraceAndValues &trace_values) const; }; struct TraceEquals { - bool operator()(const JVMPI_CallTrace *trace1, - const JVMPI_CallTrace *trace2) const; + bool operator()(const TraceAndValues &trace_values1, + const TraceAndValues &trace_values2) const; }; - std::unordered_map traces_; }; @@ -126,6 +134,20 @@ class ProfileProtoBuilder { const int32 *counts, int num_traces); + // Add traces to the proto, and append traces[i].metric_value as the label + // value corresponding to the last label_type. The traces array must not be + // deleted before calling CreateProto. + void AddTracesAppendingMetricAsLabel(const ProfileStackTrace *traces, + int num_traces); + + // Add traces to the proto, where each trace has a defined count of + // occurrences, and append traces[i].metric_value as the label + // value corresponding to the last label_type. The traces array must not be + // deleted before calling CreateProto. + void AddTracesAppendingMetricAsLabel(const ProfileStackTrace *traces, + const int32 *counts, + int num_traces); + // Add a "fake" trace with a single frame. Used to represent JVM // tasks such as JIT compilation and GC. void AddArtificialTrace(const std::string &name, int count, @@ -175,7 +197,8 @@ class ProfileProtoBuilder { ProfileProtoBuilder(JNIEnv *env, jvmtiEnv *jvmti_env, ProfileFrameCache *native_cache, int64 sampling_rate, const SampleType &count_type, - const SampleType &metric_type); + const SampleType &metric_type, + const std::list &label_types); // An implementation must decide how many frames to skip in a trace. virtual int SkipTopNativeFrames(const JVMPI_CallTrace &trace) = 0; @@ -232,10 +255,11 @@ class ProfileProtoBuilder { void AddSampleType(const SampleType &sample_type); void SetPeriodType(const SampleType &metric_type); void InitSampleValues(perftools::profiles::Sample *sample, int64 count, - int64 metric); + int64 metric, const std::list &label_values); void UpdateSampleValues(perftools::profiles::Sample *sample, int64 count, int64 size); - void AddTrace(const ProfileStackTrace &trace, int32 count); + void AddTrace(const ProfileStackTrace &trace, int32 count, + bool append_metric_value_as_label); void AddJavaInfo(const JVMPI_CallFrame &jvm_frame, perftools::profiles::Profile *profile, perftools::profiles::Sample *sample, @@ -268,6 +292,7 @@ class ProfileProtoBuilder { ProfileFrameCache *native_cache_; TraceSamples trace_samples_; LocationBuilder location_builder_; + const std::list label_types_; }; // Computes the ratio to use to scale heap data to unsample it. @@ -286,7 +311,8 @@ class CpuProfileProtoBuilder : public ProfileProtoBuilder { : ProfileProtoBuilder( jni_env, jvmti_env, cache, sampling_rate, ProfileProtoBuilder::SampleType("samples", "count"), - ProfileProtoBuilder::SampleType("cpu", "nanoseconds")) { + ProfileProtoBuilder::SampleType("cpu", "nanoseconds"), + std::list()) { builder_.mutable_profile()->set_duration_nanos(duration_ns); builder_.mutable_profile()->set_period(sampling_rate); } @@ -306,7 +332,10 @@ class HeapProfileProtoBuilder : public ProfileProtoBuilder { : ProfileProtoBuilder( jni_env, jvmti_env, cache, sampling_rate, ProfileProtoBuilder::SampleType("inuse_objects", "count"), - ProfileProtoBuilder::SampleType("inuse_space", "bytes")) {} + ProfileProtoBuilder::SampleType("inuse_space", "bytes"), + std::list({ + ProfileProtoBuilder::SampleType("bytes", "bytes") + })) {} std::unique_ptr CreateProto() override { return CreateUnsampledProto(); @@ -362,7 +391,8 @@ class ContentionProfileProtoBuilder : public ProfileProtoBuilder { : ProfileProtoBuilder( jni_env, jvmti_env, cache, sampling_rate, ProfileProtoBuilder::SampleType("contentions", "count"), - ProfileProtoBuilder::SampleType("delay", "microseconds")) { + ProfileProtoBuilder::SampleType("delay", "microseconds"), + std::list()) { builder_.mutable_profile()->set_period(sampling_rate); } From af618e824cf9d4f478bd6b46dce96758a8425628 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Fri, 3 Apr 2020 01:09:36 -0700 Subject: [PATCH 06/11] Download OpenSSL source from Github PiperOrigin-RevId: 304565178 --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 47ee03d13..f33e3c392 100644 --- a/Dockerfile +++ b/Dockerfile @@ -81,7 +81,7 @@ RUN mkdir /tmp/glog && cd /tmp/glog && \ # parallel builds, see # https://github.com/openssl/openssl/issues/5762#issuecomment-376622684. RUN mkdir /tmp/openssl && cd /tmp/openssl && \ - curl -sL https://www.openssl.org/source/openssl-1.0.2o.tar.gz | \ + curl -sL https://github.com/openssl/openssl/archive/OpenSSL_1_0_2t.tar.gz | \ tar xzv --strip=1 && \ ./config no-shared -fPIC --openssldir=/usr/local/ssl && \ make && make install_sw && \ From eb164e26ce3b3981c482ebec3ac0ce3597775a93 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Sun, 5 Apr 2020 22:44:17 -0700 Subject: [PATCH 07/11] Use the latest stable version oof OpenSSL 1.1.lf, gRPC 1.28.1, curl 7.66.0 to build Java agent. PiperOrigin-RevId: 304959666 --- Dockerfile | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/Dockerfile b/Dockerfile index f33e3c392..626a09d90 100644 --- a/Dockerfile +++ b/Dockerfile @@ -38,8 +38,6 @@ RUN apt-get update && apt-get -y -q install \ curl \ g++ \ git \ - libcurl4-openssl-dev \ - libssl-dev \ libtool \ make \ openjdk-11-jdk-headless \ @@ -47,13 +45,24 @@ RUN apt-get update && apt-get -y -q install \ unzip \ zlib1g-dev +# openssl +# This openssl (compiled with -fPIC) is used to statically link into the agent +# shared library. +ENV PKG_CONFIG_PATH=/usr/local/ssl/lib/pkgconfig +RUN mkdir /tmp/openssl && cd /tmp/openssl && \ + curl -sL https://github.com/openssl/openssl/archive/OpenSSL_1_1_1f.tar.gz | \ + tar xzv --strip=1 && \ + ./config no-shared -fPIC --openssldir=/usr/local/ssl --prefix=/usr/local/ssl && \ + make && make install_sw && \ + cd ~ && rm -rf /tmp/openssl + # curl -RUN git clone --depth=1 -b curl-7_55_1 https://github.com/curl/curl.git /tmp/curl && \ +RUN git clone --depth=1 -b curl-7_66_0 https://github.com/curl/curl.git /tmp/curl && \ cd /tmp/curl && \ ./buildconf && \ ./configure --disable-ldap --disable-shared --without-libssh2 \ --without-librtmp --without-libidn --enable-static \ - --with-pic --with-ssl && \ + --with-pic --with-ssl=/usr/local/ssl/ && \ make -j && make install && \ cd ~ && rm -rf /tmp/curl @@ -72,34 +81,19 @@ RUN mkdir /tmp/glog && cd /tmp/glog && \ make -j && make install && \ cd ~ && rm -rf /tmp/glog -# openssl -# This openssl (compiled with -fPIC) is used to statically link into the agent -# shared library. We still install libssl-dev above since getting libcurl to -# link with the custom version is tricky, see -# http://askubuntu.com/questions/475670/how-to-build-curl-with-the-latest-openssl. -# Also, intentionally not using -j as OpenSSL < 1.1.0 not quite supporting the -# parallel builds, see -# https://github.com/openssl/openssl/issues/5762#issuecomment-376622684. -RUN mkdir /tmp/openssl && cd /tmp/openssl && \ - curl -sL https://github.com/openssl/openssl/archive/OpenSSL_1_0_2t.tar.gz | \ - tar xzv --strip=1 && \ - ./config no-shared -fPIC --openssldir=/usr/local/ssl && \ - make && make install_sw && \ - cd ~ && rm -rf /tmp/openssl - # gRPC & protobuf # Use the protobuf version from gRPC for everything to avoid conflicting # versions to be linked in. Disable OpenSSL embedding: when it's on, the build # process of gRPC puts the OpenSSL static object files into the gRPC archive # which causes link errors later when the agent is linked with the static # OpenSSL library itself. -RUN git clone --depth=1 --recursive -b v1.25.0 https://github.com/grpc/grpc.git /tmp/grpc && \ +RUN git clone --depth=1 --recursive -b v1.28.1 https://github.com/grpc/grpc.git /tmp/grpc && \ cd /tmp/grpc && \ cd third_party/protobuf && \ ./autogen.sh && \ ./configure --with-pic CXXFLAGS="$(pkg-config --cflags protobuf)" LIBS="$(pkg-config --libs protobuf)" LDFLAGS="-Wl,--no-as-needed" && \ make -j && make install && ldconfig && \ cd ../.. && \ - CPPFLAGS="-I /usr/local/ssl/include" LDFLAGS="-Wl,--no-as-needed" make -j CONFIG=opt EMBED_OPENSSL=false V=1 HAS_SYSTEM_OPENSSL_NPN=0 && \ - CPPFLAGS="-I /usr/local/ssl/include" LDFLAGS="-Wl,--no-as-needed" make CONFIG=opt EMBED_OPENSSL=false V=1 HAS_SYSTEM_OPENSSL_NPN=0 install && \ + CPPFLAGS="-I /usr/local/ssl/include" LDFLAGS="-L /usr/local/ssl/lib/ -Wl,--no-as-needed" make -j CONFIG=opt EMBED_OPENSSL=false V=1 HAS_SYSTEM_OPENSSL_NPN=0 && \ + CPPFLAGS="-I /usr/local/ssl/include" LDFLAGS="-L /usr/local/ssl/lib/ -Wl,--no-as-needed" make CONFIG=opt EMBED_OPENSSL=false V=1 HAS_SYSTEM_OPENSSL_NPN=0 install && \ rm -rf /tmp/grpc From 5c4da063ae69e036baa9b9e62ac7487899fc6050 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Mon, 27 Apr 2020 11:18:42 -0700 Subject: [PATCH 08/11] Add Alpine support for Cloud Profiler Java agent PiperOrigin-RevId: 308660276 --- Dockerfile.alpine | 97 ++++++++++++++++++++++++++++ Makefile | 15 +++++ README.md | 12 ++++ build.sh | 22 +++++-- src/cloud_env.cc | 2 +- src/entry.cc | 22 +++++-- src/threads.cc | 5 ++ third_party/javaprofiler/accessors.h | 22 ++++--- 8 files changed, 175 insertions(+), 22 deletions(-) create mode 100644 Dockerfile.alpine diff --git a/Dockerfile.alpine b/Dockerfile.alpine new file mode 100644 index 000000000..9f510ca0a --- /dev/null +++ b/Dockerfile.alpine @@ -0,0 +1,97 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# +# Base image +# +FROM alpine:latest + +# +# Dependencies +# + +# Everything we can get through apt-get +RUN apk --no-cache add \ + autoconf \ + automake \ + cmake \ + curl \ + g++ \ + git \ + libexecinfo-dev \ + libexecinfo-static \ + libtool \ + linux-headers \ + make \ + nghttp2-static \ + python \ + unzip \ + zlib-dev + +# Install JDK 11 as sampling heap profiler depends on the new JVMTI APIs. + RUN apk --no-cache add openjdk11-jdk + +# openssl +# This openssl (compiled with -fPIC) is used to statically link into the agent +# shared library. +ENV PKG_CONFIG_PATH=/usr/local/ssl/lib/pkgconfig +ENV JAVA_PATH=/usr/lib/jvm/java-11-openjdk/ +RUN mkdir /tmp/openssl && cd /tmp/openssl && \ + curl -sL https://github.com/openssl/openssl/archive/OpenSSL_1_1_1f.tar.gz | \ + tar xzv --strip=1 && \ + ./config no-shared -fPIC --openssldir=/usr/local/ssl --prefix=/usr/local/ssl && \ + make && make install_sw && \ + cd ~ && rm -rf /tmp/openssl + +# curl +RUN git clone --depth=1 -b curl-7_66_0 https://github.com/curl/curl.git /tmp/curl && \ + cd /tmp/curl && \ + ./buildconf && \ + ./configure --disable-ldap --disable-shared --without-libssh2 \ + --without-librtmp --without-libidn --enable-static \ + --with-pic --with-ssl=/usr/local/ssl/ && \ + make -j && make install && \ + cd ~ && rm -rf /tmp/curl + +# gflags +RUN git clone --depth=1 -b v2.1.2 https://github.com/gflags/gflags.git /tmp/gflags && \ + cd /tmp/gflags && \ + mkdir build && cd build && \ + cmake -DCMAKE_CXX_FLAGS=-fpic -DGFLAGS_NAMESPACE=google .. && \ + make -j && make install && \ + cd ~ && rm -rf /tmp/gflags + +# google-glog +RUN mkdir /tmp/glog && cd /tmp/glog && \ + curl -sL https://github.com/google/glog/archive/v0.3.5.tar.gz | \ + tar xzv --strip=1 && LDFLAGS="-lexecinfo" ./configure --with-pic --enable-static && \ + make -j && make install && \ + cd ~ && rm -rf /tmp/glog + +# gRPC & protobuf +# Use the protobuf version from gRPC for everything to avoid conflicting +# versions to be linked in. Disable OpenSSL embedding: when it's on, the build +# process of gRPC puts the OpenSSL static object files into the gRPC archive +# which causes link errors later when the agent is linked with the static +# OpenSSL library itself. +RUN git clone --depth=1 --recursive -b v1.28.1 https://github.com/grpc/grpc.git /tmp/grpc && \ + cd /tmp/grpc && \ + cd third_party/protobuf && \ + ./autogen.sh && \ + ./configure --with-pic CXXFLAGS="$(pkg-config --cflags protobuf)" LIBS="$(pkg-config --libs protobuf)" LDFLAGS="-Wl,--no-as-needed" && \ + make -j && make install && ldconfig / && \ + cd ../.. && \ + CPPFLAGS="-I /usr/local/ssl/include" LDFLAGS="-L /usr/local/ssl/lib/ -Wl,--no-as-needed" make -j CONFIG=opt EMBED_OPENSSL=false V=1 HAS_SYSTEM_OPENSSL_NPN=0 && \ + CPPFLAGS="-I /usr/local/ssl/include" LDFLAGS="-L /usr/local/ssl/lib/ -Wl,--no-as-needed" make CONFIG=opt EMBED_OPENSSL=false V=1 HAS_SYSTEM_OPENSSL_NPN=0 install && \ + rm -rf /tmp/grpc diff --git a/Makefile b/Makefile index 554919846..159a9be80 100644 --- a/Makefile +++ b/Makefile @@ -174,6 +174,21 @@ GRPC_LIBS= \ $(LIB_ROOT_PATH)/lib/libgrpc.a \ $(LIB_ROOT_PATH)/lib/libgpr.a \ +# Detect if running on Alpine and modify various flags +ifeq ("$(wildcard /etc/alpine-release)","/etc/alpine-release") + # musl only supports global dynamic tls model, and is documented as + # async-signal-safe. See + # https://wiki.musl-libc.org/design-concepts.html#Thread-local-storage + # This selects global-dynamic TLS model in + # third_party/javaprofiler/accessors.h + CFLAGS += -DALPINE + + LIBS1 += /usr/lib/libexecinfo.a + + # libgcc is not installed by default on Alpine. + LDFLAGS += -static-libgcc +endif + all: \ $(TARGET_AGENT) \ $(TARGET_NOTICES) \ diff --git a/README.md b/README.md index 3c6f64b18..985167e8b 100644 --- a/README.md +++ b/README.md @@ -27,3 +27,15 @@ commands below. $ cd cloud-profiler-java $ ./build.sh ``` + +## To build for Alpine. + +**Per thread timers are not available on Alpine.** Since SIGEV_THREAD_ID is not +supported by `timer_create` on Alpine, per thread timers are not implemented and +the flag `-cprof_cpu_use_per_thread_timers` is ignored on this platform. + +```shell +$ git clone https://github.com/GoogleCloudPlatform/cloud-profiler-java.git +$ cd cloud-profiler-java +$ ./build.sh -a +``` diff --git a/build.sh b/build.sh index bc8f1c4a2..ba0985b33 100755 --- a/build.sh +++ b/build.sh @@ -22,8 +22,14 @@ set -o nounset # Command line arguments: [-d] # -d: specify the temporary directory for the build. -while getopts ":d:" opt; do +ALPINE_BUILD="0" +DOCKERFILE="Dockerfile" +FILE_SUFFIX="" +while getopts ":ad:" opt; do case $opt in + a) + ALPINE_BUILD="1" + ;; d) BUILD_TEMP_DIR=$OPTARG ;; @@ -57,8 +63,14 @@ trap "{ echo 'FAILED: see ${LOG_FILE} for details' ; exit 1; }" ERR mkdir -p "${BUILD_TEMP_DIR}" -PrintMessage "Building the builder Docker container..." -docker build -t cprof-agent-builder . >> "${LOG_FILE}" 2>&1 +if [[ "${ALPINE_BUILD}" = "1" ]]; then + PrintMessage "Building the builder Alpine Docker container..." + DOCKERFILE="Dockerfile.alpine" + FILE_SUFFIX="_alpine" +else + PrintMessage "Building the builder Docker container..." +fi + docker build -f "${DOCKERFILE}" -t cprof-agent-builder . >> "${LOG_FILE}" 2>&1 PrintMessage "Packaging the agent code..." mkdir -p "${BUILD_TEMP_DIR}"/build @@ -72,12 +84,12 @@ docker run -ti -v "${BUILD_TEMP_DIR}/build":/root/build \ >> "${LOG_FILE}" 2>&1 PrintMessage "Packaging the agent binaries..." -tar zcf "${BUILD_TEMP_DIR}"/profiler_java_agent.tar.gz \ +tar zcf "${BUILD_TEMP_DIR}"/profiler_java_agent${FILE_SUFFIX}.tar.gz \ -C "${BUILD_TEMP_DIR}"/build/.out \ NOTICES profiler_java_agent.so \ >> "${LOG_FILE}" 2>&1 -PrintMessage "Agent built and stored locally in: ${BUILD_TEMP_DIR}/profiler_java_agent.tar.gz" +PrintMessage "Agent built and stored locally in: ${BUILD_TEMP_DIR}/profiler_java_agent${FILE_SUFFIX}.tar.gz" trap - EXIT PrintMessage "SUCCESS" diff --git a/src/cloud_env.cc b/src/cloud_env.cc index 7a132fb72..034a3deaf 100644 --- a/src/cloud_env.cc +++ b/src/cloud_env.cc @@ -88,7 +88,7 @@ std::string GceMetadataRequest(HTTPRequest* req, const std::string& path) { } const char* Getenv(const std::string& var) { -#if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 17) +#if __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 17) || ALPINE == 1 return secure_getenv(var.c_str()); #else return __secure_getenv(var.c_str()); diff --git a/src/entry.cc b/src/entry.cc index c3e9947ed..24eec4333 100644 --- a/src/entry.cc +++ b/src/entry.cc @@ -26,7 +26,8 @@ DEFINE_bool(cprof_cpu_use_per_thread_timers, false, "when true, use per-thread CLOCK_THREAD_CPUTIME_ID timers; " - "only profiles Java threads, non-Java threads will be missed"); + "only profiles Java threads, non-Java threads will be missed. " + "This flag is ignored on Alpine."); DEFINE_bool(cprof_force_debug_non_safepoints, true, "when true, force DebugNonSafepoints flag by subscribing to the" "code generation events. This improves the accuracy of profiles," @@ -292,7 +293,16 @@ jint JNICALL Agent_OnLoad(JavaVM *vm, char *options, void *reserved) { // The process exit will free the memory. See comments to the variable on why. // Initialize before registering the JVMTI callbacks to avoid the unlikely // race of getting thread events before the thread table is born. +#ifdef ALPINE + // musl does not support SIGEV_THREAD_ID. Disable per thread timers. + if (FLAGS_cprof_cpu_use_per_thread_timers) { + LOG(WARNING) << "Per thread timers not available in Alpine. " + << "Ignoring '-cprof_cpu_use_per_thread_timers' flag."; + } + threads = new ThreadTable(false); +#else threads = new ThreadTable(FLAGS_cprof_cpu_use_per_thread_timers); +#endif if (!RegisterJvmti(jvmti)) { LOG(ERROR) << "Failed to enable JVMTI events. Continuing..."; @@ -304,21 +314,19 @@ jint JNICALL Agent_OnLoad(JavaVM *vm, char *options, void *reserved) { google::javaprofiler::Asgct::SetAsgct( google::javaprofiler::Accessors::GetJvmFunction< - google::javaprofiler::ASGCTType>("AsyncGetCallTrace")); + google::javaprofiler::ASGCTType>("AsyncGetCallTrace")); worker = new Worker(jvmti, threads); return 0; } -void JNICALL Agent_OnUnload(JavaVM *vm) { - IMPLICITLY_USE(vm); -} +void JNICALL Agent_OnUnload(JavaVM *vm) { IMPLICITLY_USE(vm); } } // namespace profiler } // namespace cloud -AGENTEXPORT jint JNICALL -Agent_OnLoad(JavaVM *vm, char *options, void *reserved) { +AGENTEXPORT jint JNICALL Agent_OnLoad(JavaVM *vm, char *options, + void *reserved) { return cloud::profiler::Agent_OnLoad(vm, options, reserved); } diff --git a/src/threads.cc b/src/threads.cc index 35212aa12..61ffdfd9d 100644 --- a/src/threads.cc +++ b/src/threads.cc @@ -27,6 +27,10 @@ namespace { const timer_t kInvalidTimer = reinterpret_cast(-1LL); timer_t CreateTimer(pid_t tid) { +#ifdef ALPINE + // Per thread timers are not available on Alpine. + return kInvalidTimer; +#else struct sigevent sevp = {}; sevp.sigev_notify = SIGEV_THREAD_ID; sevp._sigev_un._tid = tid; @@ -38,6 +42,7 @@ timer_t CreateTimer(pid_t tid) { return kInvalidTimer; } return timer; +#endif } bool SetTimer(timer_t timer, int64_t period_usec) { diff --git a/third_party/javaprofiler/accessors.h b/third_party/javaprofiler/accessors.h index 755e29f56..004bfe945 100644 --- a/third_party/javaprofiler/accessors.h +++ b/third_party/javaprofiler/accessors.h @@ -80,15 +80,19 @@ class Accessors { } private: - // This is dangerous. TLS accesses are by default not async safe, as - // they can call malloc for lazy initialization. The initial-exec - // TLS mode avoids this potential allocation, with the limitation - // that there is a fixed amount of space to hold all TLS variables - // referenced in the module. This should be OK for the cloud - // profiler agent, which is relatively small. We do provide a way - // to override the TLS model for compilation environments where the - // TLS access is async-safe. -#ifdef JAVAPROFILER_GLOBAL_DYNAMIC_TLS + // This is subtle and potentially dangerous, read this carefully. + // + // In glibc, TLS access is not signal-async-safe, as they can call malloc for + // lazy initialization. The initial-exec TLS mode avoids this potential + // allocation, with the limitation that there is a fixed amount of space to + // hold all TLS variables referenced in the module. This should be OK for the + // cloud profiler agent, which is relatively small. + // + // In environments where the TLS access is async-signal-safe, the + // global-dynamic TLS model can be used. For example, it is async-signal-safe + // in musl / Alpine, see + // https://wiki.musl-libc.org/design-concepts.html#Thread-local-storage. +#if defined(JAVAPROFILER_GLOBAL_DYNAMIC_TLS) || defined(ALPINE) static __thread JNIEnv *env_ __attribute__((tls_model("global-dynamic"))); static __thread int64 attr_ __attribute__((tls_model("global-dynamic"))); static __thread Tags *tags_ __attribute__((tls_model("global-dynamic"))); From fef3c5989b1a45230139f9fa3b0e57238ce959a1 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 7 May 2020 17:53:13 -0700 Subject: [PATCH 09/11] Internal change PiperOrigin-RevId: 310472078 --- README.md | 12 ++++++------ src/NOTICES | 3 +-- src/entry.cc | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 985167e8b..da82f8317 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ -# Stackdriver Profiler Java Agent +# Google Cloud Profiler Profiler Java Agent -This repository contains source code for the [Stackdriver -Profiler](https://cloud.google.com/profiler/) Java agent. +This repository contains source code for the +[Google Cloud Profiler Profiler](https://cloud.google.com/profiler/) Java agent. ## Installation @@ -12,9 +12,9 @@ wget -q -O- https://storage.googleapis.com/cloud-profiler/java/latest/profiler_j | sudo tar xzv -C /opt/cprof ``` -See the [Stackdriver Profiler Java profiling -doc](https://cloud.google.com/profiler/docs/profiling-java) for detailed and -most up-to-date guide on installing and using the agent. +See the +[Google Cloud Profiler Profiler Java profiling doc](https://cloud.google.com/profiler/docs/profiling-java) +for detailed and most up-to-date guide on installing and using the agent. ## Build from Source diff --git a/src/NOTICES b/src/NOTICES index b9819f05d..c02196203 100644 --- a/src/NOTICES +++ b/src/NOTICES @@ -1,4 +1,4 @@ -Google Stackdriver Profiler +Google Cloud Profiler Copyright Google Inc. Use of this software is governed by the Google Cloud Platform Terms of Service found at https://cloud.google.com/terms/ @@ -470,4 +470,3 @@ OpenSSL License * copied and put under another distribution licence * [including the GNU Public Licence.] */ - diff --git a/src/entry.cc b/src/entry.cc index 24eec4333..2809a648c 100644 --- a/src/entry.cc +++ b/src/entry.cc @@ -268,7 +268,7 @@ jint JNICALL Agent_OnLoad(JavaVM *vm, char *options, void *reserved) { ParseArguments(options); // Initializes logger -- do not log before this call - LOG(INFO) << "Stackdriver Profiler Java agent version: " + LOG(INFO) << "Google Cloud Profiler Java agent version: " << CLOUD_PROFILER_AGENT_VERSION; LOG(INFO) << "Profiler agent loaded"; google::javaprofiler::AttributeTable::Init(); From b9cc3780ac212bd69c6f44ba9936b3336dce0149 Mon Sep 17 00:00:00 2001 From: dthomson Date: Tue, 12 May 2020 17:38:26 -0700 Subject: [PATCH 10/11] Filter allocation frames from native stack traces using a regex. PiperOrigin-RevId: 311236078 --- .../javaprofiler/profile_proto_builder.cc | 13 +++--- .../javaprofiler/profile_proto_builder.h | 41 ++++--------------- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index 3b6753a51..1044c653c 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -286,6 +286,11 @@ void ProfileProtoBuilder::AddNativeInfo(const JVMPI_CallFrame &jvm_frame, } std::string function_name = native_cache_->GetFunctionName(jvm_frame); + + if (SkipFrame(function_name)) { + return; + } + perftools::profiles::Location *location = native_cache_->GetLocation(jvm_frame, &location_builder_); @@ -457,14 +462,6 @@ std::unique_ptr ProfileProtoBuilder::ForHeap( new HeapProfileProtoBuilder(jni_env, jvmti_env, sampling_rate, cache)); } -std::unique_ptr ProfileProtoBuilder::ForNativeHeap( - JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64 sampling_rate, - ProfileFrameCache *cache) { - assert(cache != nullptr); - return std::unique_ptr(new NativeHeapProfileProtoBuilder( - jni_env, jvmti_env, sampling_rate, cache)); -} - std::unique_ptr ProfileProtoBuilder::ForCpu( JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64 duration_ns, int64 sampling_rate, ProfileFrameCache *cache) { diff --git a/third_party/javaprofiler/profile_proto_builder.h b/third_party/javaprofiler/profile_proto_builder.h index eac0b324d..2dcf1aa81 100644 --- a/third_party/javaprofiler/profile_proto_builder.h +++ b/third_party/javaprofiler/profile_proto_builder.h @@ -158,20 +158,15 @@ class ProfileProtoBuilder { virtual std::unique_ptr CreateProto() = 0; // Create a heap profile. - // jvmti_env can be null as well but then all calls to AddTraces will return + // jvmti_env can be null but then all calls to AddTraces will return // unknown. - // ForHeap/ForNativeHeap is the only case where we accept a null cache since - // the heap profiles can be using JVMTI's GetStackTrace and remain in pure - // Java land frames. Other ForX methods will fail an assertion when attempting - // a nullptr cache. + // Accepts a null cache since the heap profiles can be using JVMTI's + // GetStackTrace and remain in pure Java land frames. Other ForX methods + // will fail an assertion when attempting a nullptr cache. static std::unique_ptr ForHeap( JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64 sampling_rate, ProfileFrameCache *cache = nullptr); - static std::unique_ptr ForNativeHeap( - JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64 sampling_rate, - ProfileFrameCache *cache = nullptr); - static std::unique_ptr ForCpu(JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64 duration_ns, @@ -200,6 +195,10 @@ class ProfileProtoBuilder { const SampleType &metric_type, const std::list &label_types); + virtual bool SkipFrame(const std::string &function_name) const { + return false; + } + // An implementation must decide how many frames to skip in a trace. virtual int SkipTopNativeFrames(const JVMPI_CallTrace &trace) = 0; @@ -360,30 +359,6 @@ class HeapProfileProtoBuilder : public ProfileProtoBuilder { } }; -class NativeHeapProfileProtoBuilder : public HeapProfileProtoBuilder { - public: - NativeHeapProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, - int64 sampling_rate, ProfileFrameCache *cache) - : HeapProfileProtoBuilder(jni_env, jvmti_env, sampling_rate, cache) {} - - protected: - // In cases of long native frames, we really only want to skip the - // frames_to_skip first frames, which would be something akin to: - // absl::base_internal::MallocHook::InvokeNewHookSlow - // absl::base_internal::MallocHook::InvokeNewHook - // calloc - int SkipTopNativeFrames(const JVMPI_CallTrace &trace) override { - // If frames_to_skip changes, change the number of frames checked against - // kNativeFrameLineNum below to check the correct number of frames. - const int frames_to_skip = 3; - return (trace.num_frames >= frames_to_skip - && trace.frames[0].lineno == kNativeFrameLineNum - && trace.frames[1].lineno == kNativeFrameLineNum - && trace.frames[2].lineno == kNativeFrameLineNum) - ? frames_to_skip : 0; - } -}; - class ContentionProfileProtoBuilder : public ProfileProtoBuilder { public: ContentionProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, From 05f441cbba463124c656eacf6a413e49fd8bc4a0 Mon Sep 17 00:00:00 2001 From: nolanmar Date: Mon, 1 Jun 2020 12:00:22 -0700 Subject: [PATCH 11/11] Remove python dependency from Dockerfiles and pin alpine Dockerfile to 3.11. PiperOrigin-RevId: 314174911 --- Dockerfile | 1 - Dockerfile.alpine | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index 626a09d90..baf2b6b25 100644 --- a/Dockerfile +++ b/Dockerfile @@ -41,7 +41,6 @@ RUN apt-get update && apt-get -y -q install \ libtool \ make \ openjdk-11-jdk-headless \ - python \ unzip \ zlib1g-dev diff --git a/Dockerfile.alpine b/Dockerfile.alpine index 9f510ca0a..80bcb78de 100644 --- a/Dockerfile.alpine +++ b/Dockerfile.alpine @@ -15,7 +15,7 @@ # # Base image # -FROM alpine:latest +FROM alpine:3.11 # # Dependencies @@ -35,7 +35,6 @@ RUN apk --no-cache add \ linux-headers \ make \ nghttp2-static \ - python \ unzip \ zlib-dev