From 0db398d717fce47af64d7dda32c936de25439d22 Mon Sep 17 00:00:00 2001 From: jbassett Date: Thu, 16 Apr 2026 11:57:46 -0700 Subject: [PATCH 1/5] Replace redundant `absl::NullSafeStringView(str.c_str())` with `str`. `c_str()` is already guaranteed to be non-null. `data()` is also usually non-null, but if it is null, then the string_view is empty, so the `NullSafeStringView` wrapper is still a no-op. PiperOrigin-RevId: 900840592 --- src/perf_data_converter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/perf_data_converter.cc b/src/perf_data_converter.cc index 331f807..b6da6a3 100644 --- a/src/perf_data_converter.cc +++ b/src/perf_data_converter.cc @@ -59,7 +59,7 @@ const char* ExecModeString(quipper::AddressContext context) { // In order to successfully unmarshal the proto in Go, all strings inserted into // the profile string table must be valid UTF-8. int64_t UTF8StringId(const std::string& s, ProfileBuilder* builder) { - return builder->StringId(absl::NullSafeStringView(s.c_str())); + return builder->StringId(s); } // List of profile location IDs, currently used to represent a call stack. From b878b6fa503545125511d35b723499abeef7969e Mon Sep 17 00:00:00 2001 From: jbassett Date: Thu, 23 Apr 2026 09:39:25 -0700 Subject: [PATCH 2/5] Use `absl::string_view` now that StringId supports it Pass `absl::string_view` parameters, eliminate redundant `std::string` objects which existed to get a `.c_str()`, and so on. PiperOrigin-RevId: 904499066 --- src/perf_data_converter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/perf_data_converter.cc b/src/perf_data_converter.cc index b6da6a3..7f21153 100644 --- a/src/perf_data_converter.cc +++ b/src/perf_data_converter.cc @@ -58,7 +58,7 @@ const char* ExecModeString(quipper::AddressContext context) { // this also ensures the string contains structurally valid UTF-8. // In order to successfully unmarshal the proto in Go, all strings inserted into // the profile string table must be valid UTF-8. -int64_t UTF8StringId(const std::string& s, ProfileBuilder* builder) { +int64_t UTF8StringId(absl::string_view s, ProfileBuilder* builder) { return builder->StringId(s); } From d7420a2233be616deb562e687ad9cee926c3c45f Mon Sep 17 00:00:00 2001 From: jbassett Date: Mon, 11 May 2026 11:00:43 -0700 Subject: [PATCH 3/5] Use `string_view` and Abseil string utilities PiperOrigin-RevId: 913783581 --- src/BUILD | 1 + src/perf_data_converter.cc | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/BUILD b/src/BUILD index e864508..263f6b6 100644 --- a/src/BUILD +++ b/src/BUILD @@ -43,6 +43,7 @@ cc_library( ":perf_data_handler", ":builder", ":profile_cc_proto", + "@abseil-cpp//absl/strings", "@abseil-cpp//absl/strings:string_view", "//src/quipper:address_context", "//src/quipper:kernel", diff --git a/src/perf_data_converter.cc b/src/perf_data_converter.cc index 7f21153..29c6cb3 100644 --- a/src/perf_data_converter.cc +++ b/src/perf_data_converter.cc @@ -20,7 +20,9 @@ #include "src/quipper/base/logging.h" #include "src/builder.h" +#include "absl/strings/match.h" #include "absl/strings/string_view.h" +#include "absl/strings/strip.h" #include "src/perf_data_handler.h" #include "src/quipper/address_context.h" #include "src/quipper/perf_data.pb.h" From 078bd3a3951bdda7500effaddf640a90540bc5ad Mon Sep 17 00:00:00 2001 From: skanev Date: Tue, 12 May 2026 18:21:29 -0700 Subject: [PATCH 4/5] Don't CHECK-fail when we can't remap mmap events. PiperOrigin-RevId: 914579485 --- src/quipper/perf_parser.cc | 51 ++++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/src/quipper/perf_parser.cc b/src/quipper/perf_parser.cc index 3980a8f..694dc53 100644 --- a/src/quipper/perf_parser.cc +++ b/src/quipper/perf_parser.cc @@ -108,7 +108,9 @@ bool PerfParser::ParseRawEvents() { } parsed_events_.resize(write_index); - ProcessEvents(); + if (!ProcessEvents()) { + return false; + } if (!options_.discard_unused_events) return true; @@ -190,7 +192,8 @@ bool PerfParser::ProcessEvents() { // Process user events if (event.header().type() >= PERF_RECORD_USER_TYPE_START) { if (!ProcessUserEvents(event)) { - return false; + LOG(WARNING) << "Failed processing user event."; + return true; } continue; } @@ -214,8 +217,14 @@ bool PerfParser::ProcessEvents() { (event.header().misc() & quipper::PERF_RECORD_MISC_CPUMODE_MASK) == quipper::PERF_RECORD_MISC_KERNEL; // Use the array index of the current mmap event as a unique identifier. - CHECK(MapMmapEvent(event.mutable_mmap_event(), i, is_kernel)) - << "Unable to map " << mmap_type_name << " event!"; + if (!MapMmapEvent(event.mutable_mmap_event(), i, is_kernel)) { + LOG(ERROR) << "Unable to map " << mmap_type_name << " event! " + << "file: " << event.mmap_event().filename() << " " + << "addr: " << std::hex << event.mmap_event().start() + << " " + << "len: " << event.mmap_event().len() << std::dec; + return false; + } // No samples in this MMAP region yet, hopefully. parsed_event.num_samples_in_mmap_region = 0; DSOInfo dso_info; @@ -239,7 +248,10 @@ bool PerfParser::ProcessEvents() { << ":" << event.fork_event().tid(); // clang-format on ++stats_.num_fork_events; - CHECK(MapForkEvent(event.fork_event())) << "Unable to map FORK event!"; + if (!MapForkEvent(event.fork_event())) { + LOG(ERROR) << "Unable to map FORK event!"; + return false; + } break; case PERF_RECORD_EXIT: // EXIT events have the same structure as FORK events. @@ -256,7 +268,10 @@ bool PerfParser::ProcessEvents() { << event.comm_event().comm(); // clang-format on ++stats_.num_comm_events; - CHECK(MapCommEvent(event.comm_event())); + if (!MapCommEvent(event.comm_event())) { + LOG(ERROR) << "Unable to map COMM event!"; + return false; + } commands_.insert(event.comm_event().comm()); const PidTid pidtid = std::make_pair(event.comm_event().pid(), event.comm_event().tid()); @@ -284,12 +299,15 @@ bool PerfParser::ProcessEvents() { << ". Doing nothing."; break; default: - LOG(ERROR) << "Unknown event type: " - << GetEventName(event.header().type()); - return false; + LOG(WARNING) << "Unknown event type: " + << GetEventName(event.header().type()); + return true; } } - if (!FillInDsoBuildIds()) return false; + if (!FillInDsoBuildIds()) { + LOG(WARNING) << "Failed filling in buildIDs"; + return true; + } // Print stats collected from parsing. // clang-format off @@ -312,9 +330,9 @@ bool PerfParser::ProcessEvents() { LOG(INFO) << "Input perf.data has no sample events due to " "PERF_RECORD_SAMPLE being skipped."; } else { - LOG(ERROR) << "Input perf.data has no sample events."; + LOG(WARNING) << "Input perf.data has no sample events."; } - return false; + return true; } float sample_mapping_percentage = @@ -322,10 +340,11 @@ bool PerfParser::ProcessEvents() { stats_.num_sample_events * 100.; float threshold = options_.sample_mapping_percentage_threshold; if (sample_mapping_percentage < threshold) { - LOG(ERROR) << "Only " << static_cast(sample_mapping_percentage) - << "% of samples had all locations mapped to a module, expected " - << "at least " << static_cast(threshold) << "%"; - return false; + LOG(WARNING) + << "Only " << static_cast(sample_mapping_percentage) + << "% of samples had all locations mapped to a module, expected " + << "at least " << static_cast(threshold) << "%"; + return true; } return true; } From 270559479c097a17ba96d59a9039fa2135125d6e Mon Sep 17 00:00:00 2001 From: jbassett Date: Wed, 13 May 2026 09:00:05 -0700 Subject: [PATCH 5/5] Use `absl::string_view` over `absl::NullSafeStringView` - perf_data_converter.cc: `ExecModeString` returns string literals. PiperOrigin-RevId: 914893664 --- src/perf_data_converter.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/perf_data_converter.cc b/src/perf_data_converter.cc index 29c6cb3..c124ff1 100644 --- a/src/perf_data_converter.cc +++ b/src/perf_data_converter.cc @@ -38,7 +38,7 @@ typedef perftools::profiles::Builder ProfileBuilder; typedef uint32_t Pid; typedef uint32_t Tid; -const char* ExecModeString(quipper::AddressContext context) { +absl::string_view ExecModeString(quipper::AddressContext context) { switch (context) { case quipper::AddressContext::kHostKernel: return ExecutionModeHostKernel; @@ -684,8 +684,7 @@ void PerfDataConverter::AddOrUpdateSample( sample_key.exec_mode != quipper::AddressContext::kUnknown) { auto* label = sample->add_label(); label->set_key(builder->StringIdForMigration(ExecutionModeLabelKey)); - label->set_str(builder->StringId( - absl::NullSafeStringView(ExecModeString(sample_key.exec_mode)))); + label->set_str(builder->StringId(ExecModeString(sample_key.exec_mode))); } if (IncludeThreadTypeLabels() && sample_key.thread_type != 0) { auto* label = sample->add_label();