From a1f24cd51880389d347c62547e01ba04a2f92c9c Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Fri, 19 Apr 2024 20:59:41 +0000 Subject: [PATCH 1/8] Fix lambda naming handling for Java profiles. Lambda naming format has changed in JDK 21. - JDK 11: com.google.something.Something$$Lambda$197.1849072452.run. - JDK 21+: com.google.something.Something$$Lambda.0x00007ff38d3c11f8.run. This fixes various profiles, so that the same lambda is aggregated across different processes of the same binary. PiperOrigin-RevId: 626462690 --- third_party/javaprofiler/stacktrace_fixer.cc | 60 ++++++++++++++------ 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/third_party/javaprofiler/stacktrace_fixer.cc b/third_party/javaprofiler/stacktrace_fixer.cc index 417fe6741..5e461b41f 100644 --- a/third_party/javaprofiler/stacktrace_fixer.cc +++ b/third_party/javaprofiler/stacktrace_fixer.cc @@ -15,6 +15,8 @@ #include #include +#include +#include #include "third_party/javaprofiler/stacktrace_fixer.h" @@ -41,6 +43,9 @@ void SimplifySuffixedName(std::string *name, const char (&trigger)[M], } } +constexpr char digits[] = "0123456789"; +constexpr char hexdigits[] = "0123456789abcdef"; + // Simplifies the name of a method in a dynamic class (with '$$FastClassBy*$$' // 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 @@ -48,15 +53,16 @@ void SimplifySuffixedName(std::string *name, const char (&trigger)[M], void SimplifyDynamicClassName(std::string *name) { // Replace $$[0-9a-f]+ by $$ to remove unique values, for example in // $FastClassByCGLIB$$fd6bdf6d.invoke. - SimplifySuffixedName(name, "$$", "0123456789abcdef"); + SimplifySuffixedName(name, "$$", hexdigits); } -// 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. +// Simplifies the name of a lambda method to remove unique values. +// For JDK 11 and older, replaces $$Lambda$[0-9]+\.[0-9]+ with $$Lambda$. +// For example: com.google.something.Something$$Lambda$197.1849072452.run. +// For JDK 21 and newer, replaces $$Lambda\.0x[0-9a-f]+ with $$Lambda. +// For example: com.google.something.Something$$Lambda.0x00007ff38d3c11f8.run. void SimplifyLambdaName(std::string *name) { - constexpr char trigger[] = "$$Lambda$"; - constexpr char digits[] = "0123456789"; + constexpr char trigger[] = "$$Lambda"; const size_t trigger_length = strlen(trigger); // Assume and handle just one instance of a $$Lambda$ pattern. @@ -65,18 +71,42 @@ void SimplifyLambdaName(std::string *name) { return; } first += trigger_length; - if (first >= name->size() || !isdigit((*name)[first])) { + if (first >= name->size()) { return; } - size_t last = name->find_first_not_of(digits, first); - if (last == std::string::npos || (*name)[last] != '.') { - return; + bool is_pre_jdk21 = false; + if ((*name)[first] == '$') { + is_pre_jdk21 = true; + first++; } - last++; // skip the dot - if (last >= name->size() || !isdigit((*name)[last])) { - return; + + size_t last; + if (is_pre_jdk21) { + if (first >= name->size() || !std::isdigit((*name)[first])) { + return; + } + last = name->find_first_not_of(digits, first); + if (last == std::string::npos || (*name)[last] != '.') { + return; + } + last++; // skip the dot + if (last >= name->size() || !std::isdigit((*name)[last])) { + return; + } + last = name->find_first_not_of(digits, last); + } else { + // Skip '0x' + if (first + 3 >= name->size() || (*name)[first] != '.' || + (*name)[first + 1] != '0' || (*name)[first + 2] != 'x') { + return; + } + last = name->find_first_not_of(hexdigits, first + 3); + // check for '.' after the 0x... address + if (last != std::string::npos && (*name)[last] != '.') { + return; + } } - last = name->find_first_not_of(digits, last); + if (last == std::string::npos) { name->erase(first); return; @@ -84,8 +114,6 @@ void SimplifyLambdaName(std::string *name) { name->erase(first, last - first); } -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. From 8f9da9f52fe0d6566f36d7d71a9e1080ce9fc01d Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Wed, 24 Apr 2024 07:12:31 +0000 Subject: [PATCH 2/8] Automated Code Change PiperOrigin-RevId: 627629848 --- src/cloud_env.cc | 4 ++-- src/string.cc | 2 +- src/worker.cc | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cloud_env.cc b/src/cloud_env.cc index 1a04c49d9..3c93f51f1 100644 --- a/src/cloud_env.cc +++ b/src/cloud_env.cc @@ -161,7 +161,7 @@ std::string CloudEnv::ProjectID(HTTPRequest* req) { return resp; } - project_id_ = resp; + project_id_ = std::move(resp); return project_id_; } @@ -218,7 +218,7 @@ std::string CloudEnv::Oauth2AccessToken(HTTPRequest* req) { continue; } if (pair[0] == "access_token") { - return pair[1]; + return std::move(pair[1]); } } LOG(ERROR) << "Could not parse access token out of '" << resp << "'"; diff --git a/src/string.cc b/src/string.cc index 229212ad2..7db47be44 100644 --- a/src/string.cc +++ b/src/string.cc @@ -47,7 +47,7 @@ bool ParseKeyValueList(const std::string& s, if (k.empty()) { return false; } - (*out)[k] = kv.substr(pos + 1); + (*out)[std::move(k)] = kv.substr(pos + 1); } return true; } diff --git a/src/worker.cc b/src/worker.cc index a2ba23854..d32229e4d 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -215,7 +215,7 @@ void Worker::ProfileThread(jvmtiEnv *jvmti_env, JNIEnv *jni_env, void *arg) { LOG(ERROR) << "No profile bytes collected, skipping the upload"; continue; } - if (!w->throttler_->Upload(profile)) { + if (!w->throttler_->Upload(std::move(profile))) { LOG(ERROR) << "Error on profile upload, discarding the profile"; } } From a546a18540d4fd0e6aa5594b1a9db448bf06b4fa Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Sat, 11 May 2024 07:30:27 +0000 Subject: [PATCH 3/8] Automated Code Change PiperOrigin-RevId: 632717546 --- src/cloud_env.cc | 1 + src/worker.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/src/cloud_env.cc b/src/cloud_env.cc index 3c93f51f1..5cd0d8f8f 100644 --- a/src/cloud_env.cc +++ b/src/cloud_env.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include "src/clock.h" diff --git a/src/worker.cc b/src/worker.cc index d32229e4d..7153171e6 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "src/clock.h" From e60a4c6d24f744f85cda7f449c9014bbe610f948 Mon Sep 17 00:00:00 2001 From: dthomson Date: Tue, 4 Jun 2024 23:02:31 +0000 Subject: [PATCH 4/8] Internal change PiperOrigin-RevId: 640312912 --- third_party/javaprofiler/heap_sampler.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index ac7f823da..0524476c1 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -35,7 +35,7 @@ namespace { using google::javaprofiler::JVMPI_CallFrame; std::vector TransformFrames(jvmtiFrameInfo *stack_frames, - int count) { + jint count) { std::vector frames(count); for (int i = 0; i < count; i++) { From 44bdb87f6001dee6933fc73ba28633d3d1f20215 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Wed, 12 Jun 2024 09:42:28 +0000 Subject: [PATCH 5/8] Leaving this comment so that when this code migrates to the latest version of gRPC, the action items are clear. The build will break. And when the build breaks the marked pieces of code can be safely deleted. PiperOrigin-RevId: 642554063 --- src/throttler_api.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/throttler_api.cc b/src/throttler_api.cc index 39f1a24b2..0e0245588 100644 --- a/src/throttler_api.cc +++ b/src/throttler_api.cc @@ -279,6 +279,12 @@ APIThrottler::APIThrottler( closed_(false), backing_off_for_testing_(false) { grpc_init(); + // Note from the gRPC team : + // gpr_set_log_function will work only till gRPC version 1.64. Versions after + // that do not support gpr_set_log_function. However since your function + // GRPCLog is equivalent to what gRPC is doing internally, it would be safe to + // delete this line and the entire GRPCLog function when you upgrade gRPC to + // version 1.65 or later. gpr_set_log_function(GRPCLog); // Create a random number generator. From 096bf6d228f17208b459f219a734c702b4822ecb Mon Sep 17 00:00:00 2001 From: dthomson Date: Thu, 13 Jun 2024 20:56:39 +0000 Subject: [PATCH 6/8] Factor out the MemoryInfo cache as a class. PiperOrigin-RevId: 643112333 --- third_party/javaprofiler/method_info.cc | 20 +++-- third_party/javaprofiler/method_info.h | 32 +++---- .../javaprofiler/profile_proto_builder.cc | 87 +++++++++---------- .../javaprofiler/profile_proto_builder.h | 40 ++++++--- 4 files changed, 95 insertions(+), 84 deletions(-) diff --git a/third_party/javaprofiler/method_info.cc b/third_party/javaprofiler/method_info.cc index 89017c85d..688c17164 100644 --- a/third_party/javaprofiler/method_info.cc +++ b/third_party/javaprofiler/method_info.cc @@ -16,17 +16,27 @@ #include "third_party/javaprofiler/method_info.h" +#include + #include "third_party/javaprofiler/display.h" +#include "third_party/javaprofiler/stacktrace_decls.h" namespace google { namespace javaprofiler { -int64_t MethodInfo::Location(int line_number) { - auto it = locations_.find(line_number); - if (it == locations_.end()) { - return kInvalidLocationId; +int64_t MethodInfo::LineNumber(const JVMPI_CallFrame &frame) { + // lineno is actually the BCI of the frame. + int bci = frame.lineno; + + auto it = line_numbers_.find(bci); + if (it != line_numbers_.end()) { + return it->second; } - return it->second; + + int line_number = GetLineNumber(jvmti_env_, frame.method_id, bci); + + line_numbers_[bci] = line_number; + return line_number; } } // namespace javaprofiler diff --git a/third_party/javaprofiler/method_info.h b/third_party/javaprofiler/method_info.h index 4481d66d1..9eca8494a 100644 --- a/third_party/javaprofiler/method_info.h +++ b/third_party/javaprofiler/method_info.h @@ -18,10 +18,10 @@ #define GOOGLE_JAVAPROFILER_METHOD_INFO_H_ #include +#include +#include #include -#include "perftools/profiles/proto/builder.h" -#include "third_party/javaprofiler/globals.h" #include "third_party/javaprofiler/stacktrace_decls.h" namespace google { @@ -34,24 +34,18 @@ namespace javaprofiler { class MethodInfo { public: // Constructor providing all the information regarding a method. - MethodInfo(const std::string &method_name, const std::string &class_name, - const std::string &file_name, const int &start_line) - : method_name_(method_name), + MethodInfo(jvmtiEnv *jvmti_env, + const std::string &method_name, + const std::string &class_name, + const std::string &file_name, + const int &start_line) + : jvmti_env_(jvmti_env), + method_name_(method_name), class_name_(class_name), file_name_(file_name), start_line_(start_line) {} - // An invalid location Id. - static const int64_t kInvalidLocationId = 0; - - // Return the location representing info, returns kInvalidLocationId - // if not found. - int64_t Location(int line_number); - - // Put a location ID assocated to a ByteCode Index (BCI). - void AddLocation(int bci, int64_t location_id) { - locations_[bci] = location_id; - } + int64_t LineNumber(const JVMPI_CallFrame &frame); const std::string &MethodName() const { return method_name_; } @@ -59,16 +53,18 @@ class MethodInfo { const std::string &FileName() const { return file_name_; } - const int& StartLine() const { return start_line_; } + int StartLine() const { return start_line_; } private: + jvmtiEnv *jvmti_env_; + std::string method_name_; std::string class_name_; std::string file_name_; int start_line_; // Cache of jlocation results. - std::unordered_map locations_; + std::unordered_map line_numbers_; }; } // namespace javaprofiler diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index b5629fdf4..ec372f0a9 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -16,19 +16,23 @@ #include "third_party/javaprofiler/stacktrace_fixer.h" #include "third_party/javaprofiler/profile_proto_builder.h" +using std::string; using std::unique_ptr; namespace google { namespace javaprofiler { + namespace { + // Hashes a string using an initial hash value of h. -size_t HashString(const std::string &str, size_t h) { - hash hash_string; +size_t HashString(const string &str, size_t h) { + hash hash_string; return 31U * h + hash_string(str); } const int kCount = 0; const int kMetric = 1; + } // namespace ProfileProtoBuilder::ProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, @@ -39,6 +43,7 @@ ProfileProtoBuilder::ProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, : sampling_rate_(sampling_rate), jni_env_(jni_env), jvmti_env_(jvmti_env), + method_info_cache_(jni_env, jvmti_env), native_cache_(native_cache), location_builder_(&builder_) { AddSampleType(count_type); @@ -71,7 +76,7 @@ void ProfileProtoBuilder::AddTraces(const ProfileStackTrace *traces, } } -void ProfileProtoBuilder::AddArtificialTrace(const std::string &name, int count, +void ProfileProtoBuilder::AddArtificialTrace(const string &name, int count, int sampling_rate) { perftools::profiles::Location *location = location_builder_.LocationFor( name, name, "", 0, -1, 0); @@ -202,46 +207,35 @@ void ProfileProtoBuilder::AddJavaInfo( return; } - MethodInfo *method_info = Method(jvm_frame.method_id); - sample->add_location_id(Location(method_info, jvm_frame)); -} - -int64_t ProfileProtoBuilder::Location(MethodInfo *method, - const JVMPI_CallFrame &frame) { - // lineno is actually the BCI of the frame. - int bci = frame.lineno; - - int64_t location_id = method->Location(bci); + MethodInfo *method_info = method_info_cache_.Method(jvm_frame.method_id); - // Non-zero as a location id is a valid location ID. - if (location_id != MethodInfo::kInvalidLocationId) { - return location_id; - } - - int line_number = GetLineNumber(jvmti_env_, frame.method_id, bci); + int64_t line_number = method_info->LineNumber(jvm_frame); perftools::profiles::Location *location = - location_builder_.LocationFor(method->ClassName(), - method->MethodName(), - method->FileName(), - method->StartLine(), + location_builder_.LocationFor(method_info->ClassName(), + method_info->MethodName(), + method_info->FileName(), + method_info->StartLine(), line_number, 0); - method->AddLocation(bci, location->id()); - return location->id(); + sample->add_location_id(location->id()); +} + +MethodInfoCache::MethodInfoCache(JNIEnv *jni_env, jvmtiEnv *jvmti_env) + : jni_env_(jni_env), jvmti_env_(jvmti_env) { } -MethodInfo *ProfileProtoBuilder::Method(jmethodID method_id) { +MethodInfo *MethodInfoCache::Method(jmethodID method_id) { auto it = methods_.find(method_id); if (it != methods_.end()) { return it->second.get(); } - std::string file_name; - std::string class_name; - std::string method_name; - std::string signature; + string file_name; + string class_name; + string method_name; + string signature; int start_line = 0; // Set lineno to zero to get method first line. @@ -250,10 +244,11 @@ MethodInfo *ProfileProtoBuilder::Method(jmethodID method_id) { &class_name, &method_name, &signature, &start_line); FixMethodParameters(&signature); - std::string full_method_name = class_name + "." + method_name + signature; + string full_method_name = class_name + "." + method_name + signature; - std::unique_ptr unique_method( - new MethodInfo(full_method_name, class_name, file_name, start_line)); + unique_ptr unique_method( + new MethodInfo(jvmti_env_, full_method_name, class_name, file_name, + start_line)); auto method_ptr = unique_method.get(); methods_[method_id] = std::move(unique_method); @@ -271,16 +266,14 @@ void ProfileProtoBuilder::AddNativeInfo(const JVMPI_CallFrame &jvm_frame, return; } - std::string function_name = native_cache_->GetFunctionName(jvm_frame); + string function_name = native_cache_->GetFunctionName(jvm_frame); if (SkipFrame(function_name)) { return; } perftools::profiles::Location *location = - native_cache_->GetLocation(jvm_frame, - &location_builder_); - + native_cache_->GetLocation(jvm_frame, &location_builder_); sample->add_location_id(location->id()); } @@ -305,7 +298,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); @@ -327,8 +320,8 @@ bool LocationBuilder::LocationInfoEquals::operator()( } perftools::profiles::Location *LocationBuilder::LocationFor( - const std::string &class_name, const std::string &function_name, - const std::string &file_name, int start_line, int line_number, + const string &class_name, const string &function_name, + const string &file_name, int start_line, int line_number, int64_t address) { auto profile = builder_->mutable_profile(); @@ -361,7 +354,7 @@ perftools::profiles::Location *LocationBuilder::LocationFor( auto line = location->add_line(); - std::string simplified_name = function_name; + string simplified_name = function_name; SimplifyFunctionName(&simplified_name); auto function_id = builder_->FunctionId(simplified_name.c_str(), function_name.c_str(), @@ -489,31 +482,31 @@ double CalculateSamplingRatio(int64_t rate, int64_t count, return 1 / (1 - exp(-size / r)); } -std::unique_ptr ProfileProtoBuilder::ForHeap( +unique_ptr ProfileProtoBuilder::ForHeap( JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64_t sampling_rate, ProfileFrameCache *cache) { // Cache can be nullptr because the heap sampler can be using a JVMTI // Java-only stackframe gatherer. - return std::unique_ptr( + return unique_ptr( new HeapProfileProtoBuilder(jni_env, jvmti_env, sampling_rate, cache)); } -std::unique_ptr ProfileProtoBuilder::ForCpu( +unique_ptr ProfileProtoBuilder::ForCpu( JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64_t duration_ns, int64_t sampling_rate, ProfileFrameCache *cache) { CHECK (cache != nullptr) << "CPU profiles may have native frames, cache must be provided"; - return std::unique_ptr( + return unique_ptr( new CpuProfileProtoBuilder(jni_env, jvmti_env, duration_ns, sampling_rate, cache)); } -std::unique_ptr ProfileProtoBuilder::ForContention( +unique_ptr ProfileProtoBuilder::ForContention( JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64_t sampling_rate, int64_t duration_nanos, ProfileFrameCache *cache) { CHECK (cache != nullptr) << "Contention profiles may have native frames, cache must be provided"; - return std::unique_ptr(new ContentionProfileProtoBuilder( + return unique_ptr(new ContentionProfileProtoBuilder( jni_env, jvmti_env, sampling_rate, duration_nanos, cache)); } diff --git a/third_party/javaprofiler/profile_proto_builder.h b/third_party/javaprofiler/profile_proto_builder.h index 29ac9215c..d2c93895a 100644 --- a/third_party/javaprofiler/profile_proto_builder.h +++ b/third_party/javaprofiler/profile_proto_builder.h @@ -211,6 +211,31 @@ class ProfileFrameCache { virtual ~ProfileFrameCache() {} }; +// Caching jmethodID resolution: +// - This allows a one-time calculation of a given jmethodID during proto +// creation. +// - The cache reduces the number of JVMTI calls to symbolize the stacks. +// - jmethodIDs are never invalid per se or re-used: if ever the jmethodID's +// class is unloaded, the JVMTI calls will return an error code that +// caught by the various JVMTI calls done. +// Though it would theoretically be possible to cache the jmethodID for the +// lifetime of the program, this just implementation keeps the cache live +// during this proto creation. The reason is that jmethodIDs might become +// stale/unloaded and there would need to be extra work to determine +// cache-size management. +class MethodInfoCache { + public: + MethodInfoCache(JNIEnv *jni_env, jvmtiEnv *jvmti_env); + + MethodInfo *Method(jmethodID id); + + private: + JNIEnv *jni_env_; + jvmtiEnv *jvmti_env_; + + std::unordered_map> methods_; +}; + // Create profile protobufs from traces obtained from JVM profiling. class ProfileProtoBuilder { public: @@ -306,7 +331,6 @@ class ProfileProtoBuilder { perftools::profiles::Sample *sample); void UnsampleMetrics(); - MethodInfo *Method(jmethodID id); int64_t Location(MethodInfo *method, const JVMPI_CallFrame &frame); void AddLabels(const TraceAndLabels &trace_and_labels, @@ -315,19 +339,7 @@ class ProfileProtoBuilder { JNIEnv *jni_env_; jvmtiEnv *jvmti_env_; - // Caching jmethodID resolution: - // - This allows a one-time calculation of a given jmethodID during proto - // creation. - // - The cache reduces the number of JVMTI calls to symbolize the stacks. - // - jmethodIDs are never invalid per se or re-used: if ever the jmethodID's - // class is unloaded, the JVMTI calls will return an error code that - // caught by the various JVMTI calls done. - // Though it would theoretically be possible to cache the jmethodID for the - // lifetime of the program, this just implementation keeps the cache live - // during this proto creation. The reason is that jmethodIDs might become - // stale/unloaded and there would need to be extra work to determine - // cache-size management. - std::unordered_map> methods_; + MethodInfoCache method_info_cache_; ProfileFrameCache *native_cache_; TraceSamples trace_samples_; LocationBuilder location_builder_; From 96bb5e80df96238604ed70638a57fdf24d16a430 Mon Sep 17 00:00:00 2001 From: dthomson Date: Mon, 24 Jun 2024 17:07:21 +0000 Subject: [PATCH 7/8] Move skip frames functionality into the base class ProfileProtoBuilder. PiperOrigin-RevId: 646134895 --- .../javaprofiler/profile_proto_builder.cc | 39 +++++++++-- .../javaprofiler/profile_proto_builder.h | 66 ++++++++----------- 2 files changed, 62 insertions(+), 43 deletions(-) diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index ec372f0a9..34cfcc4cf 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -18,6 +18,7 @@ using std::string; using std::unique_ptr; +using std::vector; namespace google { namespace javaprofiler { @@ -39,13 +40,17 @@ ProfileProtoBuilder::ProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, ProfileFrameCache *native_cache, int64_t sampling_rate, const SampleType &count_type, - const SampleType &metric_type) + const SampleType &metric_type, + bool skip_top_native_frames, + vector skip_frames) : sampling_rate_(sampling_rate), jni_env_(jni_env), jvmti_env_(jvmti_env), method_info_cache_(jni_env, jvmti_env), native_cache_(native_cache), - location_builder_(&builder_) { + location_builder_(&builder_), + skip_top_native_frames_(skip_top_native_frames) + { AddSampleType(count_type); AddSampleType(metric_type); builder_.mutable_profile()->set_default_sample_type( @@ -103,6 +108,29 @@ void ProfileProtoBuilder::UnsampleMetrics() { } } +bool ProfileProtoBuilder::SkipFrame( + const string &function_name) const { + bool result = false; + return result; +} + +int ProfileProtoBuilder::SkipTopNativeFrames(const JVMPI_CallTrace &trace) { + if (!skip_top_native_frames_) { + return 0; + } + + // Skip until we see the first Java frame. + for (int i = 0; i < trace.num_frames; i++) { + if (trace.frames[i].lineno != kNativeFrameLineNum) { + return i; + } + } + + // All are native is weird for Java heap samples but do nothing in this + // case. + return 0; +} + unique_ptr ProfileProtoBuilder::CreateSampledProto() { #ifndef STANDALONE_BUILD @@ -488,7 +516,8 @@ unique_ptr ProfileProtoBuilder::ForHeap( // Cache can be nullptr because the heap sampler can be using a JVMTI // Java-only stackframe gatherer. return unique_ptr( - new HeapProfileProtoBuilder(jni_env, jvmti_env, sampling_rate, cache)); + new HeapProfileProtoBuilder(jni_env, jvmti_env, sampling_rate, cache, + true, {})); } unique_ptr ProfileProtoBuilder::ForCpu( @@ -498,7 +527,7 @@ unique_ptr ProfileProtoBuilder::ForCpu( << "CPU profiles may have native frames, cache must be provided"; return unique_ptr( new CpuProfileProtoBuilder(jni_env, jvmti_env, duration_ns, - sampling_rate, cache)); + sampling_rate, cache, false, {})); } unique_ptr ProfileProtoBuilder::ForContention( @@ -507,7 +536,7 @@ unique_ptr ProfileProtoBuilder::ForContention( CHECK (cache != nullptr) << "Contention profiles may have native frames, cache must be provided"; return unique_ptr(new ContentionProfileProtoBuilder( - jni_env, jvmti_env, sampling_rate, duration_nanos, cache)); + jni_env, jvmti_env, sampling_rate, duration_nanos, cache, false, {})); } } // namespace javaprofiler diff --git a/third_party/javaprofiler/profile_proto_builder.h b/third_party/javaprofiler/profile_proto_builder.h index d2c93895a..487f917c8 100644 --- a/third_party/javaprofiler/profile_proto_builder.h +++ b/third_party/javaprofiler/profile_proto_builder.h @@ -25,6 +25,7 @@ #include #include #include +#include #include "perftools/profiles/proto/builder.h" #include "third_party/javaprofiler/method_info.h" @@ -295,14 +296,9 @@ class ProfileProtoBuilder { ProfileProtoBuilder(JNIEnv *env, jvmtiEnv *jvmti_env, ProfileFrameCache *native_cache, int64_t sampling_rate, const SampleType &count_type, - const SampleType &metric_type); - - 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; + const SampleType &metric_type, + bool skip_top_native_frames, + std::vector skip_frames); // Build the proto, unsampling the sample metrics. Calling any other method // on the class after calling this has undefined behavior. @@ -316,6 +312,8 @@ class ProfileProtoBuilder { int64_t sampling_rate_ = 0; private: + bool SkipFrame(const std::string &function_name) const; + int SkipTopNativeFrames(const JVMPI_CallTrace &trace); void AddSampleType(const SampleType &sample_type); void SetPeriodType(const SampleType &metric_type); void InitSampleValues(perftools::profiles::Sample *sample, int64_t count, @@ -343,6 +341,8 @@ class ProfileProtoBuilder { ProfileFrameCache *native_cache_; TraceSamples trace_samples_; LocationBuilder location_builder_; + + bool skip_top_native_frames_; }; // Computes the ratio to use to scale heap data to unsample it. @@ -358,11 +358,15 @@ class CpuProfileProtoBuilder : public ProfileProtoBuilder { public: CpuProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64_t duration_ns, int64_t sampling_rate, - ProfileFrameCache *cache) + ProfileFrameCache *cache, + bool skip_top_native_frames, + std::vector skip_frames) : ProfileProtoBuilder( jni_env, jvmti_env, cache, sampling_rate, ProfileProtoBuilder::SampleType("samples", "count"), - ProfileProtoBuilder::SampleType("cpu", "nanoseconds")) { + ProfileProtoBuilder::SampleType("cpu", "nanoseconds"), + skip_top_native_frames, + skip_frames) { builder_.mutable_profile()->set_duration_nanos(duration_ns); builder_.mutable_profile()->set_period(sampling_rate); } @@ -370,52 +374,41 @@ class CpuProfileProtoBuilder : public ProfileProtoBuilder { std::unique_ptr CreateProto() override { return CreateSampledProto(); } - - protected: - int SkipTopNativeFrames(const JVMPI_CallTrace &trace) override { return 0; } }; class HeapProfileProtoBuilder : public ProfileProtoBuilder { public: HeapProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, - int64_t sampling_rate, ProfileFrameCache *cache) + int64_t sampling_rate, ProfileFrameCache *cache, + bool skip_top_native_frames, + std::vector skip_frames) : ProfileProtoBuilder( jni_env, jvmti_env, cache, sampling_rate, ProfileProtoBuilder::SampleType("inuse_objects", "count"), - ProfileProtoBuilder::SampleType("inuse_space", "bytes")) {} + ProfileProtoBuilder::SampleType("inuse_space", "bytes"), + skip_top_native_frames, + skip_frames) + { + } std::unique_ptr CreateProto() override { return CreateUnsampledProto(); } - - protected: - // Depending on the JDK or how we got the frames (e.g. internal JVM or - // JVMTI), we might have native frames on top of the Java frames - // (basically where the JVM internally allocated the object). - // Returns the first Java frame index or 0 if none were found. - int SkipTopNativeFrames(const JVMPI_CallTrace &trace) override { - // Skip until we see the first Java frame. - for (int i = 0; i < trace.num_frames; i++) { - if (trace.frames[i].lineno != kNativeFrameLineNum) { - return i; - } - } - - // All are native is weird for Java heap samples but do nothing in this - // case. - return 0; - } }; class ContentionProfileProtoBuilder : public ProfileProtoBuilder { public: ContentionProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64_t duration_nanos, int64_t sampling_rate, - ProfileFrameCache *cache) + ProfileFrameCache *cache, + bool skip_top_native_frames, + std::vector skip_frames) : ProfileProtoBuilder( jni_env, jvmti_env, cache, sampling_rate, ProfileProtoBuilder::SampleType("contentions", "count"), - ProfileProtoBuilder::SampleType("delay", "microseconds")) { + ProfileProtoBuilder::SampleType("delay", "microseconds"), + skip_top_native_frames, + skip_frames) { builder_.mutable_profile()->set_duration_nanos(duration_nanos); builder_.mutable_profile()->set_period(sampling_rate); builder_.mutable_profile()->set_default_sample_type( @@ -428,9 +421,6 @@ class ContentionProfileProtoBuilder : public ProfileProtoBuilder { return std::unique_ptr(builder_.Consume()); } - protected: - int SkipTopNativeFrames(const JVMPI_CallTrace &trace) override { return 0; } - private: // Multiply the (count, metric) values by the sampling_rate. void MultiplyBySamplingRate(); From 567bd91ea77f481a59f37cc2bc25c492549551ba Mon Sep 17 00:00:00 2001 From: dthomson Date: Wed, 26 Jun 2024 20:53:45 +0000 Subject: [PATCH 8/8] Allow native symbolization in profiles to be disabled. PiperOrigin-RevId: 647067972 --- .../javaprofiler/profile_proto_builder.cc | 22 +++++++++++-------- .../javaprofiler/profile_proto_builder.h | 5 +++++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index 34cfcc4cf..93b3cc962 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -289,7 +289,7 @@ void ProfileProtoBuilder::AddNativeInfo(const JVMPI_CallFrame &jvm_frame, if (!native_cache_) { int64_t address = reinterpret_cast(jvm_frame.method_id); perftools::profiles::Location *location = location_builder_.LocationFor( - "", "[Unknown non-Java frame]", "", 0, 0, address); + "", "[Unknown non-Java frame]", "", 0, kNativeFrameLineNum, address); sample->add_location_id(location->id()); return; } @@ -380,16 +380,20 @@ perftools::profiles::Location *LocationBuilder::LocationFor( locations_[info] = location; - auto line = location->add_line(); + // If this is a native frame with no symbolization, we don't want to add a + // line number to the profile. + if (line_number != kNativeFrameLineNum) { + auto line = location->add_line(); - 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(), start_line); + 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(), start_line); - line->set_function_id(function_id); - line->set_line(line_number); + line->set_function_id(function_id); + line->set_line(line_number); + } return location; } diff --git a/third_party/javaprofiler/profile_proto_builder.h b/third_party/javaprofiler/profile_proto_builder.h index 487f917c8..fbb738f7e 100644 --- a/third_party/javaprofiler/profile_proto_builder.h +++ b/third_party/javaprofiler/profile_proto_builder.h @@ -157,6 +157,11 @@ class TraceSamples { traces_; }; +enum class NativeSymbolization { + SYMBOLS, + NO_SYMBOLS +}; + // Store locations previously seen so that the profile is only // modified for new locations. class LocationBuilder {