From ac558e0594f9f35d0ceff238860719dd508d8786 Mon Sep 17 00:00:00 2001 From: sharmapranav Date: Mon, 6 Mar 2023 16:23:21 +0000 Subject: [PATCH 01/29] Internal Change PiperOrigin-RevId: 514417320 --- build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sh b/build.sh index ec5ccbd9c..97099397f 100755 --- a/build.sh +++ b/build.sh @@ -38,7 +38,7 @@ while getopts ":d:m:v:" opt; do m) TARGET=$OPTARG if [[ "${TARGET}" == "arm64" ]]; then - if [[ "${MACHINE_TYPE}" != "aarch64" ]] || [[ "${MACHINE_TYPE}" != "arm64" ]]; then + if [[ "${MACHINE_TYPE}" != "aarch64" ]] && [[ "${MACHINE_TYPE}" != "arm64" ]]; then echo "-m arm64 is supported only when running on an ARM64 system." exit; fi From ec99768a6b9c804d6c0c1563912e5034968e8f60 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 9 Mar 2023 09:48:28 +0000 Subject: [PATCH 02/29] Internal Code Change PiperOrigin-RevId: 515270706 --- third_party/perftools/profiles/proto/builder.cc | 3 +++ third_party/perftools/profiles/proto/builder.h | 1 + 2 files changed, 4 insertions(+) diff --git a/third_party/perftools/profiles/proto/builder.cc b/third_party/perftools/profiles/proto/builder.cc index 62e253bfb..908ea7bdd 100644 --- a/third_party/perftools/profiles/proto/builder.cc +++ b/third_party/perftools/profiles/proto/builder.cc @@ -19,6 +19,9 @@ #include #include +#include +#include +#include #include #include diff --git a/third_party/perftools/profiles/proto/builder.h b/third_party/perftools/profiles/proto/builder.h index 4f1cb9b5e..c5675d9c8 100644 --- a/third_party/perftools/profiles/proto/builder.h +++ b/third_party/perftools/profiles/proto/builder.h @@ -22,6 +22,7 @@ #include #include #include +#include #include From 7661eceeba55d16f715217d917c81509a670f6a7 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Mon, 20 Mar 2023 20:19:03 +0000 Subject: [PATCH 03/29] Add missing proto build steps to Makefile builds PiperOrigin-RevId: 518057341 --- Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 8234047c5..6b83cb67a 100644 --- a/Makefile +++ b/Makefile @@ -82,6 +82,8 @@ PROFILER_API_SOURCES = \ $(GENFILES_PATH)/google/api/client.pb.cc \ $(GENFILES_PATH)/google/api/http.pb.cc \ $(GENFILES_PATH)/google/api/launch_stage.pb.cc \ + $(GENFILES_PATH)/google/api/resource.pb.cc \ + $(GENFILES_PATH)/google/api/field_behavior.pb.cc \ $(GENFILES_PATH)/google/devtools/cloudprofiler/v2/profiler.grpc.pb.cc \ $(GENFILES_PATH)/google/devtools/cloudprofiler/v2/profiler.pb.cc \ $(GENFILES_PATH)/google/protobuf/duration.pb.cc \ @@ -235,6 +237,14 @@ $(GENFILES_PATH)/%launch_stage.pb.h $(GENFILES_PATH)/%launch_stage.pb.cc : third mkdir -p $(dir $@) $(PROTOC) -Ithird_party/googleapis -I$(PROTOBUF_INCLUDE_PATH) --cpp_out=$(GENFILES_PATH) $< +$(GENFILES_PATH)/%resource.pb.h $(GENFILES_PATH)/%resource.pb.cc : third_party/googleapis/%resource.proto + mkdir -p $(dir $@) + $(PROTOC) -Ithird_party/googleapis -I$(PROTOBUF_INCLUDE_PATH) --cpp_out=$(GENFILES_PATH) $< + +$(GENFILES_PATH)/%field_behavior.pb.h $(GENFILES_PATH)/%field_behavior.pb.cc : third_party/googleapis/%field_behavior.proto + mkdir -p $(dir $@) + $(PROTOC) -Ithird_party/googleapis -I$(PROTOBUF_INCLUDE_PATH) --cpp_out=$(GENFILES_PATH) $< + $(GENFILES_PATH)/%client.pb.h $(GENFILES_PATH)/%client.pb.cc : third_party/googleapis/%client.proto mkdir -p $(dir $@) $(PROTOC) -Ithird_party/googleapis -I$(PROTOBUF_INCLUDE_PATH) --cpp_out=$(GENFILES_PATH) $< From d351ca41028320c50fefe517d79eea6933dda0f8 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 30 Mar 2023 06:51:36 +0000 Subject: [PATCH 04/29] Internal Code Change PiperOrigin-RevId: 520552498 --- third_party/javaprofiler/accessors.cc | 2 +- third_party/javaprofiler/method_info.cc | 2 +- .../javaprofiler/profile_proto_builder.cc | 32 +++++++++---------- third_party/javaprofiler/profile_test_lib.cc | 8 ++--- third_party/javaprofiler/stacktraces.cc | 32 +++++++++---------- third_party/javaprofiler/tags.cc | 8 ++--- 6 files changed, 42 insertions(+), 42 deletions(-) diff --git a/third_party/javaprofiler/accessors.cc b/third_party/javaprofiler/accessors.cc index ef1eb7b3f..de84c48bd 100644 --- a/third_party/javaprofiler/accessors.cc +++ b/third_party/javaprofiler/accessors.cc @@ -20,7 +20,7 @@ namespace google { namespace javaprofiler { __thread JNIEnv *Accessors::env_; -__thread int64 Accessors::attr_; +__thread int64_t Accessors::attr_; __thread Tags *Accessors::tags_; void Accessors::InitTags() { diff --git a/third_party/javaprofiler/method_info.cc b/third_party/javaprofiler/method_info.cc index 79568c56a..89017c85d 100644 --- a/third_party/javaprofiler/method_info.cc +++ b/third_party/javaprofiler/method_info.cc @@ -21,7 +21,7 @@ namespace google { namespace javaprofiler { -int64 MethodInfo::Location(int line_number) { +int64_t MethodInfo::Location(int line_number) { auto it = locations_.find(line_number); if (it == locations_.end()) { return kInvalidLocationId; diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index 4ebacbc3d..34bfe582c 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -33,7 +33,7 @@ const int kMetric = 1; ProfileProtoBuilder::ProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, ProfileFrameCache *native_cache, - int64 sampling_rate, + int64_t sampling_rate, const SampleType &count_type, const SampleType &metric_type) : sampling_rate_(sampling_rate), @@ -61,8 +61,7 @@ void ProfileProtoBuilder::AddTraces(const ProfileStackTrace *traces, } void ProfileProtoBuilder::AddTraces(const ProfileStackTrace *traces, - const int32 *counts, - int num_traces) { + const int32_t *counts, int num_traces) { if (native_cache_) { native_cache_->ProcessTraces(traces, num_traces); } @@ -81,7 +80,7 @@ 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); } void ProfileProtoBuilder::UnsampleMetrics() { @@ -131,13 +130,13 @@ void ProfileProtoBuilder::SetPeriodType(const SampleType &metric_type) { } void ProfileProtoBuilder::UpdateSampleValues( - perftools::profiles::Sample *sample, int64 count, int64 size) { + perftools::profiles::Sample *sample, int64_t count, int64_t size) { sample->set_value(kCount, sample->value(kCount) + count); sample->set_value(kMetric, sample->value(kMetric) + size); } void ProfileProtoBuilder::InitSampleValues(perftools::profiles::Sample *sample, - int64 count, int64 metric) { + int64_t count, int64_t metric) { sample->add_value(count); sample->add_value(metric); } @@ -160,7 +159,7 @@ void ProfileProtoBuilder::AddLabels(const TraceAndLabels &trace_and_labels, } void ProfileProtoBuilder::AddTrace(const ProfileStackTrace &profile_trace, - int32 count) { + int32_t count) { const TraceAndLabels &trace_and_labels = profile_trace.trace_and_labels; auto sample = trace_samples_.SampleFor(trace_and_labels); jint metric_value = profile_trace.metric_value; @@ -207,12 +206,12 @@ void ProfileProtoBuilder::AddJavaInfo( sample->add_location_id(Location(method_info, jvm_frame)); } -int64 ProfileProtoBuilder::Location(MethodInfo *method, - const JVMPI_CallFrame &frame) { +int64_t ProfileProtoBuilder::Location(MethodInfo *method, + const JVMPI_CallFrame &frame) { // lineno is actually the BCI of the frame. int bci = frame.lineno; - int64 location_id = method->Location(bci); + int64_t location_id = method->Location(bci); // Non-zero as a location id is a valid location ID. if (location_id != MethodInfo::kInvalidLocationId) { @@ -475,7 +474,8 @@ void TraceSamples::Add(const TraceAndLabels &trace, traces_[trace] = sample; } -double CalculateSamplingRatio(int64 rate, int64 count, int64 metric_value) { +double CalculateSamplingRatio(int64_t rate, int64_t count, + int64_t metric_value) { if (rate <= 1 || count < 1 || metric_value < 1) { return 1.0; } @@ -490,7 +490,7 @@ double CalculateSamplingRatio(int64 rate, int64 count, int64 metric_value) { } std::unique_ptr ProfileProtoBuilder::ForHeap( - JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64 sampling_rate, + 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. @@ -499,8 +499,8 @@ std::unique_ptr ProfileProtoBuilder::ForHeap( } std::unique_ptr ProfileProtoBuilder::ForCpu( - JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64 duration_ns, - int64 sampling_rate, ProfileFrameCache *cache) { + 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( @@ -509,8 +509,8 @@ std::unique_ptr ProfileProtoBuilder::ForCpu( } std::unique_ptr ProfileProtoBuilder::ForContention( - JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64 sampling_rate, - int64 duration_nanos, ProfileFrameCache *cache) { + 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( diff --git a/third_party/javaprofiler/profile_test_lib.cc b/third_party/javaprofiler/profile_test_lib.cc index f41c6f996..8c99146aa 100644 --- a/third_party/javaprofiler/profile_test_lib.cc +++ b/third_party/javaprofiler/profile_test_lib.cc @@ -48,7 +48,7 @@ static jvmtiError GetMethodName(jvmtiEnv* jvmti, jmethodID method_id, char** gsig_str) { JvmProfileTestLib::method_name_call_count++; - switch (reinterpret_cast(method_id)) { + switch (reinterpret_cast(method_id)) { case 1: CreateJvmtiString(jvmti, "methodName", name_str); CreateJvmtiString(jvmti, "(I)B", sig_str); @@ -74,7 +74,7 @@ static jvmtiError GetMethodName(jvmtiEnv* jvmti, jmethodID method_id, static jvmtiError GetClassSignature(jvmtiEnv* jvmti, jclass declaring_class, char** sig_str, char** gsig_str) { - switch (reinterpret_cast(declaring_class)) { + switch (reinterpret_cast(declaring_class)) { case 1: CreateJvmtiString(jvmti, "Lcom/google/SomeClass;", sig_str); break; @@ -104,7 +104,7 @@ static jvmtiError GetMethodDeclaringClass(jvmtiEnv *env, jmethodID method, static jvmtiError GetSourceFileName(jvmtiEnv *env, jclass klass, char **source_name_ptr) { if (source_name_ptr) { - switch (reinterpret_cast(klass)) { + switch (reinterpret_cast(klass)) { case 1: *source_name_ptr = strdup("SomeClass.java"); break; @@ -151,7 +151,7 @@ static jvmtiError GetLineNumberTable(jvmtiEnv *env, jmethodID method, static jvmtiError GetStackTrace(jvmtiEnv* env, jthread thread, jint start_depth, jint max_frame_count, jvmtiFrameInfo* frame_buffer, jint* count_ptr) { - uint64 thread_num = reinterpret_cast(thread); + uint64_t thread_num = reinterpret_cast(thread); if (thread_num < 0 || thread_num >= JvmProfileTestLib::GetMaxThreads()) { *count_ptr = 0; diff --git a/third_party/javaprofiler/stacktraces.cc b/third_party/javaprofiler/stacktraces.cc index 3a3232a74..87b27619e 100644 --- a/third_party/javaprofiler/stacktraces.cc +++ b/third_party/javaprofiler/stacktraces.cc @@ -24,14 +24,14 @@ 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]); + uint64_t hash_val = CalculateHash(attr, trace->num_frames, &trace->frames[0]); - for (int64 i = 0; i < MaxEntries(); i++) { - int64 idx = (i + hash_val) % MaxEntries(); + for (int64_t i = 0; i < MaxEntries(); i++) { + int64_t idx = (i + hash_val) % MaxEntries(); auto &entry = traces_[idx]; - int64 count_zero = 0; + int64_t count_zero = 0; entry.active_updates.fetch_add(1, std::memory_order_acquire); - int64 count = entry.count.load(std::memory_order_acquire); + int64_t count = entry.count.load(std::memory_order_acquire); switch (count) { case 0: if (entry.count.compare_exchange_weak(count_zero, kTraceCountLocked, @@ -49,7 +49,7 @@ bool AsyncSafeTraceMultiset::Add(int attr, JVMPI_CallTrace *trace) { entry.trace.frames = fb; entry.trace.num_frames = num_frames; entry.attr = attr; - entry.count.store(static_cast(1), std::memory_order_release); + entry.count.store(static_cast(1), std::memory_order_release); return true; } break; @@ -80,13 +80,13 @@ bool AsyncSafeTraceMultiset::Add(int attr, JVMPI_CallTrace *trace) { return false; } -int AsyncSafeTraceMultiset::Extract(int location, int64 *attr, int max_frames, - JVMPI_CallFrame *frames, int64 *count) { +int AsyncSafeTraceMultiset::Extract(int location, int64_t *attr, int max_frames, + JVMPI_CallFrame *frames, int64_t *count) { if (location < 0 || location >= MaxEntries()) { return 0; } auto &entry = traces_[location]; - int64 c = entry.count.load(std::memory_order_acquire); + int64_t c = entry.count.load(std::memory_order_acquire); if (c <= 0) { // Unused or in process of being updated, skip for now. return 0; @@ -115,8 +115,8 @@ int AsyncSafeTraceMultiset::Extract(int location, int64 *attr, int max_frames, return num_frames; } -void TraceMultiset::Add(int64 attr, int num_frames, JVMPI_CallFrame *frames, - int64 count) { +void TraceMultiset::Add(int64_t attr, int num_frames, JVMPI_CallFrame *frames, + int64_t count) { CallTrace t; t.attr = attr; t.frames = std::vector(frames, frames + num_frames); @@ -131,10 +131,10 @@ void TraceMultiset::Add(int64 attr, int num_frames, JVMPI_CallFrame *frames, int HarvestSamples(AsyncSafeTraceMultiset *from, TraceMultiset *to) { int trace_count = 0; - int64 num_traces = from->MaxEntries(); - for (int64 i = 0; i < num_traces; i++) { + int64_t num_traces = from->MaxEntries(); + for (int64_t i = 0; i < num_traces; i++) { JVMPI_CallFrame frame[kMaxFramesToCapture]; - int64 attr, count; + int64_t attr, count; int num_frames = from->Extract(i, &attr, kMaxFramesToCapture, &frame[0], &count); @@ -146,10 +146,10 @@ int HarvestSamples(AsyncSafeTraceMultiset *from, TraceMultiset *to) { return trace_count; } -uint64 CalculateHash(int64 attr, int num_frames, +uint64_t CalculateHash(int64_t attr, int num_frames, const JVMPI_CallFrame *frame) { // Make hash-value - uint64 h = attr; + uint64_t h = attr; h += h << 10; h ^= h >> 6; for (int i = 0; i < num_frames; i++) { diff --git a/third_party/javaprofiler/tags.cc b/third_party/javaprofiler/tags.cc index e81801055..600d10a41 100644 --- a/third_party/javaprofiler/tags.cc +++ b/third_party/javaprofiler/tags.cc @@ -112,8 +112,8 @@ bool Tags::operator==(const Tags &other) const { return true; } -uint64 Tags::Hash() const { - uint64 h = 0; +uint64_t Tags::Hash() const { + uint64_t h = 0; for (const auto &val : values_) { h += val.Hash(); h += h << 10; @@ -142,13 +142,13 @@ std::vector> Tags::GetAll() return all_pairs; } -void Tags::SetAttribute(int64 value) { +void Tags::SetAttribute(int64_t value) { // TODO: Check whether the conversion between integer and string is a // performance concern. Set(kAttrKey, AsyncRefCountedString(std::to_string(value))); } -int64 Tags::GetAttribute() const { +int64_t Tags::GetAttribute() const { const std::string *value = Get(kAttrKey).Get(); if (value == nullptr) { return 0; From 936195ea446527514755ef43befffb26bd34a7c3 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 30 Mar 2023 11:22:22 +0000 Subject: [PATCH 05/29] Internal Code Change PiperOrigin-RevId: 520600713 --- src/cloud_env.cc | 1 + src/cloud_env.h | 2 ++ src/entry.cc | 1 + src/globals.h | 1 + src/http.cc | 2 ++ src/http.h | 2 ++ src/profiler.cc | 2 ++ src/profiler.h | 1 + src/proto.cc | 4 ++++ src/proto.h | 2 ++ src/string.cc | 4 ++++ src/threads.cc | 3 +++ src/threads.h | 1 + src/throttler.h | 1 + src/throttler_api.cc | 7 +++++++ src/throttler_timed.cc | 4 ++++ src/throttler_timed.h | 3 +++ src/uploader.cc | 1 + src/uploader.h | 2 ++ src/uploader_file.h | 1 + src/uploader_gcs.h | 1 + src/worker.cc | 5 +++++ src/worker.h | 1 + 23 files changed, 52 insertions(+) diff --git a/src/cloud_env.cc b/src/cloud_env.cc index 034a3deaf..1a04c49d9 100644 --- a/src/cloud_env.cc +++ b/src/cloud_env.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include "src/clock.h" #include "src/http.h" diff --git a/src/cloud_env.h b/src/cloud_env.h index 38e32c5f5..d5d750cd5 100644 --- a/src/cloud_env.h +++ b/src/cloud_env.h @@ -17,6 +17,8 @@ #ifndef CLOUD_PROFILER_AGENT_JAVA_CLOUD_ENV_H_ #define CLOUD_PROFILER_AGENT_JAVA_CLOUD_ENV_H_ +#include + #include "src/globals.h" namespace cloud { diff --git a/src/entry.cc b/src/entry.cc index 3d0d17dcd..221fec989 100644 --- a/src/entry.cc +++ b/src/entry.cc @@ -15,6 +15,7 @@ #include #include +#include #include "src/globals.h" #include "src/string.h" diff --git a/src/globals.h b/src/globals.h index 3c63458c4..00668e15d 100644 --- a/src/globals.h +++ b/src/globals.h @@ -23,6 +23,7 @@ #include #include +#include #include #include "third_party/javaprofiler/jvmti_error.h" diff --git a/src/http.cc b/src/http.cc index 60bc94062..6facc2d9a 100644 --- a/src/http.cc +++ b/src/http.cc @@ -14,6 +14,8 @@ #include "src/http.h" +#include + #include "curl/curl.h" namespace cloud { diff --git a/src/http.h b/src/http.h index 99527fcea..be927ed97 100644 --- a/src/http.h +++ b/src/http.h @@ -17,6 +17,8 @@ #ifndef CLOUD_PROFILER_AGENT_JAVA_HTTP_H_ #define CLOUD_PROFILER_AGENT_JAVA_HTTP_H_ +#include + #include "src/globals.h" typedef void CURL; diff --git a/src/profiler.cc b/src/profiler.cc index 69cbe791f..8eed75946 100644 --- a/src/profiler.cc +++ b/src/profiler.cc @@ -21,6 +21,8 @@ #include #include +#include +#include #include "src/clock.h" #include "src/globals.h" diff --git a/src/profiler.h b/src/profiler.h index 33841dd13..e0aaaa676 100644 --- a/src/profiler.h +++ b/src/profiler.h @@ -20,6 +20,7 @@ #include #include +#include #include "src/threads.h" #include "third_party/javaprofiler/stacktraces.h" diff --git a/src/proto.cc b/src/proto.cc index ffa8d4055..ead9e1395 100644 --- a/src/proto.cc +++ b/src/proto.cc @@ -20,6 +20,10 @@ #include #include +#include +#include +#include +#include #include "perftools/profiles/proto/builder.h" #include "third_party/javaprofiler/display.h" diff --git a/src/proto.h b/src/proto.h index 56f247f71..3dfea6546 100644 --- a/src/proto.h +++ b/src/proto.h @@ -17,6 +17,8 @@ #ifndef CLOUD_PROFILER_AGENT_JAVA_PROTO_H_ #define CLOUD_PROFILER_AGENT_JAVA_PROTO_H_ +#include + #include "src/profiler.h" #include "perftools/profiles/proto/builder.h" diff --git a/src/string.cc b/src/string.cc index ff4c769e6..229212ad2 100644 --- a/src/string.cc +++ b/src/string.cc @@ -14,7 +14,11 @@ #include "src/string.h" +#include #include +#include +#include +#include namespace cloud { namespace profiler { diff --git a/src/threads.cc b/src/threads.cc index e55e3ed8b..5a128bc20 100644 --- a/src/threads.cc +++ b/src/threads.cc @@ -19,6 +19,9 @@ #include #include +#include +#include + namespace cloud { namespace profiler { diff --git a/src/threads.h b/src/threads.h index 6a44d0aa6..b1ec072d0 100644 --- a/src/threads.h +++ b/src/threads.h @@ -21,6 +21,7 @@ #include // NOLINT(build/c++11) #include +#include #include "src/globals.h" diff --git a/src/throttler.h b/src/throttler.h index 93de22497..816040485 100644 --- a/src/throttler.h +++ b/src/throttler.h @@ -18,6 +18,7 @@ #define CLOUD_PROFILER_AGENT_JAVA_THROTTLER_H_ #include +#include namespace cloud { namespace profiler { diff --git a/src/throttler_api.cc b/src/throttler_api.cc index 23a87262a..39f1a24b2 100644 --- a/src/throttler_api.cc +++ b/src/throttler_api.cc @@ -22,7 +22,14 @@ #include #include // NOLINT +#include +#include +#include +#include #include +#include +#include +#include #include "src/clock.h" #include "src/cloud_env.h" diff --git a/src/throttler_timed.cc b/src/throttler_timed.cc index 30d175747..8d2de2838 100644 --- a/src/throttler_timed.cc +++ b/src/throttler_timed.cc @@ -15,6 +15,10 @@ #include "src/throttler_timed.h" #include +#include +#include +#include +#include #include "src/uploader_file.h" #include "src/uploader_gcs.h" diff --git a/src/throttler_timed.h b/src/throttler_timed.h index 64db3e87f..2f40b6d7a 100644 --- a/src/throttler_timed.h +++ b/src/throttler_timed.h @@ -20,6 +20,9 @@ #include #include #include +#include +#include +#include #include "src/clock.h" #include "src/throttler.h" diff --git a/src/uploader.cc b/src/uploader.cc index a19c691f2..7a1e28530 100644 --- a/src/uploader.cc +++ b/src/uploader.cc @@ -15,6 +15,7 @@ #include "src/uploader.h" #include // NOLINT(build/c++11) +#include namespace cloud { namespace profiler { diff --git a/src/uploader.h b/src/uploader.h index d3080f6ee..6808c95c3 100644 --- a/src/uploader.h +++ b/src/uploader.h @@ -17,6 +17,8 @@ #ifndef CLOUD_PROFILER_AGENT_JAVA_UPLOADER_H_ #define CLOUD_PROFILER_AGENT_JAVA_UPLOADER_H_ +#include + #include "src/globals.h" namespace cloud { diff --git a/src/uploader_file.h b/src/uploader_file.h index 4bf3ca653..89841bd1f 100644 --- a/src/uploader_file.h +++ b/src/uploader_file.h @@ -18,6 +18,7 @@ #define CLOUD_PROFILER_AGENT_JAVA_UPLOADER_FILE_H_ #include +#include #include "src/uploader.h" diff --git a/src/uploader_gcs.h b/src/uploader_gcs.h index 764a39cd3..d7e3fdd14 100644 --- a/src/uploader_gcs.h +++ b/src/uploader_gcs.h @@ -18,6 +18,7 @@ #define CLOUD_PROFILER_AGENT_JAVA_UPLOADER_GCS_H_ #include +#include #include "src/cloud_env.h" #include "src/uploader.h" diff --git a/src/worker.cc b/src/worker.cc index 9349838a5..8db17ba8b 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -14,6 +14,11 @@ #include "src/worker.h" +#include +#include +#include +#include + #include "src/clock.h" #include "src/profiler.h" #include "src/throttler_api.h" diff --git a/src/worker.h b/src/worker.h index 82566ead9..d384360f9 100644 --- a/src/worker.h +++ b/src/worker.h @@ -18,6 +18,7 @@ #define CLOUD_PROFILER_AGENT_JAVA_WORKER_H_ #include +#include #include // NOLINT #include "src/globals.h" From 36ff8bf11053d9f4982dfb6783bba11e815490da Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Fri, 31 Mar 2023 07:42:47 +0000 Subject: [PATCH 06/29] Internal Code Change PiperOrigin-RevId: 520853442 --- src/profiler.cc | 1 + src/worker.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/src/profiler.cc b/src/profiler.cc index 8eed75946..9410cd4bc 100644 --- a/src/profiler.cc +++ b/src/profiler.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include #include diff --git a/src/worker.cc b/src/worker.cc index 8db17ba8b..a2ba23854 100644 --- a/src/worker.cc +++ b/src/worker.cc @@ -14,6 +14,7 @@ #include "src/worker.h" +#include #include #include #include From 92f1c25739da60d47090fe3930a7b51ef6d968d5 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Mon, 3 Apr 2023 17:14:15 +0000 Subject: [PATCH 07/29] Prevent segfaults due to race conditions when Java Heap sampler is enabled. PiperOrigin-RevId: 521492609 --- src/entry.cc | 8 ++-- third_party/javaprofiler/heap_sampler.cc | 49 +++++++++++++++--------- third_party/javaprofiler/heap_sampler.h | 5 +-- 3 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/entry.cc b/src/entry.cc index 221fec989..ab8686bcc 100644 --- a/src/entry.cc +++ b/src/entry.cc @@ -125,9 +125,11 @@ void JNICALL OnVMInit(jvmtiEnv *jvmti, JNIEnv *jni_env, jthread thread) { if (FLAGS_cprof_enable_heap_sampling) { // TODO: Allow using the JVM's stack tracer with a flag once // we can get the current context in a cloud build. - google::javaprofiler::HeapMonitor::Enable( - jvmti, jni_env, FLAGS_cprof_heap_sampling_interval, - false /* use_jvm_trace */); + if (!google::javaprofiler::HeapMonitor::Enable( + jvmti, jni_env, FLAGS_cprof_heap_sampling_interval, + false /* use_jvm_trace */)) { + LOG(WARNING) << "Failed to start HeapMonitor."; + } } worker->Start(jni_env); diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index 10943eee4..ed9fd6b8a 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -91,6 +91,10 @@ extern "C" JNIEXPORT void SampledObjectAlloc(jvmtiEnv *jvmti_env, JNIEnv *jni_env, jthread thread, jobject object, jclass object_klass, jlong size) { + if (!google::javaprofiler::HeapMonitor::Enabled()) { + return; + } + if (!google::javaprofiler::HeapMonitor::HasAllocationInstrumentation() && !google::javaprofiler::HeapMonitor::HasGarbageInstrumentation()) { google::javaprofiler::HeapMonitor::AddSample(jni_env, thread, object, @@ -401,8 +405,8 @@ void HeapMonitor::AddGarbageInstrumentation( } void HeapMonitor::ShutdownGCWaitingThread() { - GetInstance()->NotifyGCWaitingThreadInternal(GcEvent::SHUTDOWN); - GetInstance()->WaitForShutdown(); + this->NotifyGCWaitingThreadInternal(GcEvent::SHUTDOWN); + this->WaitForShutdown(); } void HeapMonitor::WaitForShutdown() { @@ -430,16 +434,6 @@ bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, return false; } - jvmti_.store(jvmti); - sampling_interval_.store(sampling_interval); - use_jvm_trace_.store(use_jvm_trace); - - // Ensure this is really a singleton i.e. don't recreate it if sampling is - // re-enabled. - if (heap_monitor_ == nullptr) { - heap_monitor_.store(new HeapMonitor()); - } - jvmtiCapabilities caps; memset(&caps, 0, sizeof(caps)); // Get line numbers, sample events, and filename for the tests. @@ -460,10 +454,6 @@ bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, return false; } - if (!GetInstance()->CreateGCWaitingThread(jvmti, jni)) { - return false; - } - if (jvmti->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_SAMPLED_OBJECT_ALLOC, nullptr) != JVMTI_ERROR_NONE) { @@ -487,24 +477,44 @@ bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, Asgct::SetAsgct(Accessors::GetJvmFunction("AsyncGetCallTrace")); } + sampling_interval_.store(sampling_interval); + use_jvm_trace_.store(use_jvm_trace); + jvmti_.store(jvmti); + + // Ensure this is really a singleton i.e. don't recreate it if sampling is + // re-enabled. + if (heap_monitor_ == nullptr) { + HeapMonitor* monitor = new HeapMonitor(); + if (!monitor->CreateGCWaitingThread(jvmti, jni)) { + return false; + } + heap_monitor_.store(monitor); + } + return true; } void HeapMonitor::Disable() { + // Already disabled case jvmtiEnv *jvmti = jvmti_.load(); if (!jvmti) { return; } + HeapMonitor* monitor = heap_monitor_.load(); + if (!monitor) { + LOG(ERROR) << "heap monitor not loaded"; + return; + } + jvmti_.store(nullptr); jvmti->SetEventNotificationMode(JVMTI_DISABLE, JVMTI_EVENT_SAMPLED_OBJECT_ALLOC, nullptr); jvmti->SetEventNotificationMode(JVMTI_DISABLE, JVMTI_EVENT_GARBAGE_COLLECTION_FINISH, nullptr); - jvmti_.store(nullptr); - // Notify the agent thread that we are done. - google::javaprofiler::HeapMonitor::GetInstance()->ShutdownGCWaitingThread(); + monitor->ShutdownGCWaitingThread(); + heap_monitor_.store(nullptr); } std::unique_ptr HeapMonitor::GetHeapProfiles( @@ -564,6 +574,7 @@ HeapMonitor::GcEvent HeapMonitor::WaitForGC() { void HeapMonitor::GCWaitingThread(jvmtiEnv* jvmti_env, JNIEnv* jni_env, void* arg) { + while( !Enabled()) {} GetInstance()->GCWaitingThreadRun(jni_env); } diff --git a/third_party/javaprofiler/heap_sampler.h b/third_party/javaprofiler/heap_sampler.h index 48eafc177..3481e19a3 100644 --- a/third_party/javaprofiler/heap_sampler.h +++ b/third_party/javaprofiler/heap_sampler.h @@ -231,7 +231,7 @@ class HeapMonitor { bool use_jvm_trace); static void Disable(); - static bool Enabled() { return jvmti_ != nullptr; } + static bool Enabled() { return heap_monitor_ != nullptr; } // Returns a perftools::profiles::Profile with the objects provided by the // HeapEventStorage. @@ -278,8 +278,6 @@ class HeapMonitor { GetInstance()->NotifyGCWaitingThreadInternal(GcEvent::GC_FINISHED); } - static void ShutdownGCWaitingThread(); - // Not copyable or movable. HeapMonitor(const HeapMonitor &) = delete; HeapMonitor &operator=(const HeapMonitor &) = delete; @@ -313,6 +311,7 @@ class HeapMonitor { }; bool CreateGCWaitingThread(jvmtiEnv* jvmti, JNIEnv* jni); + void ShutdownGCWaitingThread(); static void GCWaitingThread(jvmtiEnv *jvmti_env, JNIEnv *jni_env, void *arg); void GCWaitingThreadRun(JNIEnv* jni_env); GcEvent WaitForGC(); From 6f863ed5e007fb7af729389136ae1d7a675c9414 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 6 Apr 2023 18:52:47 +0000 Subject: [PATCH 08/29] Prevent other segfaults due to race conditions when Java Heap sampler is enabled. PiperOrigin-RevId: 522395875 --- third_party/javaprofiler/heap_sampler.cc | 20 ++++++++++++-------- third_party/javaprofiler/heap_sampler.h | 12 +++++++++++- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index ed9fd6b8a..0cff6aee2 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -521,8 +521,9 @@ std::unique_ptr HeapMonitor::GetHeapProfiles( JNIEnv* env, bool force_gc) { // Note: technically this means that you cannot disable the sampler and then // get the profile afterwards; this could be changed if needed. - if (jvmti_) { - return GetInstance()->storage_.GetHeapProfiles(env, sampling_interval_, + HeapMonitor *monitor = TryGetInstance(); + if (monitor != nullptr) { + return monitor->storage_.GetHeapProfiles(env, sampling_interval_, force_gc); } return EmptyHeapProfile(env); @@ -532,8 +533,9 @@ std::unique_ptr HeapMonitor::GetPeakHeapProfiles( JNIEnv* env, bool force_gc) { // Note: technically this means that you cannot disable the sampler and then // get the profile afterwards; this could be changed if needed. - if (jvmti_) { - return GetInstance()->storage_.GetPeakHeapProfiles(env, sampling_interval_); + HeapMonitor *monitor = TryGetInstance(); + if (monitor != nullptr) { + return monitor->storage_.GetPeakHeapProfiles(env, sampling_interval_); } return EmptyHeapProfile(env); } @@ -542,8 +544,9 @@ std::unique_ptr HeapMonitor::GetGarbageHeapProfiles(JNIEnv* env, bool force_gc) { // Note: technically this means that you cannot disable the sampler and then // get the profile afterwards; this could be changed if needed. - if (jvmti_) { - return GetInstance()->storage_.GetGarbageHeapProfiles( + HeapMonitor *monitor = TryGetInstance(); + if (monitor != nullptr) { + return monitor->storage_.GetGarbageHeapProfiles( env, sampling_interval_, force_gc); } return EmptyHeapProfile(env); @@ -574,8 +577,9 @@ HeapMonitor::GcEvent HeapMonitor::WaitForGC() { void HeapMonitor::GCWaitingThread(jvmtiEnv* jvmti_env, JNIEnv* jni_env, void* arg) { - while( !Enabled()) {} - GetInstance()->GCWaitingThreadRun(jni_env); + HeapMonitor *monitor; + while ((monitor = TryGetInstance()) == nullptr) {} + monitor->GCWaitingThreadRun(jni_env); } void HeapMonitor::GCWaitingThreadRun(JNIEnv* jni_env) { diff --git a/third_party/javaprofiler/heap_sampler.h b/third_party/javaprofiler/heap_sampler.h index 3481e19a3..47b0d4ba2 100644 --- a/third_party/javaprofiler/heap_sampler.h +++ b/third_party/javaprofiler/heap_sampler.h @@ -275,7 +275,11 @@ class HeapMonitor { static void AddCallback(jvmtiEventCallbacks *callbacks); static void NotifyGCWaitingThread() { - GetInstance()->NotifyGCWaitingThreadInternal(GcEvent::GC_FINISHED); + HeapMonitor *monitor = TryGetInstance(); + if (monitor == nullptr) { + return; + } + monitor->NotifyGCWaitingThreadInternal(GcEvent::GC_FINISHED); } // Not copyable or movable. @@ -297,6 +301,12 @@ class HeapMonitor { return heap_monitor_; } + // A safe version of GetInstance() which can return a nullptr if heap_monitor_ + // happens to be empty. Used when there is no guarantee heap_monitor_ exists. + static HeapMonitor *TryGetInstance() { + return heap_monitor_; + } + ProfileFrameCache *GetFrameCache() { ProfileFrameCache *cache = nullptr; return cache; From e39cf712e98c2a3dc9958823a78f9b36c4e13a49 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Tue, 11 Apr 2023 23:27:06 +0000 Subject: [PATCH 09/29] Prevent memory leak due to race conditions when Java Heap sampler is enabled. PiperOrigin-RevId: 523535577 --- 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 0cff6aee2..f0b948d9c 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -484,7 +484,7 @@ bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, // Ensure this is really a singleton i.e. don't recreate it if sampling is // re-enabled. if (heap_monitor_ == nullptr) { - HeapMonitor* monitor = new HeapMonitor(); + static HeapMonitor* monitor = new HeapMonitor(); if (!monitor->CreateGCWaitingThread(jvmti, jni)) { return false; } From ce2c8babf07409eb285572d2ddc8cfbe28490936 Mon Sep 17 00:00:00 2001 From: sharmapranav Date: Fri, 19 May 2023 15:54:22 +0000 Subject: [PATCH 10/29] Internal testing logs PiperOrigin-RevId: 533466682 --- src/entry.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/entry.cc b/src/entry.cc index ab8686bcc..da84d6c54 100644 --- a/src/entry.cc +++ b/src/entry.cc @@ -278,6 +278,10 @@ jint JNICALL Agent_OnLoad(JavaVM *vm, char *options, void *reserved) { LOG(INFO) << "Google Cloud Profiler Java agent version: " << CLOUD_PROFILER_AGENT_VERSION; LOG(INFO) << "Profiler agent loaded"; + // TODO: Remove this log after the mentioned bug is closed + LOG(INFO) << "This release is used in internal DiRT Testing - It has no " + << "impact on the functionality of the release. Please ignore this " + << "log line - it will be removed in the next release."; google::javaprofiler::AttributeTable::Init(); // Try to get the latest JVMTI_VERSION the agent was built with. From f337ee2919d3a0af5a522b312ca1b3472943532c Mon Sep 17 00:00:00 2001 From: sharmapranav Date: Mon, 22 May 2023 15:15:01 +0000 Subject: [PATCH 11/29] Internal testing logs PiperOrigin-RevId: 534069370 --- src/entry.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/entry.cc b/src/entry.cc index da84d6c54..ab8686bcc 100644 --- a/src/entry.cc +++ b/src/entry.cc @@ -278,10 +278,6 @@ jint JNICALL Agent_OnLoad(JavaVM *vm, char *options, void *reserved) { LOG(INFO) << "Google Cloud Profiler Java agent version: " << CLOUD_PROFILER_AGENT_VERSION; LOG(INFO) << "Profiler agent loaded"; - // TODO: Remove this log after the mentioned bug is closed - LOG(INFO) << "This release is used in internal DiRT Testing - It has no " - << "impact on the functionality of the release. Please ignore this " - << "log line - it will be removed in the next release."; google::javaprofiler::AttributeTable::Init(); // Try to get the latest JVMTI_VERSION the agent was built with. From eb3a79b206f492ea1d1ee89d8526d257d11361ce Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Sat, 15 Jul 2023 19:11:44 +0000 Subject: [PATCH 12/29] Split out ThreadProfile code to a separate target in order to add unit tests. PiperOrigin-RevId: 548381574 --- third_party/perftools/profiles/proto/builder.cc | 15 +++++++++++++++ third_party/perftools/profiles/proto/builder.h | 5 +++++ 2 files changed, 20 insertions(+) diff --git a/third_party/perftools/profiles/proto/builder.cc b/third_party/perftools/profiles/proto/builder.cc index 908ea7bdd..5572f8e6f 100644 --- a/third_party/perftools/profiles/proto/builder.cc +++ b/third_party/perftools/profiles/proto/builder.cc @@ -47,6 +47,21 @@ typedef std::unordered_set IndexSet; namespace perftools { namespace profiles { +void AddCallstackToSample(Sample *sample, const void *const *stack, int depth, + CallstackType type) { + if (depth <= 0) return; + if (type == kInterrupt) { + sample->add_location_id(reinterpret_cast(stack[0])); + } else { + sample->add_location_id(reinterpret_cast(stack[0]) - 1); + } + // These are raw stack unwind addresses, so adjust them by -1 to land + // inside the call instruction (although potentially misaligned). + for (int i = 1; i < depth; i++) { + sample->add_location_id(reinterpret_cast(stack[i]) - 1); + } +} + Builder::Builder() : profile_(new Profile()) { // string_table[0] must be "" profile_->add_string_table(""); diff --git a/third_party/perftools/profiles/proto/builder.h b/third_party/perftools/profiles/proto/builder.h index c5675d9c8..719617b16 100644 --- a/third_party/perftools/profiles/proto/builder.h +++ b/third_party/perftools/profiles/proto/builder.h @@ -58,6 +58,11 @@ typedef std::unordered_map, int64, namespace perftools { namespace profiles { +enum CallstackType { kRegular = 0, kInterrupt = 1 }; + +void AddCallstackToSample(Sample *sample, const void *const *stack, int depth, + CallstackType type); + // Provides mechanisms to facilitate the generation of profiles // on a compressed protobuf: // - Manages the creation of the string table. From c61ed70b29f969b16d6d1b4ece964c4f4460d1f5 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 10 Aug 2023 15:05:37 +0000 Subject: [PATCH 13/29] Internal Code Change PiperOrigin-RevId: 555494681 --- src/globals.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/globals.h b/src/globals.h index 00668e15d..a5826223f 100644 --- a/src/globals.h +++ b/src/globals.h @@ -104,6 +104,10 @@ class JvmtiScopedPtr { JvmtiScopedPtr(jvmtiEnv *jvmti, T *ref) : jvmti_(jvmti), ref_(ref) {} + // This type is neither copyable nor movable. + JvmtiScopedPtr(const JvmtiScopedPtr &) = delete; + JvmtiScopedPtr &operator=(const JvmtiScopedPtr &) = delete; + ~JvmtiScopedPtr() { if (NULL != ref_) { JVMTI_ERROR(jvmti_->Deallocate((unsigned char *)ref_)); @@ -122,8 +126,6 @@ class JvmtiScopedPtr { private: jvmtiEnv *jvmti_; T *ref_; - - DISALLOW_IMPLICIT_CONSTRUCTORS(JvmtiScopedPtr); }; // Things that should probably be user-configurable From e4e93e871f2a2d949ed909b5998bd0f4bb4eb30e Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Sat, 12 Aug 2023 01:13:54 +0000 Subject: [PATCH 14/29] Remove uses of the deprecated DISALLOW_IMPLICIT_CONSTRUCTORS in favor of explicitly deleted constructors and assignment operators.. PiperOrigin-RevId: 556161979 --- third_party/javaprofiler/globals.h | 16 ++++++++-------- third_party/javaprofiler/stacktraces.h | 12 ++++++++---- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/third_party/javaprofiler/globals.h b/third_party/javaprofiler/globals.h index 7ceeb4034..57669e77b 100644 --- a/third_party/javaprofiler/globals.h +++ b/third_party/javaprofiler/globals.h @@ -33,10 +33,6 @@ using std::string; TypeName(const TypeName &); \ void operator=(const TypeName &) -#define DISALLOW_IMPLICIT_CONSTRUCTORS(TypeName) \ - TypeName(); \ - DISALLOW_COPY_AND_ASSIGN(TypeName) - // Short version: reinterpret_cast produces undefined behavior in many // cases where memcpy doesn't. template @@ -59,6 +55,10 @@ class JvmtiScopedPtr { public: explicit JvmtiScopedPtr(jvmtiEnv *jvmti) : jvmti_(jvmti), ref_(NULL) {} + // This type is neither copyable nor movable. + JvmtiScopedPtr(const JvmtiScopedPtr&) = delete; + JvmtiScopedPtr& operator=(const JvmtiScopedPtr&) = delete; + JvmtiScopedPtr(jvmtiEnv *jvmti, T *ref) : jvmti_(jvmti), ref_(ref) {} ~JvmtiScopedPtr() { @@ -77,8 +77,6 @@ class JvmtiScopedPtr { private: jvmtiEnv *jvmti_; T *ref_; - - DISALLOW_IMPLICIT_CONSTRUCTORS(JvmtiScopedPtr); }; template @@ -86,6 +84,10 @@ class ScopedLocalRef { public: ScopedLocalRef(JNIEnv *jni, T ref) : jni_(jni), ref_(ref) {} + // This type is neither copyable nor movable. + ScopedLocalRef(const ScopedLocalRef&) = delete; + ScopedLocalRef& operator=(const ScopedLocalRef&) = delete; + ~ScopedLocalRef() { if (NULL != ref_) { jni_->DeleteLocalRef(ref_); @@ -97,8 +99,6 @@ class ScopedLocalRef { private: JNIEnv *jni_; T ref_; - - DISALLOW_IMPLICIT_CONSTRUCTORS(ScopedLocalRef); }; } // namespace javaprofiler diff --git a/third_party/javaprofiler/stacktraces.h b/third_party/javaprofiler/stacktraces.h index 1ceb50109..747181256 100644 --- a/third_party/javaprofiler/stacktraces.h +++ b/third_party/javaprofiler/stacktraces.h @@ -44,6 +44,10 @@ const int kNumCallTraceErrors = 10; class Asgct { public: + // This type is neither copyable nor movable. + Asgct(const Asgct&) = delete; + Asgct& operator=(const Asgct&) = delete; + static void SetAsgct(ASGCTType asgct) { asgct_ = asgct; } // AsyncGetCallTrace function, to be dlsym'd. @@ -51,12 +55,14 @@ class Asgct { private: static ASGCTType asgct_; - - DISALLOW_IMPLICIT_CONSTRUCTORS(Asgct); }; class AttributeTable { public: + // This type is neither copyable nor movable. + AttributeTable(const AttributeTable&) = delete; + AttributeTable& operator=(const AttributeTable&) = delete; + static void Init() { mutex_ = new (std::mutex); string_map_ = new (std::unordered_map); @@ -93,8 +99,6 @@ class AttributeTable { static std::mutex *mutex_; static std::unordered_map *string_map_; static std::vector *strings_; - - DISALLOW_IMPLICIT_CONSTRUCTORS(AttributeTable); }; // Multiset of stack traces. There is a maximum number of distinct From db5c7f7e60f67898f582f044d81092dd853245d7 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 17 Aug 2023 01:54:38 +0000 Subject: [PATCH 15/29] Internal Code Change PiperOrigin-RevId: 557667699 --- src/cloud_env.h | 6 +++++- src/http.h | 6 +++++- src/profiler.cc | 6 ++++-- src/proto.cc | 5 ++++- src/threads.h | 6 ++++-- src/uploader_file.h | 5 ++++- src/uploader_gcs.h | 5 ++++- src/worker.h | 5 ++++- 8 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/cloud_env.h b/src/cloud_env.h index d5d750cd5..5a58dfb59 100644 --- a/src/cloud_env.h +++ b/src/cloud_env.h @@ -31,6 +31,11 @@ class HTTPRequest; class CloudEnv { public: CloudEnv(); + + // This type is neither copyable nor movable. + CloudEnv(const CloudEnv&) = delete; + CloudEnv& operator=(const CloudEnv&) = delete; + virtual ~CloudEnv() {} // Returns the current cloud project ID. @@ -70,7 +75,6 @@ class CloudEnv { std::string zone_name_; std::string service_; std::string service_version_; - DISALLOW_COPY_AND_ASSIGN(CloudEnv); }; // Returns the default instance of a cloud env object. The returned diff --git a/src/http.h b/src/http.h index be927ed97..f2c56c790 100644 --- a/src/http.h +++ b/src/http.h @@ -33,6 +33,11 @@ const int kHTTPStatusOK = 200; class HTTPRequest { public: HTTPRequest(); + + // This type is neither copyable nor movable. + HTTPRequest(const HTTPRequest&) = delete; + HTTPRequest& operator=(const HTTPRequest&) = delete; + virtual ~HTTPRequest(); virtual void AddAuthBearerHeader(const std::string& token); @@ -53,7 +58,6 @@ class HTTPRequest { CURL* curl_; struct curl_slist* headers_; - DISALLOW_COPY_AND_ASSIGN(HTTPRequest); }; } // namespace profiler diff --git a/src/profiler.cc b/src/profiler.cc index 9410cd4bc..959432845 100644 --- a/src/profiler.cc +++ b/src/profiler.cc @@ -52,12 +52,14 @@ namespace { class ErrnoRaii { public: ErrnoRaii() { stored_errno_ = errno; } + + // This type is neither copyable nor movable. + ErrnoRaii(const ErrnoRaii &) = delete; + ErrnoRaii &operator=(const ErrnoRaii &) = delete; ~ErrnoRaii() { errno = stored_errno_; } private: int stored_errno_; - - DISALLOW_COPY_AND_ASSIGN(ErrnoRaii); }; } // namespace diff --git a/src/proto.cc b/src/proto.cc index ead9e1395..31c4218ec 100644 --- a/src/proto.cc +++ b/src/proto.cc @@ -45,6 +45,10 @@ class ProfileProtoBuilder { } } + // This type is neither copyable nor movable. + ProfileProtoBuilder(const ProfileProtoBuilder &) = delete; + ProfileProtoBuilder &operator=(const ProfileProtoBuilder &) = delete; + // Populate the profile with a set of traces void Populate(JNIEnv *jni, const char *profile_type, const google::javaprofiler::TraceMultiset &traces, @@ -94,7 +98,6 @@ class ProfileProtoBuilder { std::unordered_map address_location_; const google::javaprofiler::NativeProcessInfo &native_info_; - DISALLOW_COPY_AND_ASSIGN(ProfileProtoBuilder); }; namespace { diff --git a/src/threads.h b/src/threads.h index b1ec072d0..e4feac2da 100644 --- a/src/threads.h +++ b/src/threads.h @@ -38,6 +38,10 @@ class ThreadTable { explicit ThreadTable(bool use_timers) : use_timers_(use_timers), period_usec_() {} + // This type is neither copyable nor movable. + ThreadTable(const ThreadTable&) = delete; + ThreadTable& operator=(const ThreadTable&) = delete; + // Registers the current thread. void RegisterCurrent(); // Unregisters the current thread. @@ -62,8 +66,6 @@ class ThreadTable { bool use_timers_; // Non-zero when the thread timers have been started. int64_t period_usec_; - - DISALLOW_COPY_AND_ASSIGN(ThreadTable); }; // Returns the thread ID of the current thread. diff --git a/src/uploader_file.h b/src/uploader_file.h index 89841bd1f..914e6383d 100644 --- a/src/uploader_file.h +++ b/src/uploader_file.h @@ -29,6 +29,10 @@ class FileUploader : public cloud::profiler::ProfileUploader { public: explicit FileUploader(const std::string &prefix) : prefix_(prefix) {} + // This type is neither copyable nor movable. + FileUploader(const FileUploader &) = delete; + FileUploader &operator=(const FileUploader &) = delete; + bool Upload(const std::string &profile_type, const std::string &profile) override { std::string filename = ProfilePath(prefix_, profile_type); @@ -54,7 +58,6 @@ class FileUploader : public cloud::profiler::ProfileUploader { private: std::string prefix_; - DISALLOW_COPY_AND_ASSIGN(FileUploader); }; } // namespace profiler diff --git a/src/uploader_gcs.h b/src/uploader_gcs.h index d7e3fdd14..25856f5b0 100644 --- a/src/uploader_gcs.h +++ b/src/uploader_gcs.h @@ -40,6 +40,10 @@ class GcsUploader : public cloud::profiler::ProfileUploader { GcsUploader(CloudEnv* env, const std::string& prefix) : env_(env), prefix_(prefix) {} + // This type is neither copyable nor movable. + GcsUploader(const GcsUploader&) = delete; + GcsUploader& operator=(const GcsUploader&) = delete; + // Implements ProfileUploader interface. bool Upload(const std::string& profile_type, const std::string& profile) override; @@ -47,7 +51,6 @@ class GcsUploader : public cloud::profiler::ProfileUploader { private: CloudEnv* env_; std::string prefix_; - DISALLOW_COPY_AND_ASSIGN(GcsUploader); }; } // namespace profiler diff --git a/src/worker.h b/src/worker.h index d384360f9..b9651da60 100644 --- a/src/worker.h +++ b/src/worker.h @@ -33,6 +33,10 @@ class Worker { Worker(jvmtiEnv *jvmti, ThreadTable *threads) : jvmti_(jvmti), threads_(threads), stopping_() {} + // This type is neither copyable nor movable. + Worker(const Worker &) = delete; + Worker &operator=(const Worker &) = delete; + void Start(JNIEnv *jni); void Stop(); @@ -48,7 +52,6 @@ class Worker { std::mutex mutex_; // Held by the worker thread while it's running. std::atomic stopping_; static std::atomic enabled_; - DISALLOW_COPY_AND_ASSIGN(Worker); }; } // namespace profiler From 2e6da4d3593f2a27228ec5d0cf1ed39fe3396f8c Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Wed, 11 Oct 2023 21:09:34 +0000 Subject: [PATCH 16/29] Change `T const&&` to `T`. const r-value vector reference always results in a copy. PiperOrigin-RevId: 572682185 --- third_party/javaprofiler/heap_sampler.cc | 4 ++-- third_party/javaprofiler/heap_sampler.h | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index f0b948d9c..7b76b2533 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -159,8 +159,8 @@ HeapEventStorage::HeapEventStorage(jvmtiEnv *jvmti, void HeapEventStorage::Add(JNIEnv *jni, jthread thread, jobject object, jclass klass, jlong size, - const std::vector &&frames, - jbyte *name, jint name_len, jlong thread_id) { + std::vector frames, jbyte *name, + jint name_len, jlong thread_id) { jweak weak_ref = jni->NewWeakGlobalRef(object); if (jni->ExceptionCheck()) { LOG(WARNING) << "Failed to create NewWeakGlobalRef, skipping heap sample"; diff --git a/third_party/javaprofiler/heap_sampler.h b/third_party/javaprofiler/heap_sampler.h index 47b0d4ba2..748455e5f 100644 --- a/third_party/javaprofiler/heap_sampler.h +++ b/third_party/javaprofiler/heap_sampler.h @@ -51,11 +51,14 @@ class HeapObjectTrace { public: // This object owns the jweak object parameter. It is freed when the object // is sent to the garbage list, and the object is set to nullptr. - HeapObjectTrace(jweak object, jlong size, - const std::vector &&frames, jbyte *name, - int name_length, jlong thread_id) - : object_(object), size_(size), frames_(std::move(frames)), - name_(name), name_length_(name_length), thread_id_(thread_id) {} + HeapObjectTrace(jweak object, jlong size, std::vector frames, + jbyte *name, int name_length, jlong thread_id) + : object_(object), + size_(size), + frames_(std::move(frames)), + name_(name), + name_length_(name_length), + thread_id_(thread_id) {} HeapObjectTrace(jweak object, jlong size, const std::vector &frames) @@ -132,7 +135,7 @@ class HeapEventStorage { // Adds an object to the storage system. void Add(JNIEnv *jni, jthread thread, jobject object, jclass klass, - jlong size, const std::vector &&frames, jbyte *name, + jlong size, std::vector frames, jbyte *name, jint name_len, jlong thread_id); // Returns a perftools::profiles::Profile with the objects stored via From c10f720e090f38f21375619f2da12d458765b773 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Tue, 24 Oct 2023 17:27:35 +0000 Subject: [PATCH 17/29] Fix SIGSEGV in `HeapMonitor` during shutdown PiperOrigin-RevId: 576185633 --- third_party/javaprofiler/heap_sampler.cc | 48 +++++++++++++++++++----- third_party/javaprofiler/heap_sampler.h | 11 +----- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index 7b76b2533..3385748c4 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -361,9 +361,15 @@ void HeapMonitor::AddSample(JNIEnv *jni_env, jthread thread, jobject object, if (trace == nullptr) { return; } - - GetInstance()->storage_.Add(jni_env, thread, object, object_klass, size, - std::move(*trace), name, name_len, thread_id); + // HeapMonitor is never deleted, but the pointer is cleared during + // disablement. Ensure we are not racing with disablement and check if the + // HeapMonitor is nullptr. + HeapMonitor *instance = TryGetInstance(); + if (instance == nullptr) { + return; + } + instance->storage_.Add(jni_env, thread, object, object_klass, size, + std::move(*trace), name, name_len, thread_id); } void HeapMonitor::InvokeAllocationInstrumentationFunctions(jlong thread_id, @@ -371,22 +377,38 @@ void HeapMonitor::InvokeAllocationInstrumentationFunctions(jlong thread_id, int name_length, int size, jlong gcontext) { - for (auto fn : GetInstance()->alloc_inst_functions_) { + HeapMonitor *instance = TryGetInstance(); + if (instance == nullptr) { + return; + } + for (auto fn : instance->alloc_inst_functions_) { fn(thread_id, name, name_length, size, gcontext); } } void HeapMonitor::AddAllocationInstrumentation( AllocationInstrumentationFunction fn) { - GetInstance()->alloc_inst_functions_.push_back(fn); + HeapMonitor *instance = TryGetInstance(); + if (instance == nullptr) { + return; + } + instance->alloc_inst_functions_.push_back(fn); } bool HeapMonitor::HasAllocationInstrumentation() { - return !GetInstance()->alloc_inst_functions_.empty(); + HeapMonitor *instance = TryGetInstance(); + if (instance == nullptr) { + return false; + } + return !instance->alloc_inst_functions_.empty(); } bool HeapMonitor::HasGarbageInstrumentation() { - return !GetInstance()->gc_inst_functions_.empty(); + HeapMonitor *instance = TryGetInstance(); + if (instance == nullptr) { + return false; + } + return !instance->gc_inst_functions_.empty(); } void HeapMonitor::InvokeGarbageInstrumentationFunctions(jlong thread_id, @@ -394,14 +416,22 @@ void HeapMonitor::InvokeGarbageInstrumentationFunctions(jlong thread_id, int name_length, int size, jlong gcontext) { - for (auto fn : GetInstance()->gc_inst_functions_) { + HeapMonitor *instance = TryGetInstance(); + if (instance == nullptr) { + return; + } + for (auto fn : instance->gc_inst_functions_) { fn(thread_id, name, name_length, size, gcontext); } } void HeapMonitor::AddGarbageInstrumentation( GarbageInstrumentationFunction fn) { - GetInstance()->gc_inst_functions_.push_back(fn); + HeapMonitor *instance = TryGetInstance(); + if (instance == nullptr) { + return; + } + instance->gc_inst_functions_.push_back(fn); } void HeapMonitor::ShutdownGCWaitingThread() { diff --git a/third_party/javaprofiler/heap_sampler.h b/third_party/javaprofiler/heap_sampler.h index 748455e5f..04948689b 100644 --- a/third_party/javaprofiler/heap_sampler.h +++ b/third_party/javaprofiler/heap_sampler.h @@ -297,15 +297,8 @@ class HeapMonitor { static std::atomic heap_monitor_; - // We initialize heap_monitor_ in Enable, so ensure Enable is called before - // any call to this method. - static HeapMonitor *GetInstance() { - assert(heap_monitor_ != nullptr); - return heap_monitor_; - } - - // A safe version of GetInstance() which can return a nullptr if heap_monitor_ - // happens to be empty. Used when there is no guarantee heap_monitor_ exists. + // Return a nullptr if heap_monitor_ happens to be empty. You must check + // whether the return value is nullptr. static HeapMonitor *TryGetInstance() { return heap_monitor_; } From 1088b13a4a5c779a89efc5d5804a57709858c7b2 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 26 Oct 2023 18:41:59 +0000 Subject: [PATCH 18/29] Update roots.pem to align with the current public set PiperOrigin-RevId: 576940801 --- src/pem_roots.cc | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/pem_roots.cc b/src/pem_roots.cc index 5512b5b3c..c192ed412 100644 --- a/src/pem_roots.cc +++ b/src/pem_roots.cc @@ -844,16 +844,16 @@ smPi9WIsgtRqAEFQ8TmDn5XpNpaYbg== # Issuer: CN=COMODO Certification Authority O=COMODO CA Limited # Subject: CN=COMODO Certification Authority O=COMODO CA Limited # Label: "COMODO Certification Authority" -# Serial: 104350513648249232941998508985834464573 -# MD5 Fingerprint: 5c:48:dc:f7:42:72:ec:56:94:6d:1c:cc:71:35:80:75 -# SHA1 Fingerprint: 66:31:bf:9e:f7:4f:9e:b6:c9:d5:a6:0c:ba:6a:be:d1:f7:bd:ef:7b -# SHA256 Fingerprint: 0c:2c:d6:3d:f7:80:6f:a3:99:ed:e8:09:11:6b:57:5b:f8:79:89:f0:65:18:f9:80:8c:86:05:03:17:8b:af:66 +# Serial: 43390818032842818540635488309124489234 +# MD5 Fingerprint: 20:E7:4F:82:C2:7E:94:80:34:82:8A:13:A9:17:1D:97 +# SHA1 Fingerprint EE:86:93:87:FF:FD:83:49:AB:5A:D1:43:22:58:87:89:A4:57:B0:12 +# SHA256 Fingerprint: 1A:0D:20:44:5D:E5:BA:18:62:D1:9E:F8:80:85:8C:BC:E5:01:02:B3:6E:8F:0A:04:0C:3C:69:E7:45:22:FE:6E -----BEGIN CERTIFICATE----- -MIIEHTCCAwWgAwIBAgIQToEtioJl4AsC7j41AkblPTANBgkqhkiG9w0BAQUFADCB +MIID0DCCArigAwIBAgIQIKTEf93f4cdTYwcTiHdgEjANBgkqhkiG9w0BAQUFADCB gTELMAkGA1UEBhMCR0IxGzAZBgNVBAgTEkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4G A1UEBxMHU2FsZm9yZDEaMBgGA1UEChMRQ09NT0RPIENBIExpbWl0ZWQxJzAlBgNV -BAMTHkNPTU9ETyBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eTAeFw0wNjEyMDEwMDAw -MDBaFw0yOTEyMzEyMzU5NTlaMIGBMQswCQYDVQQGEwJHQjEbMBkGA1UECBMSR3Jl +BAMTHkNPTU9ETyBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eTAeFw0xMTAxMDEwMDAw +MDBaFw0zMDEyMzEyMzU5NTlaMIGBMQswCQYDVQQGEwJHQjEbMBkGA1UECBMSR3Jl YXRlciBNYW5jaGVzdGVyMRAwDgYDVQQHEwdTYWxmb3JkMRowGAYDVQQKExFDT01P RE8gQ0EgTGltaXRlZDEnMCUGA1UEAxMeQ09NT0RPIENlcnRpZmljYXRpb24gQXV0 aG9yaXR5MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0ECLi3LjkRv3 @@ -862,16 +862,14 @@ UcEbVASY06m/weaKXTuH+7uIzg3jLz8GlvCiKVCZrts7oVewdFFxze1CkU1B/qnI Q5sVW7euNJH+1GImGEaaP+vB+fGQV+useg2L23IwambV4EajcNxo2f8ESIl33rXp +2dtQem8Ob0y2WIC8bGoPW43nOIv4tOiJovGuFVDiOEjPqXSJDlqR6sA1KGzqSX+ DT+nHbrTUcELpNqsOO9VUCQFZUaTNE8tja3G1CEZ0o7KBWFxB3NH5YoZEr0ETc5O -nKVIrLsm9wIDAQABo4GOMIGLMB0GA1UdDgQWBBQLWOWLxkwVN6RAqTCpIb5HNlpW -/zAOBgNVHQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zBJBgNVHR8EQjBAMD6g -PKA6hjhodHRwOi8vY3JsLmNvbW9kb2NhLmNvbS9DT01PRE9DZXJ0aWZpY2F0aW9u -QXV0aG9yaXR5LmNybDANBgkqhkiG9w0BAQUFAAOCAQEAPpiem/Yb6dc5t3iuHXIY -SdOH5EOC6z/JqvWote9VfCFSZfnVDeFs9D6Mk3ORLgLETgdxb8CPOGEIqB6BCsAv -IC9Bi5HcSEW88cbeunZrM8gALTFGTO3nnc+IlP8zwFboJIYmuNg4ON8qa90SzMc/ -RxdMosIGlgnW2/4/PEZB31jiVg88O8EckzXZOFKs7sjsLjBOlDW0JB9LeGna8gI4 -zJVSk/BwJVmcIGfE7vmLV2H0knZ9P4SNVbfo5azV8fUZVqZa+5Acr5Pr5RzUZ5dd -BA6+C4OmF4O5MBKgxTMVBbkN+8cFduPYSo38NBejxiEovjBFMR7HeL5YYTisO+IB -ZQ== +nKVIrLsm9wIDAQABo0IwQDAdBgNVHQ4EFgQUC1jli8ZMFTekQKkwqSG+RzZaVv8w +DgYDVR0PAQH/BAQDAgEGMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQEFBQAD +ggEBAC/JxBwHO89hAgCx2SFRdXIDMLDEFh9sAIsQrK/xR9SuEDwMGvjUk2ysEDd8 +t6aDZK3N3w6HM503sMZ7OHKx8xoOo/lVem0DZgMXlUrxsXrfViEGQo+x06iF3u6X +HWLrp+cxEmbDD6ZLLkGC9/3JG6gbr+48zuOcrigHoSybJMIPIyaDMouGDx8rEkYl +Fo92kANr3ryqImhrjKGsKxE5pttwwn1y6TPn/CbxdFqR5p2ErPioBhlG5qfpqjQi +pKGfeq23sqSaM4hxAjwu1nqyH6LKwN0vEJT9s4yEIHlG1QXUEOTS22RPuFvuG8Ug +R1uUq27UlTMdphVx8fiUylQ5PsE= -----END CERTIFICATE----- # Operating CA: Sectigo From 6ec426393ac6474380f5409351fafc11ab63f02b Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Tue, 31 Oct 2023 21:51:10 +0000 Subject: [PATCH 19/29] Fix potential crash in shutdown due to null jvmti_ pointer. PiperOrigin-RevId: 578310101 --- third_party/javaprofiler/heap_sampler.cc | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index 3385748c4..cfcce8dbb 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -525,17 +525,12 @@ bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, } void HeapMonitor::Disable() { - // Already disabled case - jvmtiEnv *jvmti = jvmti_.load(); - if (!jvmti) { - return; - } HeapMonitor* monitor = heap_monitor_.load(); - if (!monitor) { - LOG(ERROR) << "heap monitor not loaded"; + if (monitor == nullptr) { + // Already disabled. return; } - jvmti_.store(nullptr); + jvmtiEnv *jvmti = jvmti_.load(); jvmti->SetEventNotificationMode(JVMTI_DISABLE, JVMTI_EVENT_SAMPLED_OBJECT_ALLOC, nullptr); From 42ca11c80b8640991a4528f8254caa7731dbad1d Mon Sep 17 00:00:00 2001 From: aalexand Date: Wed, 1 Nov 2023 22:18:00 +0000 Subject: [PATCH 20/29] Update profile.proto to match pprof's original. PiperOrigin-RevId: 578657868 --- .../perftools/profiles/proto/profile.proto | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/third_party/perftools/profiles/proto/profile.proto b/third_party/perftools/profiles/proto/profile.proto index ed75177fe..2a239ce96 100644 --- a/third_party/perftools/profiles/proto/profile.proto +++ b/third_party/perftools/profiles/proto/profile.proto @@ -1,4 +1,4 @@ -// Copyright 2018 Google LLC +// Copyright 2016 Google Inc. All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -11,8 +11,6 @@ // 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. -// -//////////////////////////////////////////////////////////////////////////////// // Profile is a common stacktrace profile format. // @@ -60,9 +58,9 @@ message Profile { // Mapping from address ranges to the image/binary/library mapped // into that address range. mapping[0] will be the main binary. repeated Mapping mapping = 3; - // Useful program location + // Locations referenced by samples. repeated Location location = 4; - // Functions referenced by locations + // Functions referenced by locations. repeated Function function = 5; // A common table for strings referenced by various messages. // string_table[0] must always be "". @@ -71,7 +69,7 @@ message Profile { // regexp will be dropped from the samples, along with their successors. int64 drop_frames = 7; // Index into string table. // frames with Function.function_name fully matching the following - // regexp will be kept, even if it matches drop_functions. + // regexp will be kept, even if it matches drop_frames. int64 keep_frames = 8; // Index into string table. // The following fields are informational, do not affect @@ -86,7 +84,11 @@ message Profile { ValueType period_type = 11; // The number of events between sampled occurrences. int64 period = 12; - // Freeform text associated to the profile. + // Free-form text associated with the profile. The text is displayed as is + // to the user by the tools that read profiles (e.g. by pprof). This field + // should not be used to store any machine-readable information, it is only + // for human-friendly content. The profile must stay functional if this field + // is cleaned. repeated int64 comment = 13; // Indices into string table. // Index into the string table of the type of the preferred sample // value. If unset, clients should default to the last sample value. @@ -115,7 +117,13 @@ message Sample { // lists of the originals. repeated int64 value = 2; // label includes additional context for this sample. It can include - // things like a thread id, allocation size, etc + // things like a thread id, allocation size, etc. + // + // NOTE: While possible, having multiple values for the same label key is + // strongly discouraged and should never be used. Most tools (e.g. pprof) do + // not have good (or any) support for multi-value labels. And an even more + // discouraged case is having a string label and a numeric label of the same + // name on a sample. Again, possible to express, but should not be used. repeated Label label = 3; } From 59b51b39d914eae68fdce183872703f35b189585 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Wed, 8 Nov 2023 20:01:21 +0000 Subject: [PATCH 21/29] Fix ClangTidy warnings and format code. PiperOrigin-RevId: 580609161 --- third_party/javaprofiler/heap_sampler.cc | 121 +++++++++++------------ third_party/javaprofiler/heap_sampler.h | 111 ++++++++------------- 2 files changed, 96 insertions(+), 136 deletions(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index cfcce8dbb..14f8f477c 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -14,9 +14,15 @@ * limitations under the License. */ -#include +#include +#include + +#include +#include // NOLINT +#include #include -#include +#include // NOLINT +#include #include "third_party/javaprofiler/accessors.h" #include "third_party/javaprofiler/heap_sampler.h" @@ -28,8 +34,8 @@ namespace { using google::javaprofiler::JVMPI_CallFrame; -std::vector -TransformFrames(jvmtiFrameInfo *stack_frames, int count) { +std::vector TransformFrames(jvmtiFrameInfo *stack_frames, + int count) { std::vector frames(count); for (int i = 0; i < count; i++) { @@ -81,8 +87,8 @@ static jstring GetClassName(JNIEnv *jni_env, jobject object, static jlong GetThreadId(JNIEnv *jni_env, jthread thread) { jclass thread_class = jni_env->FindClass("java/lang/Thread"); - jmethodID get_id_method_id = jni_env->GetMethodID(thread_class, - "getId", "()J"); + jmethodID get_id_method_id = + jni_env->GetMethodID(thread_class, "getId", "()J"); jlong thread_id = jni_env->CallLongMethod(thread, get_id_method_id); return thread_id; } @@ -97,16 +103,15 @@ extern "C" JNIEXPORT void SampledObjectAlloc(jvmtiEnv *jvmti_env, if (!google::javaprofiler::HeapMonitor::HasAllocationInstrumentation() && !google::javaprofiler::HeapMonitor::HasGarbageInstrumentation()) { - google::javaprofiler::HeapMonitor::AddSample(jni_env, thread, object, - object_klass, size, nullptr, 0, - 0); + google::javaprofiler::HeapMonitor::AddSample( + jni_env, thread, object, object_klass, size, nullptr, 0, 0); return; } jstring class_name = GetClassName(jni_env, object, object_klass); const char *name_str = jni_env->GetStringUTFChars(class_name, NULL); int name_len = strlen(name_str); - jbyte *name_bytes = const_cast( - reinterpret_cast(name_str)); + jbyte *name_bytes = + const_cast(reinterpret_cast(name_str)); jlong thread_id = GetThreadId(jni_env, thread); @@ -130,12 +135,9 @@ namespace javaprofiler { const HeapEventStorage::GcCallback HeapMonitor::gc_callback_ = [](const HeapObjectTrace &elem) { - HeapMonitor::InvokeGarbageInstrumentationFunctions(elem.ThreadId(), - elem.Name(), - elem.NameLength(), - elem.Size(), - 0); -}; + HeapMonitor::InvokeGarbageInstrumentationFunctions( + elem.ThreadId(), elem.Name(), elem.NameLength(), elem.Size(), 0); + }; std::atomic HeapMonitor::heap_monitor_; @@ -144,18 +146,16 @@ std::atomic HeapMonitor::sampling_interval_; std::atomic HeapMonitor::use_jvm_trace_; std::vector HeapMonitor::alloc_inst_functions_; -std::vector - HeapMonitor::gc_inst_functions_; +std::vector HeapMonitor::gc_inst_functions_; -HeapEventStorage::HeapEventStorage(jvmtiEnv *jvmti, - ProfileFrameCache *cache, - int max_garbage_size, - GcCallback gc_callback) +HeapEventStorage::HeapEventStorage(jvmtiEnv *jvmti, ProfileFrameCache *cache, + int max_garbage_size, GcCallback gc_callback) : peak_profile_size_(0), max_garbage_size_(max_garbage_size), cur_garbage_pos_(0), - jvmti_(jvmti), cache_(cache), gc_callback_(gc_callback) { -} + jvmti_(jvmti), + cache_(cache), + gc_callback_(gc_callback) {} void HeapEventStorage::Add(JNIEnv *jni, jthread thread, jobject object, jclass klass, jlong size, @@ -240,15 +240,13 @@ HeapEventStorage::StackTraceArrayBuilder::StackTraceArrayBuilder( : objects_size_(objects_size), stack_trace_data_( new google::javaprofiler::ProfileStackTrace[objects_size_]), - call_trace_data_(new JVMPI_CallTrace[objects_size_]) { -} + call_trace_data_(new JVMPI_CallTrace[objects_size_]) {} void HeapEventStorage::StackTraceArrayBuilder::AddTrace( HeapObjectTrace &object) { std::vector &frames = object.Frames(); - call_trace_data_[curr_trace_] = {nullptr, - static_cast(frames.size()), + call_trace_data_[curr_trace_] = {nullptr, static_cast(frames.size()), frames.data()}; stack_trace_data_[curr_trace_] = {&call_trace_data_[curr_trace_], @@ -306,7 +304,7 @@ std::unique_ptr HeapEventStorage::GetProfiles( } } -bool HeapMonitor::CreateGCWaitingThread(jvmtiEnv* jvmti, JNIEnv* jni) { +bool HeapMonitor::CreateGCWaitingThread(jvmtiEnv *jvmti, JNIEnv *jni) { jclass cls = jni->FindClass("java/lang/Thread"); jmethodID constructor = jni->GetMethodID(cls, "", "()V"); jobject thread = jni->NewGlobalRef(jni->NewObject(cls, constructor)); @@ -356,8 +354,8 @@ void HeapMonitor::AddSample(JNIEnv *jni_env, jthread thread, jobject object, jclass object_klass, jlong size, jbyte *name, jint name_len, jlong thread_id) { auto trace = use_jvm_trace_.load() - ? GetTrace(jni_env) - : GetTraceUsingJvmti(jni_env, jvmti_.load(), thread); + ? GetTrace(jni_env) + : GetTraceUsingJvmti(jni_env, jvmti_.load(), thread); if (trace == nullptr) { return; } @@ -372,11 +370,8 @@ void HeapMonitor::AddSample(JNIEnv *jni_env, jthread thread, jobject object, std::move(*trace), name, name_len, thread_id); } -void HeapMonitor::InvokeAllocationInstrumentationFunctions(jlong thread_id, - jbyte *name, - int name_length, - int size, - jlong gcontext) { +void HeapMonitor::InvokeAllocationInstrumentationFunctions( + jlong thread_id, jbyte *name, int name_length, int size, jlong gcontext) { HeapMonitor *instance = TryGetInstance(); if (instance == nullptr) { return; @@ -411,11 +406,8 @@ bool HeapMonitor::HasGarbageInstrumentation() { return !instance->gc_inst_functions_.empty(); } -void HeapMonitor::InvokeGarbageInstrumentationFunctions(jlong thread_id, - jbyte *name, - int name_length, - int size, - jlong gcontext) { +void HeapMonitor::InvokeGarbageInstrumentationFunctions( + jlong thread_id, jbyte *name, int name_length, int size, jlong gcontext) { HeapMonitor *instance = TryGetInstance(); if (instance == nullptr) { return; @@ -425,8 +417,7 @@ void HeapMonitor::InvokeGarbageInstrumentationFunctions(jlong thread_id, } } -void HeapMonitor::AddGarbageInstrumentation( - GarbageInstrumentationFunction fn) { +void HeapMonitor::AddGarbageInstrumentation(GarbageInstrumentationFunction fn) { HeapMonitor *instance = TryGetInstance(); if (instance == nullptr) { return; @@ -447,7 +438,7 @@ void HeapMonitor::WaitForShutdown() { std::unique_lock lock(gc_waiting_mutex_); // If we are woken up without having been notified, just go back to sleep. - gc_waiting_cv_.wait(lock, [this] { return gc_thread_shutdown; } ); + gc_waiting_cv_.wait(lock, [this] { return gc_thread_shutdown; }); } void HeapMonitor::AddCallback(jvmtiEventCallbacks *callbacks) { @@ -456,7 +447,7 @@ void HeapMonitor::AddCallback(jvmtiEventCallbacks *callbacks) { } // Currently, we enable once and forget about it. -bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, +bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv *jni, int sampling_interval, bool use_jvm_trace) { if (!Supported(jvmti)) { LOG(WARNING) << "Heap sampling is not supported by the JVM, disabling the " @@ -496,8 +487,7 @@ bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, JVMTI_EVENT_GARBAGE_COLLECTION_FINISH, nullptr) != JVMTI_ERROR_NONE) { jvmti->SetEventNotificationMode(JVMTI_DISABLE, - JVMTI_EVENT_SAMPLED_OBJECT_ALLOC, - nullptr); + JVMTI_EVENT_SAMPLED_OBJECT_ALLOC, nullptr); LOG(WARNING) << "Failed to enable garbage collection finish event, " << "disabling the heap sampling monitor"; return false; @@ -514,7 +504,7 @@ bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, // Ensure this is really a singleton i.e. don't recreate it if sampling is // re-enabled. if (heap_monitor_ == nullptr) { - static HeapMonitor* monitor = new HeapMonitor(); + static HeapMonitor *monitor = new HeapMonitor(); if (!monitor->CreateGCWaitingThread(jvmti, jni)) { return false; } @@ -525,7 +515,7 @@ bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, } void HeapMonitor::Disable() { - HeapMonitor* monitor = heap_monitor_.load(); + HeapMonitor *monitor = heap_monitor_.load(); if (monitor == nullptr) { // Already disabled. return; @@ -534,28 +524,26 @@ void HeapMonitor::Disable() { jvmti->SetEventNotificationMode(JVMTI_DISABLE, JVMTI_EVENT_SAMPLED_OBJECT_ALLOC, nullptr); - jvmti->SetEventNotificationMode(JVMTI_DISABLE, - JVMTI_EVENT_GARBAGE_COLLECTION_FINISH, - nullptr); + jvmti->SetEventNotificationMode( + JVMTI_DISABLE, JVMTI_EVENT_GARBAGE_COLLECTION_FINISH, nullptr); // Notify the agent thread that we are done. monitor->ShutdownGCWaitingThread(); heap_monitor_.store(nullptr); } std::unique_ptr HeapMonitor::GetHeapProfiles( - JNIEnv* env, bool force_gc) { + JNIEnv *env, bool force_gc) { // Note: technically this means that you cannot disable the sampler and then // get the profile afterwards; this could be changed if needed. HeapMonitor *monitor = TryGetInstance(); if (monitor != nullptr) { - return monitor->storage_.GetHeapProfiles(env, sampling_interval_, - force_gc); + return monitor->storage_.GetHeapProfiles(env, sampling_interval_, force_gc); } return EmptyHeapProfile(env); } std::unique_ptr HeapMonitor::GetPeakHeapProfiles( - JNIEnv* env, bool force_gc) { + JNIEnv *env, bool force_gc) { // Note: technically this means that you cannot disable the sampler and then // get the profile afterwards; this could be changed if needed. HeapMonitor *monitor = TryGetInstance(); @@ -566,13 +554,13 @@ std::unique_ptr HeapMonitor::GetPeakHeapProfiles( } std::unique_ptr -HeapMonitor::GetGarbageHeapProfiles(JNIEnv* env, bool force_gc) { +HeapMonitor::GetGarbageHeapProfiles(JNIEnv *env, bool force_gc) { // Note: technically this means that you cannot disable the sampler and then // get the profile afterwards; this could be changed if needed. HeapMonitor *monitor = TryGetInstance(); if (monitor != nullptr) { - return monitor->storage_.GetGarbageHeapProfiles( - env, sampling_interval_, force_gc); + return monitor->storage_.GetGarbageHeapProfiles(env, sampling_interval_, + force_gc); } return EmptyHeapProfile(env); } @@ -593,21 +581,22 @@ HeapMonitor::GcEvent HeapMonitor::WaitForGC() { std::unique_lock lock(gc_waiting_mutex_); // If we are woken up without having been notified, just go back to sleep. - gc_waiting_cv_.wait(lock, [this] { return !gc_notify_events_.empty(); } ); + 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, - void* arg) { +void HeapMonitor::GCWaitingThread(jvmtiEnv *jvmti_env, JNIEnv *jni_env, + void *arg) { HeapMonitor *monitor; - while ((monitor = TryGetInstance()) == nullptr) {} + while ((monitor = TryGetInstance()) == nullptr) { + } monitor->GCWaitingThreadRun(jni_env); } -void HeapMonitor::GCWaitingThreadRun(JNIEnv* jni_env) { +void HeapMonitor::GCWaitingThreadRun(JNIEnv *jni_env) { while (true) { GcEvent event = WaitForGC(); @@ -625,7 +614,7 @@ void HeapMonitor::GCWaitingThreadRun(JNIEnv* jni_env) { LOG(INFO) << "Heap sampling GC waiting thread finished"; } -void HeapMonitor::CompactData(JNIEnv* jni_env) { +void HeapMonitor::CompactData(JNIEnv *jni_env) { storage_.CompactSamples(jni_env); } diff --git a/third_party/javaprofiler/heap_sampler.h b/third_party/javaprofiler/heap_sampler.h index 04948689b..156ff1027 100644 --- a/third_party/javaprofiler/heap_sampler.h +++ b/third_party/javaprofiler/heap_sampler.h @@ -30,16 +30,12 @@ #include "third_party/javaprofiler/profile_proto_builder.h" -typedef void (*AllocationInstrumentationFunction)(jlong thread_id, - jbyte *name, - int name_length, - int size, +typedef void (*AllocationInstrumentationFunction)(jlong thread_id, jbyte *name, + int name_length, int size, jlong gcontext); -typedef void (*GarbageInstrumentationFunction)(jlong thread_id, - jbyte *name, - int name_length, - int size, +typedef void (*GarbageInstrumentationFunction)(jlong thread_id, jbyte *name, + int name_length, int size, jlong gcontext); namespace google { @@ -65,34 +61,24 @@ class HeapObjectTrace { : object_(object), size_(size), frames_(frames) {} // Allow moving. - HeapObjectTrace(HeapObjectTrace&& o) = default; - HeapObjectTrace& operator=(HeapObjectTrace&& o) = default; + HeapObjectTrace(HeapObjectTrace &&o) = default; + HeapObjectTrace &operator=(HeapObjectTrace &&o) = default; // No copying allowed. - HeapObjectTrace(const HeapObjectTrace& o) = delete; - HeapObjectTrace& operator=(const HeapObjectTrace& o) = delete; + HeapObjectTrace(const HeapObjectTrace &o) = delete; + HeapObjectTrace &operator=(const HeapObjectTrace &o) = delete; - std::vector &Frames() { - return frames_; - } + std::vector &Frames() { return frames_; } - int Size() const { - return size_; - } + int Size() const { return size_; } - jbyte *Name() const { - return name_; - } + jbyte *Name() const { return name_; } - int NameLength() const { - return name_length_; - } + int NameLength() const { return name_length_; } - jlong ThreadId() const { - return thread_id_; - } + jlong ThreadId() const { return thread_id_; } - void DeleteWeakReference(JNIEnv* env) { + void DeleteWeakReference(JNIEnv *env) { env->DeleteWeakGlobalRef(object_); object_ = nullptr; } @@ -105,9 +91,7 @@ class HeapObjectTrace { // Make copying an explicit operation for the one case we need it (adding // to the peak heapz storage) - HeapObjectTrace Copy() { - return HeapObjectTrace(object_, size_, frames_); - } + HeapObjectTrace Copy() { return HeapObjectTrace(object_, size_, frames_); } private: jweak object_; @@ -124,10 +108,10 @@ class HeapEventStorage { public: typedef std::function GcCallback; - HeapEventStorage(jvmtiEnv *jvmti, - ProfileFrameCache *cache = nullptr, - int max_garbage_size = 200, - GcCallback gc_callback = [](const HeapObjectTrace &t){}); + HeapEventStorage( + jvmtiEnv *jvmti, ProfileFrameCache *cache = nullptr, + int max_garbage_size = 200, + GcCallback gc_callback = [](const HeapObjectTrace &t) {}); // TODO: establish correct shutdown sequence: how do we ensure that // things are not going to go awfully wrong at shutdown, is it this class' job @@ -158,15 +142,15 @@ class HeapEventStorage { // profiles; // setting force_gc to true has a performance impact and is discouraged. std::unique_ptr GetGarbageHeapProfiles( - JNIEnv* env, int sampling_interval, bool force_gc = false) { + JNIEnv *env, int sampling_interval, bool force_gc = false) { return GetProfiles(env, sampling_interval, force_gc, false); } void CompactSamples(JNIEnv *env); // Not copyable or movable. - HeapEventStorage(const HeapEventStorage&) = delete; - HeapEventStorage& operator=(const HeapEventStorage&) = delete; + HeapEventStorage(const HeapEventStorage &) = delete; + HeapEventStorage &operator=(const HeapEventStorage &) = delete; private: // Helper for creating a google::javaprofiler::ProfileStackTrace array @@ -177,7 +161,7 @@ class HeapEventStorage { void AddTrace(HeapObjectTrace &object); - google::javaprofiler::ProfileStackTrace* GetStackTraceData() const { + google::javaprofiler::ProfileStackTrace *GetStackTraceData() const { return stack_trace_data_.get(); } @@ -189,12 +173,11 @@ class HeapEventStorage { std::unique_ptr call_trace_data_; }; - static std::unique_ptr ConvertToProto( ProfileProtoBuilder *builder, std::vector &objects); std::unique_ptr GetProfiles( - JNIEnv* env, int sampling_interval, bool force_gc, bool get_live); + JNIEnv *env, int sampling_interval, bool force_gc, bool get_live); // Add object to the garbage list: it uses a queue with a max size of // max_garbage_size, provided via the constructor. @@ -203,9 +186,8 @@ class HeapEventStorage { // Moves live objects from objects to still_live_objects; live elements from // the objects vector are replaced with nullptr via std::move. - void MoveLiveObjects( - JNIEnv *env, std::vector *objects, - std::vector *still_live_objects); + void MoveLiveObjects(JNIEnv *env, std::vector *objects, + std::vector *still_live_objects); int64_t ProfileSize(const std::vector &objects) const; @@ -230,7 +212,7 @@ class HeapEventStorage { // Due to the JVMTI callback, everything here is static. class HeapMonitor { public: - static bool Enable(jvmtiEnv *jvmti, JNIEnv* jni, int sampling_interval, + static bool Enable(jvmtiEnv *jvmti, JNIEnv *jni, int sampling_interval, bool use_jvm_trace); static void Disable(); @@ -239,36 +221,32 @@ class HeapMonitor { // Returns a perftools::profiles::Profile with the objects provided by the // HeapEventStorage. static std::unique_ptr GetHeapProfiles( - JNIEnv* env, bool force_gc); + JNIEnv *env, bool force_gc); // Returns a perftools::profiles::Profile with the GC'd objects provided by // the HeapEventStorage. static std::unique_ptr GetGarbageHeapProfiles( - JNIEnv* env, bool force_gc); + JNIEnv *env, bool force_gc); // Return the largest profile recorded so far. static std::unique_ptr GetPeakHeapProfiles( - JNIEnv* env, bool force_gc); + JNIEnv *env, bool force_gc); static void AddSample(JNIEnv *jni_env, jthread thread, jobject object, jclass object_klass, jlong size, jbyte *name, jint name_len, jlong thread_id); - static void InvokeAllocationInstrumentationFunctions(jlong thread_id, - jbyte *name, - int name_length, - int size, - jlong gcontext); + static void InvokeAllocationInstrumentationFunctions( + jlong thread_id, jbyte *name, int name_length, int size, jlong gcontext); static void AddAllocationInstrumentation( - AllocationInstrumentationFunction fn); + AllocationInstrumentationFunction fn); static bool HasAllocationInstrumentation(); static void InvokeGarbageInstrumentationFunctions(jlong thread_id, jbyte *name, - int name_length, - int size, + int name_length, int size, jlong gcontext); static void AddGarbageInstrumentation(GarbageInstrumentationFunction fn); @@ -292,39 +270,32 @@ class HeapMonitor { private: static const HeapEventStorage::GcCallback gc_callback_; - HeapMonitor() : storage_(jvmti_.load(), GetFrameCache(), 200, gc_callback_) { - } + HeapMonitor() : storage_(jvmti_.load(), GetFrameCache(), 200, gc_callback_) {} static std::atomic heap_monitor_; // Return a nullptr if heap_monitor_ happens to be empty. You must check // whether the return value is nullptr. - static HeapMonitor *TryGetInstance() { - return heap_monitor_; - } + static HeapMonitor *TryGetInstance() { return heap_monitor_; } ProfileFrameCache *GetFrameCache() { ProfileFrameCache *cache = nullptr; return cache; } - static bool Supported(jvmtiEnv* jvmti); + static bool Supported(jvmtiEnv *jvmti); - enum class GcEvent { - NO_EVENT, - GC_FINISHED, - SHUTDOWN - }; + enum class GcEvent { NO_EVENT, GC_FINISHED, SHUTDOWN }; - bool CreateGCWaitingThread(jvmtiEnv* jvmti, JNIEnv* jni); + bool CreateGCWaitingThread(jvmtiEnv *jvmti, JNIEnv *jni); void ShutdownGCWaitingThread(); static void GCWaitingThread(jvmtiEnv *jvmti_env, JNIEnv *jni_env, void *arg); - void GCWaitingThreadRun(JNIEnv* jni_env); + void GCWaitingThreadRun(JNIEnv *jni_env); GcEvent WaitForGC(); void NotifyGCWaitingThreadInternal(GcEvent event); void WaitForShutdown(); - void CompactData(JNIEnv* jni_env); + void CompactData(JNIEnv *jni_env); static std::vector alloc_inst_functions_; static std::vector gc_inst_functions_; From 7398414cf93d5b38e418ef11bfdaa5d677923d7f Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 7 Dec 2023 19:20:43 +0000 Subject: [PATCH 22/29] Make HeapMonitor::Enable()'s use_jvm_trace parameter default to false. We plan to delete code path for use_jvm_trace == true in the future. PiperOrigin-RevId: 588853678 --- src/entry.cc | 5 +---- third_party/javaprofiler/heap_sampler.h | 4 +++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/entry.cc b/src/entry.cc index ab8686bcc..09d558149 100644 --- a/src/entry.cc +++ b/src/entry.cc @@ -123,11 +123,8 @@ void JNICALL OnVMInit(jvmtiEnv *jvmti, JNIEnv *jni_env, jthread thread) { } if (FLAGS_cprof_enable_heap_sampling) { - // TODO: Allow using the JVM's stack tracer with a flag once - // we can get the current context in a cloud build. if (!google::javaprofiler::HeapMonitor::Enable( - jvmti, jni_env, FLAGS_cprof_heap_sampling_interval, - false /* use_jvm_trace */)) { + jvmti, jni_env, FLAGS_cprof_heap_sampling_interval)) { LOG(WARNING) << "Failed to start HeapMonitor."; } } diff --git a/third_party/javaprofiler/heap_sampler.h b/third_party/javaprofiler/heap_sampler.h index 156ff1027..10ee30cd4 100644 --- a/third_party/javaprofiler/heap_sampler.h +++ b/third_party/javaprofiler/heap_sampler.h @@ -213,7 +213,9 @@ class HeapEventStorage { class HeapMonitor { public: static bool Enable(jvmtiEnv *jvmti, JNIEnv *jni, int sampling_interval, - bool use_jvm_trace); + // TODO: Remove 'use_jvm_trace' and associated + // code after Q2 2024. + bool use_jvm_trace = false); static void Disable(); static bool Enabled() { return heap_monitor_ != nullptr; } From 814af5491c9098a3b9669073f77414527ff186de Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Fri, 8 Dec 2023 22:33:04 +0000 Subject: [PATCH 23/29] Allow users to configure the max number of garbage traces via a parameter to HeapMonitor::Enable(). PiperOrigin-RevId: 589244025 --- third_party/javaprofiler/heap_sampler.cc | 4 ++-- third_party/javaprofiler/heap_sampler.h | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index 14f8f477c..1b3697a19 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -448,7 +448,7 @@ void HeapMonitor::AddCallback(jvmtiEventCallbacks *callbacks) { // Currently, we enable once and forget about it. bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv *jni, int sampling_interval, - bool use_jvm_trace) { + int max_garbage_size, bool use_jvm_trace) { if (!Supported(jvmti)) { LOG(WARNING) << "Heap sampling is not supported by the JVM, disabling the " << " heap sampling monitor"; @@ -504,7 +504,7 @@ bool HeapMonitor::Enable(jvmtiEnv *jvmti, JNIEnv *jni, int sampling_interval, // Ensure this is really a singleton i.e. don't recreate it if sampling is // re-enabled. if (heap_monitor_ == nullptr) { - static HeapMonitor *monitor = new HeapMonitor(); + static HeapMonitor *monitor = new HeapMonitor(max_garbage_size); if (!monitor->CreateGCWaitingThread(jvmti, jni)) { return false; } diff --git a/third_party/javaprofiler/heap_sampler.h b/third_party/javaprofiler/heap_sampler.h index 10ee30cd4..c51510b5b 100644 --- a/third_party/javaprofiler/heap_sampler.h +++ b/third_party/javaprofiler/heap_sampler.h @@ -213,6 +213,7 @@ class HeapEventStorage { class HeapMonitor { public: static bool Enable(jvmtiEnv *jvmti, JNIEnv *jni, int sampling_interval, + int max_garbage_size = 200, // TODO: Remove 'use_jvm_trace' and associated // code after Q2 2024. bool use_jvm_trace = false); @@ -272,7 +273,9 @@ class HeapMonitor { private: static const HeapEventStorage::GcCallback gc_callback_; - HeapMonitor() : storage_(jvmti_.load(), GetFrameCache(), 200, gc_callback_) {} + HeapMonitor(int max_garbage_size) + : storage_(jvmti_.load(), GetFrameCache(), max_garbage_size, + gc_callback_) {} static std::atomic heap_monitor_; From f349ca48d336c2883edffd10f36467033254bb58 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Thu, 14 Dec 2023 20:18:28 +0000 Subject: [PATCH 24/29] Fix overflow in heap_sampler when a single object's size is greater than 2147483647. Changed type for sample size from jint (int32_t) to jlong (int64_t). PiperOrigin-RevId: 591017093 --- third_party/javaprofiler/heap_sampler.cc | 4 ++-- third_party/javaprofiler/heap_sampler.h | 17 ++++++++++------- .../javaprofiler/profile_proto_builder.cc | 2 +- .../javaprofiler/profile_proto_builder.h | 7 ++++--- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/third_party/javaprofiler/heap_sampler.cc b/third_party/javaprofiler/heap_sampler.cc index 1b3697a19..ac7f823da 100644 --- a/third_party/javaprofiler/heap_sampler.cc +++ b/third_party/javaprofiler/heap_sampler.cc @@ -371,7 +371,7 @@ void HeapMonitor::AddSample(JNIEnv *jni_env, jthread thread, jobject object, } void HeapMonitor::InvokeAllocationInstrumentationFunctions( - jlong thread_id, jbyte *name, int name_length, int size, jlong gcontext) { + jlong thread_id, jbyte *name, int name_length, jlong size, jlong gcontext) { HeapMonitor *instance = TryGetInstance(); if (instance == nullptr) { return; @@ -407,7 +407,7 @@ bool HeapMonitor::HasGarbageInstrumentation() { } void HeapMonitor::InvokeGarbageInstrumentationFunctions( - jlong thread_id, jbyte *name, int name_length, int size, jlong gcontext) { + jlong thread_id, jbyte *name, int name_length, jlong size, jlong gcontext) { HeapMonitor *instance = TryGetInstance(); if (instance == nullptr) { return; diff --git a/third_party/javaprofiler/heap_sampler.h b/third_party/javaprofiler/heap_sampler.h index c51510b5b..51b2f61a3 100644 --- a/third_party/javaprofiler/heap_sampler.h +++ b/third_party/javaprofiler/heap_sampler.h @@ -31,11 +31,11 @@ #include "third_party/javaprofiler/profile_proto_builder.h" typedef void (*AllocationInstrumentationFunction)(jlong thread_id, jbyte *name, - int name_length, int size, + int name_length, jlong size, jlong gcontext); typedef void (*GarbageInstrumentationFunction)(jlong thread_id, jbyte *name, - int name_length, int size, + int name_length, jlong size, jlong gcontext); namespace google { @@ -70,7 +70,7 @@ class HeapObjectTrace { std::vector &Frames() { return frames_; } - int Size() const { return size_; } + jlong Size() const { return size_; } jbyte *Name() const { return name_; } @@ -95,7 +95,7 @@ class HeapObjectTrace { private: jweak object_; - int size_; + jlong size_; std::vector frames_; jbyte *name_; int name_length_; @@ -239,8 +239,11 @@ class HeapMonitor { jclass object_klass, jlong size, jbyte *name, jint name_len, jlong thread_id); - static void InvokeAllocationInstrumentationFunctions( - jlong thread_id, jbyte *name, int name_length, int size, jlong gcontext); + static void InvokeAllocationInstrumentationFunctions(jlong thread_id, + jbyte *name, + int name_length, + jlong size, + jlong gcontext); static void AddAllocationInstrumentation( AllocationInstrumentationFunction fn); @@ -249,7 +252,7 @@ class HeapMonitor { static void InvokeGarbageInstrumentationFunctions(jlong thread_id, jbyte *name, - int name_length, int size, + int name_length, jlong size, jlong gcontext); static void AddGarbageInstrumentation(GarbageInstrumentationFunction fn); diff --git a/third_party/javaprofiler/profile_proto_builder.cc b/third_party/javaprofiler/profile_proto_builder.cc index 34bfe582c..b5629fdf4 100644 --- a/third_party/javaprofiler/profile_proto_builder.cc +++ b/third_party/javaprofiler/profile_proto_builder.cc @@ -162,7 +162,7 @@ void ProfileProtoBuilder::AddTrace(const ProfileStackTrace &profile_trace, int32_t count) { const TraceAndLabels &trace_and_labels = profile_trace.trace_and_labels; auto sample = trace_samples_.SampleFor(trace_and_labels); - jint metric_value = profile_trace.metric_value; + auto metric_value = profile_trace.metric_value; if (sample != nullptr) { UpdateSampleValues(sample, count, metric_value); diff --git a/third_party/javaprofiler/profile_proto_builder.h b/third_party/javaprofiler/profile_proto_builder.h index 5814a6d0e..28a79c54d 100644 --- a/third_party/javaprofiler/profile_proto_builder.h +++ b/third_party/javaprofiler/profile_proto_builder.h @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -114,16 +115,16 @@ struct TraceAndLabels { // potential labels associated with this trace. struct ProfileStackTrace { // Constructor for a stack trace without any label. - ProfileStackTrace(const JVMPI_CallTrace *trace = nullptr, jint m_value = 0) + ProfileStackTrace(const JVMPI_CallTrace *trace = nullptr, int64_t m_value = 0) : metric_value(m_value), trace_and_labels(trace) {} // Constructor for a stack trace with labels. - ProfileStackTrace(const JVMPI_CallTrace *trace, jint m_value, + ProfileStackTrace(const JVMPI_CallTrace *trace, int64_t m_value, const std::vector &labels) : metric_value(m_value), trace_and_labels(trace, labels) {} // Metric associated with the trace and labels. - jint metric_value; + int64_t metric_value; // Trace and labels associated with the metric collected. TraceAndLabels trace_and_labels; From 4606afaac6977d556170a5af9a2dfe8912606281 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Wed, 27 Dec 2023 22:50:42 +0000 Subject: [PATCH 25/29] Internal Code Change PiperOrigin-RevId: 594103049 --- src/profiler.h | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/profiler.h b/src/profiler.h index e0aaaa676..5d4f9122f 100644 --- a/src/profiler.h +++ b/src/profiler.h @@ -32,12 +32,13 @@ class SignalHandler { public: SignalHandler() {} + // This type is neither copyable nor movable. + SignalHandler(const SignalHandler &) = delete; + SignalHandler &operator=(const SignalHandler &) = delete; + struct sigaction SetAction(void (*sigaction)(int, siginfo_t *, void *)); bool SetSigprofInterval(int64_t period_usec); - - private: - DISALLOW_COPY_AND_ASSIGN(SignalHandler); }; class Profiler { @@ -50,6 +51,11 @@ class Profiler { jvmti_(jvmti) { Reset(); } + + // This type is neither copyable nor movable. + Profiler(const Profiler &) = delete; + Profiler &operator=(const Profiler &) = delete; + virtual ~Profiler() {} // Collect performance data. @@ -95,8 +101,6 @@ class Profiler { // Number of samples where the stack aggregation failed. static std::atomic unknown_stack_count_; - - DISALLOW_COPY_AND_ASSIGN(Profiler); }; // CPUProfiler collects cpu profiles by setting up a CPU timer and @@ -105,6 +109,10 @@ class CPUProfiler : public Profiler { public: using Profiler::Profiler; + // This type is neither copyable nor movable. + CPUProfiler(const CPUProfiler &) = delete; + CPUProfiler &operator=(const CPUProfiler &) = delete; + // Collect profiling data. bool Collect() override; @@ -116,8 +124,6 @@ class CPUProfiler : public Profiler { // Stop data collection void Stop(); - - DISALLOW_COPY_AND_ASSIGN(CPUProfiler); }; // WallProfiler collects wallclock profiles by explicitly sending From f6843565cd72ab8c2f6851d45faa51aa1e14fb98 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Tue, 2 Jan 2024 19:18:30 +0000 Subject: [PATCH 26/29] Unconditionally define `DISALLOW_COPY_AND_ASSIGN` PiperOrigin-RevId: 595165882 --- third_party/javaprofiler/globals.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/third_party/javaprofiler/globals.h b/third_party/javaprofiler/globals.h index 57669e77b..75787aef7 100644 --- a/third_party/javaprofiler/globals.h +++ b/third_party/javaprofiler/globals.h @@ -29,9 +29,11 @@ using std::hash; using std::string; +#ifndef DISALLOW_COPY_AND_ASSIGN #define DISALLOW_COPY_AND_ASSIGN(TypeName) \ TypeName(const TypeName &); \ void operator=(const TypeName &) +#endif // Short version: reinterpret_cast produces undefined behavior in many // cases where memcpy doesn't. From 0d1fdf525cb7cf37651a302f9ab8baf301139bef Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Tue, 16 Jan 2024 02:23:03 +0000 Subject: [PATCH 27/29] Automated Code Change PiperOrigin-RevId: 598691690 --- third_party/javaprofiler/accessors.h | 10 ++-- .../javaprofiler/async_ref_counted_string.h | 3 +- third_party/javaprofiler/clock.h | 8 ++-- third_party/javaprofiler/method_info.h | 9 ++-- third_party/javaprofiler/native.h | 2 +- .../javaprofiler/profile_proto_builder.h | 46 +++++++++---------- third_party/javaprofiler/stacktraces.h | 21 +++++---- third_party/javaprofiler/tags.h | 8 ++-- 8 files changed, 57 insertions(+), 50 deletions(-) diff --git a/third_party/javaprofiler/accessors.h b/third_party/javaprofiler/accessors.h index ef100d1e3..bf360ecab 100644 --- a/third_party/javaprofiler/accessors.h +++ b/third_party/javaprofiler/accessors.h @@ -17,6 +17,8 @@ #ifndef THIRD_PARTY_JAVAPROFILER_ACCESSORS_H_ #define THIRD_PARTY_JAVAPROFILER_ACCESSORS_H_ +#include + #include "third_party/javaprofiler/globals.h" #include "third_party/javaprofiler/tags.h" @@ -29,8 +31,8 @@ class Accessors { static void SetCurrentJniEnv(JNIEnv *env) { env_ = env; } static JNIEnv *CurrentJniEnv() { return env_; } - static void SetAttribute(int64 value) { attr_ = value; } - static int64 GetAttribute() { return attr_; } + static void SetAttribute(int64_t value) { attr_ = value; } + static int64_t GetAttribute() { return attr_; } // Allocates the current thread's tags storage which can be later retrieved by // GetTags(). If the tags storage is already allocated, asserts an error. @@ -102,11 +104,11 @@ class Accessors { // 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 int64_t attr_ __attribute__((tls_model("global-dynamic"))); static __thread Tags *tags_ __attribute__((tls_model("global-dynamic"))); #else static __thread JNIEnv *env_ __attribute__((tls_model("initial-exec"))); - static __thread int64 attr_ __attribute__((tls_model("initial-exec"))); + static __thread int64_t attr_ __attribute__((tls_model("initial-exec"))); static __thread Tags *tags_ __attribute__((tls_model("initial-exec"))); #endif }; diff --git a/third_party/javaprofiler/async_ref_counted_string.h b/third_party/javaprofiler/async_ref_counted_string.h index 10560fede..63623efb9 100644 --- a/third_party/javaprofiler/async_ref_counted_string.h +++ b/third_party/javaprofiler/async_ref_counted_string.h @@ -18,6 +18,7 @@ #define THIRD_PARTY_JAVAPROFILER_ASYNC_REF_COUNTED_STRING_H_ #include +#include #include "third_party/javaprofiler/globals.h" @@ -86,7 +87,7 @@ class AsyncRefCountedString { // Returns the hash value. As a specific string only has one internal copy, // the stored string address is directly used as the hash value. - uint64 Hash() const { return reinterpret_cast(Get()); } + uint64_t Hash() const { return reinterpret_cast(Get()); } // Initializes the internal string storage. Must be called before using // AsyncRefCountedString to store any string. Should only be called once, diff --git a/third_party/javaprofiler/clock.h b/third_party/javaprofiler/clock.h index 2a7362ed8..a096c5638 100644 --- a/third_party/javaprofiler/clock.h +++ b/third_party/javaprofiler/clock.h @@ -25,8 +25,8 @@ namespace google { namespace javaprofiler { -constexpr int64 kNanosPerSecond = 1000 * 1000 * 1000; -constexpr int64 kNanosPerMilli = 1000 * 1000; +constexpr int64_t kNanosPerSecond = 1000 * 1000 * 1000; +constexpr int64_t kNanosPerMilli = 1000 * 1000; inline struct timespec TimeAdd(const struct timespec t1, const struct timespec t2) { @@ -43,13 +43,13 @@ inline bool TimeLessThan(const struct timespec &t1, const struct timespec &t2) { (t1.tv_sec == t2.tv_sec && t1.tv_nsec < t2.tv_nsec); } -inline struct timespec NanosToTimeSpec(int64 nanos) { +inline struct timespec NanosToTimeSpec(int64_t nanos) { time_t seconds = nanos / kNanosPerSecond; int32_t nano_seconds = nanos % kNanosPerSecond; return timespec{seconds, nano_seconds}; } -inline int64 TimeSpecToNanos(const struct timespec &ts) { +inline int64_t TimeSpecToNanos(const struct timespec &ts) { return kNanosPerSecond * ts.tv_sec + ts.tv_nsec; } diff --git a/third_party/javaprofiler/method_info.h b/third_party/javaprofiler/method_info.h index b3c91742e..4481d66d1 100644 --- a/third_party/javaprofiler/method_info.h +++ b/third_party/javaprofiler/method_info.h @@ -17,6 +17,7 @@ #ifndef GOOGLE_JAVAPROFILER_METHOD_INFO_H_ #define GOOGLE_JAVAPROFILER_METHOD_INFO_H_ +#include #include #include "perftools/profiles/proto/builder.h" @@ -41,14 +42,14 @@ class MethodInfo { start_line_(start_line) {} // An invalid location Id. - static const int64 kInvalidLocationId = 0; + static const int64_t kInvalidLocationId = 0; // Return the location representing info, returns kInvalidLocationId // if not found. - int64 Location(int line_number); + int64_t Location(int line_number); // Put a location ID assocated to a ByteCode Index (BCI). - void AddLocation(int bci, int64 location_id) { + void AddLocation(int bci, int64_t location_id) { locations_[bci] = location_id; } @@ -67,7 +68,7 @@ class MethodInfo { int start_line_; // Cache of jlocation results. - std::unordered_map locations_; + std::unordered_map locations_; }; } // namespace javaprofiler diff --git a/third_party/javaprofiler/native.h b/third_party/javaprofiler/native.h index 29a69e458..5fcd50bec 100644 --- a/third_party/javaprofiler/native.h +++ b/third_party/javaprofiler/native.h @@ -34,7 +34,7 @@ class NativeProcessInfo { explicit NativeProcessInfo(const std::string &procmaps_filename); struct Mapping { - uint64 start, limit; + uint64_t start, limit; std::string name; }; diff --git a/third_party/javaprofiler/profile_proto_builder.h b/third_party/javaprofiler/profile_proto_builder.h index 28a79c54d..29ac9215c 100644 --- a/third_party/javaprofiler/profile_proto_builder.h +++ b/third_party/javaprofiler/profile_proto_builder.h @@ -36,13 +36,13 @@ namespace javaprofiler { // Value and unit for a numerical label. struct NumLabelValue { // Actual value of the label. - const int64 value; + const int64_t value; // Unit for the numerical label. const std::string unit; // Constructor with a value and unit name. - NumLabelValue(int64 v = 0, const std::string &u = "") : value(v), unit(u) {} + NumLabelValue(int64_t v = 0, const std::string &u = "") : value(v), unit(u) {} // Equality operator with another NumLabelValue. bool operator==(const NumLabelValue &other) const { @@ -63,7 +63,7 @@ struct SampleLabel { : key(k), str_label(str), is_string_label(true) {} // Constructor for a numerical label. - SampleLabel(const std::string &k, int64 num, const std::string &unit = "") + SampleLabel(const std::string &k, int64_t num, const std::string &unit = "") : key(k), num_label(num, unit), is_string_label(false) {} // The label key. @@ -100,7 +100,7 @@ struct TraceAndLabels { } // Adds a numeric label to the associated trace. - void AddLabel(const std::string &key, int64 num_value, + void AddLabel(const std::string &key, int64_t num_value, const std::string &unit = "") { labels.push_back({key, num_value, unit}); } @@ -223,8 +223,7 @@ class ProfileProtoBuilder { // Adds traces to the proto, where each trace has a defined count // of occurrences. The traces array must not be deleted // before calling CreateProto. - void AddTraces(const ProfileStackTrace *traces, - const int32 *counts, + void AddTraces(const ProfileStackTrace *traces, const int32_t *counts, int num_traces); // Adds a "fake" trace with a single frame. Used to represent JVM @@ -243,18 +242,18 @@ class ProfileProtoBuilder { // 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, + JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64_t sampling_rate, ProfileFrameCache *cache = nullptr); static std::unique_ptr ForCpu(JNIEnv *jni_env, jvmtiEnv *jvmti_env, - int64 duration_ns, - int64 sampling_rate, + int64_t duration_ns, + int64_t sampling_rate, ProfileFrameCache *cache); static std::unique_ptr ForContention( - JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64 duration_ns, - int64 sampling_rate, ProfileFrameCache *cache); + JNIEnv *jni_env, jvmtiEnv *jvmti_env, int64_t duration_ns, + int64_t sampling_rate, ProfileFrameCache *cache); protected: struct SampleType { @@ -269,7 +268,7 @@ class ProfileProtoBuilder { // information about native frames can be provided. The proto buffer will then // contain "Unknown native method" frames. ProfileProtoBuilder(JNIEnv *env, jvmtiEnv *jvmti_env, - ProfileFrameCache *native_cache, int64 sampling_rate, + ProfileFrameCache *native_cache, int64_t sampling_rate, const SampleType &count_type, const SampleType &metric_type); @@ -289,16 +288,16 @@ class ProfileProtoBuilder { std::unique_ptr CreateSampledProto(); perftools::profiles::Builder builder_; - int64 sampling_rate_ = 0; + int64_t sampling_rate_ = 0; private: void AddSampleType(const SampleType &sample_type); void SetPeriodType(const SampleType &metric_type); - void InitSampleValues(perftools::profiles::Sample *sample, int64 count, - int64 metric); - void UpdateSampleValues(perftools::profiles::Sample *sample, int64 count, - int64 size); - void AddTrace(const ProfileStackTrace &trace, int32 count); + void InitSampleValues(perftools::profiles::Sample *sample, int64_t count, + int64_t metric); + void UpdateSampleValues(perftools::profiles::Sample *sample, int64_t count, + int64_t size); + void AddTrace(const ProfileStackTrace &trace, int32_t count); void AddJavaInfo(const JVMPI_CallFrame &jvm_frame, perftools::profiles::Profile *profile, perftools::profiles::Sample *sample); @@ -308,7 +307,7 @@ class ProfileProtoBuilder { void UnsampleMetrics(); MethodInfo *Method(jmethodID id); - int64 Location(MethodInfo *method, const JVMPI_CallFrame &frame); + int64_t Location(MethodInfo *method, const JVMPI_CallFrame &frame); void AddLabels(const TraceAndLabels &trace_and_labels, perftools::profiles::Sample *sample); @@ -340,12 +339,13 @@ class ProfileProtoBuilder { // on a poisson process to determine which samples to collect, based // on the desired average collection rate R. The probability of a // sample of size S to appear in that profile is 1-exp(-S/R). -double CalculateSamplingRatio(int64 rate, int64 count, int64 metric_value); +double CalculateSamplingRatio(int64_t rate, int64_t count, + int64_t metric_value); class CpuProfileProtoBuilder : public ProfileProtoBuilder { public: CpuProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, - int64 duration_ns, int64 sampling_rate, + int64_t duration_ns, int64_t sampling_rate, ProfileFrameCache *cache) : ProfileProtoBuilder( jni_env, jvmti_env, cache, sampling_rate, @@ -366,7 +366,7 @@ class CpuProfileProtoBuilder : public ProfileProtoBuilder { class HeapProfileProtoBuilder : public ProfileProtoBuilder { public: HeapProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, - int64 sampling_rate, ProfileFrameCache *cache) + int64_t sampling_rate, ProfileFrameCache *cache) : ProfileProtoBuilder( jni_env, jvmti_env, cache, sampling_rate, ProfileProtoBuilder::SampleType("inuse_objects", "count"), @@ -398,7 +398,7 @@ class HeapProfileProtoBuilder : public ProfileProtoBuilder { class ContentionProfileProtoBuilder : public ProfileProtoBuilder { public: ContentionProfileProtoBuilder(JNIEnv *jni_env, jvmtiEnv *jvmti_env, - int64 duration_nanos, int64 sampling_rate, + int64_t duration_nanos, int64_t sampling_rate, ProfileFrameCache *cache) : ProfileProtoBuilder( jni_env, jvmti_env, cache, sampling_rate, diff --git a/third_party/javaprofiler/stacktraces.h b/third_party/javaprofiler/stacktraces.h index 747181256..b491dc6d1 100644 --- a/third_party/javaprofiler/stacktraces.h +++ b/third_party/javaprofiler/stacktraces.h @@ -20,6 +20,7 @@ #define THIRD_PARTY_JAVAPROFILER_STACKTRACES_H_ #include +#include #include // NOLINT #include #include @@ -33,7 +34,7 @@ namespace javaprofiler { // Maximum number of frames to store from the stack traces sampled. const int kMaxFramesToCapture = 128; -uint64 CalculateHash(int64 attr, int num_frames, +uint64_t CalculateHash(int64_t attr, int num_frames, const JVMPI_CallFrame *frame); bool Equal(int num_frames, const JVMPI_CallFrame *f1, const JVMPI_CallFrame *f2); @@ -137,10 +138,10 @@ class AsyncSafeTraceMultiset { // there is no valid trace at this location. This operation is // thread safe with respect to Add() but only a single call to // Extract can be done at a time. - int Extract(int location, int64 *attr, int max_frames, - JVMPI_CallFrame *frames, int64 *count); + int Extract(int location, int64_t *attr, int max_frames, + JVMPI_CallFrame *frames, int64_t *count); - int64 MaxEntries() const { return kMaxStackTraces; } + int64_t MaxEntries() const { return kMaxStackTraces; } private: struct TraceData { @@ -155,7 +156,7 @@ class AsyncSafeTraceMultiset { // Number of times a trace has been encountered. // 0 indicates that the trace is unused // <0 values are reserved, used for concurrency control. - std::atomic count; + std::atomic count; // Number of active attempts to increase the counter on the trace. std::atomic active_updates; }; @@ -166,7 +167,7 @@ class AsyncSafeTraceMultiset { static const int kMaxStackTraces = 2048; // Sentinel to use as trace count while the frames are being updated. - static const int64 kTraceCountLocked = -1; + static const int64_t kTraceCountLocked = -1; TraceData traces_[kMaxStackTraces]; DISALLOW_COPY_AND_ASSIGN(AsyncSafeTraceMultiset); @@ -181,7 +182,7 @@ class TraceMultiset { private: typedef struct { std::vector frames; - int64 attr; + int64_t attr; } CallTrace; struct CallTraceHash { @@ -203,7 +204,7 @@ class TraceMultiset { } }; - typedef std::unordered_map + typedef std::unordered_map CountMap; public: @@ -211,8 +212,8 @@ class TraceMultiset { // Add a trace to the array. If it is already in the array, // increment its count. - void Add(int64 attr, int num_frames, JVMPI_CallFrame *frames, - int64 count); + void Add(int64_t attr, int num_frames, JVMPI_CallFrame *frames, + int64_t count); typedef CountMap::iterator iterator; typedef CountMap::const_iterator const_iterator; diff --git a/third_party/javaprofiler/tags.h b/third_party/javaprofiler/tags.h index 918af1bc1..a84da3a6e 100644 --- a/third_party/javaprofiler/tags.h +++ b/third_party/javaprofiler/tags.h @@ -16,6 +16,8 @@ #ifndef THIRD_PARTY_JAVAPROFILER_TAGS_H_ #define THIRD_PARTY_JAVAPROFILER_TAGS_H_ + +#include #include #include @@ -47,8 +49,8 @@ class Tags { // Legacy interfaces. The attribute value is stored internally indexed by a // special key (kAttrKey). - void SetAttribute(int64 value); - int64 GetAttribute() const; + void SetAttribute(int64_t value); + int64_t GetAttribute() const; // Async-signal-safe version of operator=(const Tags &other). It requires that // the instance should be empty (does not refer to any string). Otherwise, it @@ -61,7 +63,7 @@ class Tags { // Async-signal-safe. bool operator==(const Tags &other) const; // Async-signal-safe. - uint64 Hash() const; + uint64_t Hash() const; // Async-signal-safe. static const Tags &Empty(); From 5cac52e93b40ea228616ce38b5bf767cd3e20802 Mon Sep 17 00:00:00 2001 From: cloud-profiler-team Date: Wed, 17 Jan 2024 18:43:35 +0000 Subject: [PATCH 28/29] Sync changes to profile.proto. PiperOrigin-RevId: 599222268 --- third_party/perftools/profiles/proto/profile.proto | 2 ++ 1 file changed, 2 insertions(+) diff --git a/third_party/perftools/profiles/proto/profile.proto b/third_party/perftools/profiles/proto/profile.proto index 2a239ce96..c154c83e3 100644 --- a/third_party/perftools/profiles/proto/profile.proto +++ b/third_party/perftools/profiles/proto/profile.proto @@ -208,6 +208,8 @@ message Line { uint64 function_id = 1; // Line number in source code. int64 line = 2; + // Column number in source code. + int64 column = 3; } message Function { From 9037e4d9fb2469e075b2f232d9821199121bb8ee Mon Sep 17 00:00:00 2001 From: sharmapranav Date: Thu, 15 Feb 2024 16:12:47 +0000 Subject: [PATCH 29/29] chore: update gRPC to v1.36.4 PiperOrigin-RevId: 607338479 --- Dockerfile | 68 ++++++++++++++++++++++++++++++++++++----------- Dockerfile.alpine | 45 ++++++++++++++++++++++--------- Makefile | 22 +++++---------- 3 files changed, 92 insertions(+), 43 deletions(-) diff --git a/Dockerfile b/Dockerfile index e508a6fb4..268703e38 100644 --- a/Dockerfile +++ b/Dockerfile @@ -20,11 +20,22 @@ FROM ubuntu:xenial # # Dependencies # +# Install JDK 11 as sampling heap profiler depends on the new JVMTI APIs +# Install Kitware apt repository. This repository contains newer version of +# CMake. CMake > 3.13 is required to use "module" mode for gRPC dependencies. +# See https://github.com/grpc/grpc/blob/master/BUILDING.md#install-after-build. +# Kitware installs CMake v3.20.5 -# Install JDK 11 as sampling heap profiler depends on the new JVMTI APIs. -RUN apt-get update && apt-get install -y software-properties-common -RUN add-apt-repository -y ppa:openjdk-r/ppa -RUN apt-get update +RUN apt-get update && apt-get install -y software-properties-common apt-transport-https ca-certificates gnupg wget && \ + add-apt-repository -y ppa:openjdk-r/ppa && \ + test -f /usr/share/doc/kitware-archive-keyring/copyright || \ + wget -O - https://apt.kitware.com/keys/kitware-archive-latest.asc 2>/dev/null \ + | gpg --dearmor - \ + | tee /usr/share/keyrings/kitware-archive-keyring.gpg >/dev/null && \ + echo 'deb [signed-by=/usr/share/keyrings/kitware-archive-keyring.gpg] https://apt.kitware.com/ubuntu/ xenial main' \ + | tee /etc/apt/sources.list.d/kitware.list >/dev/null && \ + apt-get update && \ + apt-get install kitware-archive-keyring # Installing openjdk-11-jdk-headless can be very slow due to repo download # speed. @@ -41,6 +52,7 @@ RUN apt-get update && apt-get -y -q install \ libtool \ make \ openjdk-11-jdk-headless \ + pkg-config \ unzip \ zlib1g-dev @@ -81,7 +93,7 @@ RUN mkdir /tmp/glog && cd /tmp/glog && \ make -j && make install && \ cd ~ && rm -rf /tmp/glog -# gRPC & protobuf +# gRPC & protobuf - build using CMake # 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 @@ -89,13 +101,39 @@ RUN mkdir /tmp/glog && cd /tmp/glog && \ # OpenSSL library itself. # Limit the number of threads used by make, as unlimited threads causes # memory exhausted error on the Kokoro VM. -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 -j4 && make install && ldconfig && \ - cd ../.. && \ - CPPFLAGS="-I /usr/local/ssl/include" LDFLAGS="-L /usr/local/ssl/lib/ -Wl,--no-as-needed" make -j4 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 +# -DCMAKE_CXX_STANDARD=11 is necessary since it is the minimum version supported +# by gRPC dependencies. Xenial sets default version to c++98. +# +# See https://github.com/grpc/grpc/blob/v1.36.4/test/distrib/cpp/run_distrib_test_cmake_pkgconfig.sh +RUN git clone --depth=1 --recursive -b v1.36.4 https://github.com/grpc/grpc.git /tmp/grpc && \ + cd /tmp/grpc/ && \ + # Install protobuf + mkdir -p third_party/protobuf/cmake/build && \ + (cd third_party/protobuf/cmake/build && \ + cmake -Dprotobuf_BUILD_TESTS=OFF -DCMAKE_CXX_STANDARD=11 -DCMAKE_POSITION_INDEPENDENT_CODE=TRUE -DCMAKE_BUILD_TYPE=Release .. && \ + make -j4 install) && \ + # Install finish - protobuf + # Install gRPC + mkdir -p cmake/build && \ + (cd cmake/build && \ + cmake \ + -DOPENSSL_ROOT_DIR=/usr/local/ssl \ + -DOPENSSL_INCLUDE_DIR=/usr/local/ssl/include \ + -DOPENSSL_CRYPTO_LIB=/usr/local/ssl/lib \ + -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_CXX_STANDARD=11 \ + -DCMAKE_INSTALL_PREFIX=/usr/local/grpc \ + -DgRPC_INSTALL=ON \ + -DgRPC_BUILD_TESTS=OFF \ + -DgRPC_ABSL_PROVIDER=module \ + -DgRPC_CARES_PROVIDER=module \ + -DgRPC_RE2_PROVIDER=module \ + -DgRPC_ZLIB_PROVIDER=module \ + -DgRPC_PROTOBUF_PROVIDER=package \ + -DgRPC_SSL_PROVIDER=package \ + ../.. && \ + make -j4 install) && \ + cd ~ && rm -rf /tmp/grpc + +ENV PKG_CONFIG_PATH="${PKG_CONFIG_PATH}:/usr/local/ssl/lib/pkgconfig:/usr/local/grpc/lib/pkgconfig:/usr/local/lib64/pkgconfig" +ENV PATH="${PATH}:/usr/local/grpc/bin" diff --git a/Dockerfile.alpine b/Dockerfile.alpine index 5eda83bd4..f5670e196 100644 --- a/Dockerfile.alpine +++ b/Dockerfile.alpine @@ -44,7 +44,6 @@ RUN apk --no-cache add \ # 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_1t.tar.gz | \ @@ -80,7 +79,7 @@ RUN mkdir /tmp/glog && cd /tmp/glog && \ make -j && make install && \ cd ~ && rm -rf /tmp/glog -# gRPC & protobuf +# gRPC & protobuf - build using CMake # 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 @@ -88,13 +87,35 @@ RUN mkdir /tmp/glog && cd /tmp/glog && \ # OpenSSL library itself. # Limit the number of threads used by make, as unlimited threads causes # memory exhausted error on the Kokoro VM. -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 -j4 && make install && ldconfig / && \ - cd ../.. && \ - CPPFLAGS="-I /usr/local/ssl/include" LDFLAGS="-L /usr/local/ssl/lib/ -Wl,--no-as-needed" make -j4 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 +# +# See https://github.com/grpc/grpc/blob/v1.36.4/test/distrib/cpp/run_distrib_test_cmake_pkgconfig.sh +RUN git clone --depth=1 --recursive -b v1.36.4 https://github.com/grpc/grpc.git /tmp/grpc && \ + cd /tmp/grpc/ && \ + # Install protobuf + mkdir -p third_party/protobuf/cmake/build && \ + (cd third_party/protobuf/cmake/build && \ + cmake -Dprotobuf_BUILD_TESTS=OFF -DCMAKE_POSITION_INDEPENDENT_CODE=TRUE -DCMAKE_BUILD_TYPE=Release .. && \ + make -j4 install) && \ + # Install gRPC + mkdir -p cmake/build && \ + cd cmake/build && \ + cmake \ + -DOPENSSL_ROOT_DIR=/usr/local/ssl \ + -DOPENSSL_INCLUDE_DIR=/usr/local/ssl/include \ + -DOPENSSL_CRYPTO_LIB=/usr/local/ssl/lib \ + -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_INSTALL_PREFIX=/usr/local/grpc \ + -DgRPC_INSTALL=ON \ + -DgRPC_BUILD_TESTS=OFF \ + -DgRPC_ABSL_PROVIDER=module \ + -DgRPC_CARES_PROVIDER=module \ + -DgRPC_RE2_PROVIDER=module \ + -DgRPC_ZLIB_PROVIDER=module \ + -DgRPC_PROTOBUF_PROVIDER=package \ + -DgRPC_SSL_PROVIDER=package \ + ../.. && \ + make -j4 install && \ + cd ~ && rm -rf /tmp/grpc + +ENV PKG_CONFIG_PATH="${PKG_CONFIG_PATH}:/usr/local/ssl/lib/pkgconfig:/usr/local/grpc/lib/pkgconfig:/usr/local/lib64/pkgconfig" +ENV PATH="${PATH}:/usr/local/grpc/bin" diff --git a/Makefile b/Makefile index 6b83cb67a..9bd1ede3a 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,8 @@ CFLAGS = \ -Wno-array-bounds \ -g0 \ -DSTANDALONE_BUILD \ - -D_GNU_SOURCE + -D_GNU_SOURCE \ + $(shell pkg-config --cflags protobuf grpc) ifeq ($(machine_type),$(filter $(machine_type),aarch64 arm64)) # Building on an ARM64 machine. @@ -50,7 +51,7 @@ endif SRC_ROOT_PATH=. PROTOC ?= /usr/local/bin/protoc -PROTOC_GEN_GRPC ?= /usr/local/bin/grpc_cpp_plugin +GRPC_CPP_PLUGIN_PATH ?= /usr/local/grpc/bin/grpc_cpp_plugin PROFILE_PROTO_PATH ?= third_party/perftools/profiles/proto JAVA_AGENT_PATH ?= src @@ -162,7 +163,7 @@ HEADERS = \ VERSION_SCRIPT = $(JAVA_AGENT_PATH)/cloud_profiler_java_agent.lds OPT_FLAGS = -O3 -LDFLAGS = -static-libstdc++ -shared +LDFLAGS = -L/usr/local/lib -static-libstdc++ -shared $(shell pkg-config --libs --static protobuf grpc++) LDS_FLAGS = -Wl,-z,defs -Wl,--version-script=$(VERSION_SCRIPT) LIB_ROOT_PATH ?= /usr/local @@ -178,17 +179,6 @@ LIBS1= \ $(LIB_ROOT_PATH)/lib/libglog.a \ $(LIB_ROOT_PATH)/lib/libgflags.a \ -LIBS2= \ - $(LIB_ROOT_PATH)/lib/libprotobuf.a \ - $(LIB_ROOT_PATH)/ssl/lib/libssl.a \ - $(LIB_ROOT_PATH)/ssl/lib/libcrypto.a \ - -lz \ - -GRPC_LIBS= \ - $(LIB_ROOT_PATH)/lib/libgrpc++.a \ - $(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 @@ -214,7 +204,7 @@ clean: $(TARGET_AGENT): $(SOURCES) $(HEADERS) mkdir -p $(dir $@) - $(CC) $(INCLUDES) $(CFLAGS) $(OPT_FLAGS) $(LDFLAGS) $(SOURCES) $(LIBS1) $(GRPC_LIBS) $(LIBS2) -o $@ $(LDS_FLAGS) + $(CC) $(INCLUDES) $(CFLAGS) $(OPT_FLAGS) $(SOURCES) $(LIBS1) $(LDFLAGS) -o $@ $(LDS_FLAGS) $(TARGET_NOTICES): $(JAVA_AGENT_PATH)/NOTICES mkdir -p $(dir $@) @@ -227,7 +217,7 @@ $(GENFILES_PATH)/%profiler.pb.h $(GENFILES_PATH)/%profiler.pb.cc : third_party/g $(GENFILES_PATH)/%profiler.grpc.pb.h $(GENFILES_PATH)/%profiler.grpc.pb.cc : third_party/googleapis/%profiler.proto mkdir -p $(dir $@) $(PROTOC) -Ithird_party/googleapis -I$(PROTOBUF_INCLUDE_PATH) \ - --plugin=protoc-gen-grpc=$(PROTOC_GEN_GRPC) --grpc_out=services_namespace=grpc:$(GENFILES_PATH) $< + --plugin=protoc-gen-grpc=$(GRPC_CPP_PLUGIN_PATH) --grpc_out=services_namespace=grpc:$(GENFILES_PATH) $< $(GENFILES_PATH)/%annotations.pb.h $(GENFILES_PATH)/%annotations.pb.cc : third_party/googleapis/%annotations.proto mkdir -p $(dir $@)