Update to latest internal revision. (#108)

* Remove compiler warnings, mostly by comparing sizes better wrt signedness.

Stop relying on implicit conversions.
Use multiline comments where a backslash is necessary to show perf commandlines.

PiperOrigin-RevId: 399473470

* Add an icon indicating status with a link to Github Actions from our README.

PiperOrigin-RevId: 400840383

* Add {code,data}_page_size label support

PiperOrigin-RevId: 401781351

* Fix typo

PiperOrigin-RevId: 404883340

Co-authored-by: namhyung <namhyung@google.com>

README.md

@@ -1,3 +1,5 @@
+[![Github Action CI](https://github.com/google/perf_data_converter/workflows/ci/badge.svg)](https://github.com/google/perf_data_converter/actions)
+
 # Introduction
 
 The `perf_to_profile` binary can be used to turn a perf.data file, which is

src/BUILD

@@ -81,6 +81,7 @@ cc_test(
         "//src/testdata:perf-callchain-non-pebs.textproto",
         "//src/testdata:perf-callchain-pebs.textproto",
         "//src/testdata:perf-cgroup-events.textproto",
+        "//src/testdata:perf-code-data-page-sizes.textproto",
         "//src/testdata:perf-comm-and-task-comm.textproto",
         "//src/testdata:perf-cros-kernel-3_18-mapping.textproto",
         "//src/testdata:perf-include-comm-md5-prefix.textproto",

src/perf_data_converter.cc

@@ -108,6 +108,8 @@ struct SampleKey {
   uint64 thread_comm = 0;
   // The index of the sample's cgroup name in the profiles's string table.
   uint64 cgroup = 0;
+  uint64 code_page_size = 0;
+  uint64 data_page_size = 0;
   LocationIdVector stack;
 };
 
@@ -117,7 +119,8 @@ struct SampleKeyEqualityTester {
             (a.exec_mode == b.exec_mode) && (a.comm == b.comm) &&
             (a.thread_type == b.thread_type) &&
             (a.thread_comm == b.thread_comm) && (a.cgroup == b.cgroup) &&
-            (a.stack == b.stack));
+            (a.code_page_size == b.code_page_size) &&
+            (a.data_page_size == b.data_page_size) && (a.stack == b.stack));
   }
 };
 
@@ -132,6 +135,8 @@ struct SampleKeyHasher {
     hash ^= std::hash<uint64>()(k.thread_type);
     hash ^= std::hash<uint64>()(k.thread_comm);
     hash ^= std::hash<uint64>()(k.cgroup);
+    hash ^= std::hash<uint64>()(k.code_page_size);
+    hash ^= std::hash<uint64>()(k.data_page_size);
     for (const auto& id : k.stack) {
       hash ^= std::hash<uint64>()(id);
     }
@@ -269,6 +274,16 @@ class PerfDataConverter : public PerfDataHandler {
   // Returns whether cgroup labels were requested for inclusion in the
   // profile.proto's Sample.Label field.
   bool IncludeCgroupLabels() const { return (sample_labels_ & kCgroupLabel); }
+  // Returns whether code page size labels were requested for inclusion in the
+  // profile.proto's Sample.Label field.
+  bool IncludeCodePageSizeLabels() const {
+    return (sample_labels_ & kCodePageSizeLabel);
+  }
+  // Returns whether data page size labels were requested for inclusion in the
+  // profile.proto's Sample.Label field.
+  bool IncludeDataPageSizeLabels() const {
+    return (sample_labels_ & kDataPageSizeLabel);
+  }
 
   SampleKey MakeSampleKey(const PerfDataHandler::SampleContext& sample,
                           ProfileBuilder* builder);
@@ -339,6 +354,12 @@ SampleKey PerfDataConverter::MakeSampleKey(
   if (IncludeCgroupLabels() && sample.cgroup) {
     sample_key.cgroup = UTF8StringId(*sample.cgroup, builder);
   }
+  if (IncludeCodePageSizeLabels() && sample.sample.has_code_page_size()) {
+    sample_key.code_page_size = sample.sample.code_page_size();
+  }
+  if (IncludeDataPageSizeLabels() && sample.sample.has_data_page_size()) {
+    sample_key.data_page_size = sample.sample.data_page_size();
+  }
   return sample_key;
 }
 
@@ -518,6 +539,16 @@ void PerfDataConverter::AddOrUpdateSample(
       label->set_key(builder->StringId(CgroupLabelKey));
       label->set_str(sample_key.cgroup);
     }
+    if (IncludeCodePageSizeLabels() && sample_key.code_page_size != 0) {
+      auto* label = sample->add_label();
+      label->set_key(builder->StringId(CodePageSizeLabelKey));
+      label->set_num(sample_key.code_page_size);
+    }
+    if (IncludeDataPageSizeLabels() && sample_key.data_page_size != 0) {
+      auto* label = sample->add_label();
+      label->set_key(builder->StringId(DataPageSizeLabelKey));
+      label->set_num(sample_key.data_page_size);
+    }
     // Two values per collected event: the first is sample counts, the second is
     // event counts (unsampled weight for each sample).
     for (int event_id = 0; event_id < perf_data_.file_attrs_size();
@@ -690,7 +721,7 @@ void PerfDataConverter::Sample(const PerfDataHandler::SampleContext& sample) {
 
 ProcessProfiles PerfDataConverter::Profiles() {
   ProcessProfiles pps;
-  for (int i = 0; i < builders_.size(); i++) {
+  for (size_t i = 0; i < builders_.size(); i++) {
     auto& b = builders_[i];
     b.Finalize();
     auto pp = process_metas_[i].makeProcessProfile(b.mutable_profile());

src/perf_data_converter.h

@@ -25,32 +25,40 @@ namespace perftools {
 enum SampleLabels {
   kNoLabels = 0,
   // Adds label with key PidLabelKey and number value set to the process ID.
-  kPidLabel = 1,
+  kPidLabel = 1 << 0,
   // Adds label with key TidLabelKey and number value set to the thread ID.
-  kTidLabel = 2,
+  kTidLabel = 1 << 1,
   // Equivalent to kPidLabel | kTidLabel
   kPidAndTidLabels = 3,
   // Adds label with key TimestampNsLabelKey and number value set to the number
   // of nanoseconds since the system boot that this sample was taken.
-  kTimestampNsLabel = 4,
+  kTimestampNsLabel = 1 << 2,
   // Adds label with key ExecutionModeLabelKey and string value set to one of
   // the ExecutionMode* values.
-  kExecutionModeLabel = 8,
+  kExecutionModeLabel = 1 << 3,
   // Adds a label with key CommLabelKey and string value set to the sample's
   // process's command (that is, /proc/[pid]/comm). If no command is known, no
   // label is added.
-  kCommLabel = 16,
+  kCommLabel = 1 << 4,
   // Adds a label with key ThreadTypeLabelKey and string value set to the thread
   // type of the sample's thread ID. If the sample doesn't have a thread ID or
   // the sample's thread ID doesn't have a thread type, no label is added.
-  kThreadTypeLabel = 32,
+  kThreadTypeLabel = 1 << 5,
   // Adds a label with key ThreadCommLabelKey and string value set to the
   // sample's thread's command (that is, thread name, or
   // /proc/[pid]/task/[tid]/comm). If no command is known, no label is added.
-  kThreadCommLabel = 64,
+  kThreadCommLabel = 1 << 6,
   // Adds a label with CgroupLabelKey and number value set to the cgroup id.
   // If the sample doesn't have a cgroup ID, no label is added.
-  kCgroupLabel = 128,
+  kCgroupLabel = 1 << 7,
+  // Adds a label with CodePageSizeKey and number value set to the
+  // code_page_size. If the sample doesn't have a code page size, no label is
+  // added.
+  kCodePageSizeLabel = 1 << 8,
+  // Adds a label with DataPageSizeKey and number value set to the
+  // data_page_size. If the sample doesn't have a code page size, no label is
+  // added.
+  kDataPageSizeLabel = 1 << 9,
 };
 
 // Sample label key names.
@@ -62,6 +70,8 @@ const char CommLabelKey[] = "comm";
 const char ThreadTypeLabelKey[] = "thread_type";
 const char ThreadCommLabelKey[] = "thread_comm";
 const char CgroupLabelKey[] = "cgroup";
+const char CodePageSizeLabelKey[] = "code_page_size";
+const char DataPageSizeLabelKey[] = "data_page_size";
 
 // Execution mode label values.
 const char ExecutionModeHostKernel[] = "Host Kernel";

src/perf_data_converter_test.cc

@@ -33,6 +33,7 @@ using perftools::profiles::Mapping;
 using quipper::PerfDataProto;
 using testing::Contains;
 using testing::Eq;
+using testing::IsEmpty;
 using testing::UnorderedPointwise;
 
 namespace {
@@ -683,10 +684,102 @@ TEST_F(PerfDataConverterTest, ConvertsCgroup) {
     EXPECT_EQ(expected_count.second, counts_by_cgroup[expected_count.first])
         << "Different counts for cgroup: " << expected_count.first << std::endl
         << "Expected: " << expected_count.second << std::endl
-        << "Actual: " << expected_count.first << std::endl;
+        << "Actual: " << counts_by_cgroup[expected_count.first] << std::endl;
   }
 }
 
+std::pair<int, std::unordered_map<uint64, uint64>> ExtractCounts(
+    const ProcessProfiles& pps, std::string key_name) {
+  std::unordered_map<uint64, uint64> counts;
+  int total_samples = 0;
+  const int val_idx = 0;
+
+  for (const auto& pp : pps) {
+    const auto& p = pp->data;
+    EXPECT_EQ(p.string_table(p.sample_type(val_idx).type()), "cycles_sample");
+    for (const auto& sample : p.sample()) {
+      total_samples += sample.value(val_idx);
+      for (const auto& label : sample.label()) {
+        if (p.string_table(label.key()) == key_name) {
+          counts[label.num()] += sample.value(val_idx);
+        }
+      }
+    }
+  }
+  return {total_samples, counts};
+}
+
+TEST_F(PerfDataConverterTest, ConvertsCodePageSize) {
+  const string ascii_pb(
+      GetContents(GetResource("perf-code-data-page-sizes.textproto")));
+  ASSERT_FALSE(ascii_pb.empty());
+  PerfDataProto perf_data_proto;
+  ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(ascii_pb, &perf_data_proto));
+
+  const ProcessProfiles pps =
+      PerfDataProtoToProfiles(&perf_data_proto, kCodePageSizeLabel);
+  ASSERT_EQ(pps.size(), 1);
+
+  const auto counts_pair = ExtractCounts(pps, CodePageSizeLabelKey);
+  const auto total_samples = std::get<0>(counts_pair);
+  const auto& counts = std::get<1>(counts_pair);
+  const std::unordered_map<uint64, uint64> expected_counts{
+      {4096, 2},
+      {2097152, 2},
+      {1073741824, 1},
+  };
+  EXPECT_EQ(total_samples, 5);
+  EXPECT_THAT(counts, UnorderedPointwise(Eq(), expected_counts));
+}
+
+TEST_F(PerfDataConverterTest, ConvertsDataPageSize) {
+  const string ascii_pb(
+      GetContents(GetResource("perf-code-data-page-sizes.textproto")));
+  ASSERT_FALSE(ascii_pb.empty());
+  PerfDataProto perf_data_proto;
+  ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(ascii_pb, &perf_data_proto));
+
+  const ProcessProfiles pps =
+      PerfDataProtoToProfiles(&perf_data_proto, kDataPageSizeLabel);
+  ASSERT_EQ(pps.size(), 1);
+
+  const auto counts_pair = ExtractCounts(pps, DataPageSizeLabelKey);
+  const auto total_samples = std::get<0>(counts_pair);
+  const auto& counts = std::get<1>(counts_pair);
+  const std::unordered_map<uint64, uint64> expected_counts{
+      {4096, 2},
+      {2097152, 2},
+      {1073741824, 1},
+  };
+  EXPECT_EQ(total_samples, 5);
+  EXPECT_THAT(counts, UnorderedPointwise(Eq(), expected_counts));
+}
+
+// Test with a perf data doesn't have page size info.
+TEST_F(PerfDataConverterTest, ConvertsNoCodeDataPageSize) {
+  const string ascii_pb(
+      GetContents(GetResource("perf-cgroup-events.textproto")));
+  ASSERT_FALSE(ascii_pb.empty());
+  PerfDataProto perf_data_proto;
+  ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(ascii_pb, &perf_data_proto));
+
+  const ProcessProfiles pps = PerfDataProtoToProfiles(
+      &perf_data_proto, kCodePageSizeLabel | kDataPageSizeLabel);
+  ASSERT_EQ(pps.size(), 2);
+
+  auto counts_pair1 = ExtractCounts(pps, CodePageSizeLabelKey);
+  const auto total_samples1 = std::get<0>(counts_pair1);
+  const auto& code_counts = std::get<1>(counts_pair1);
+  EXPECT_EQ(total_samples1, 10);
+  EXPECT_THAT(code_counts, IsEmpty());
+
+  auto counts_pair2 = ExtractCounts(pps, DataPageSizeLabelKey);
+  const auto total_samples2 = std::get<0>(counts_pair2);
+  const auto& data_counts = std::get<1>(counts_pair2);
+  EXPECT_EQ(total_samples2, 10);
+  EXPECT_THAT(data_counts, IsEmpty());
+}
+
 }  // namespace perftools
 
 int main(int argc, char** argv) {

src/perf_data_handler_test.cc

@@ -96,7 +96,7 @@ class TestPerfDataHandler : public PerfDataHandler {
       _seen_addr_mappings.push_back(nullptr);
     }
     EXPECT_EQ(_expected_branch_stack.size(), sample.branch_stack.size());
-    for (int i = 0; i < sample.branch_stack.size(); i++) {
+    for (size_t i = 0; i < sample.branch_stack.size(); i++) {
       CheckBranchEquality(_expected_branch_stack[i], sample.branch_stack[i]);
     }
   }

src/quipper/huge_page_deducer.cc

@@ -26,7 +26,7 @@ bool IsAnon(const MMapEvent& event) {
                   event.filename() == kAnonHugepageFilename ||
                   event.filename() == kAnonHugepageDeletedFilename);
   if (is_anon && event.pgoff() != 0) {
-    LOG(WARNING) << "//anon should have offset=0 for mmap"
+    LOG(WARNING) << "//anon should have offset=0 for mmap "
                  << event.ShortDebugString();
   }
   return is_anon;

src/quipper/perf_reader.cc

@@ -1577,7 +1577,7 @@ bool PerfReader::ReadCPUTopologyMetadata(DataReader* data, size_t size) {
       return false;
     }
 
-    for (int i = 0; i < nrcpus; ++i) {
+    for (uint32_t i = 0; i < nrcpus; ++i) {
       PerfCPU cpu;
       if (!data->ReadUint32(&cpu.core_id)) {
         LOG(ERROR) << "Error reading Core ID.";

src/quipper/perf_serializer.cc

@@ -1038,7 +1038,7 @@ bool PerfSerializer::DeserializeAuxtraceInfoEvent(
     const PerfDataProto_AuxtraceInfoEvent& sample, event_t* event) const {
   struct auxtrace_info_event& auxtrace_info = event->auxtrace_info;
   auxtrace_info.type = sample.type();
-  for (u64 i = 0; i < sample.unparsed_binary_blob_priv_data_size(); ++i) {
+  for (int i = 0; i < sample.unparsed_binary_blob_priv_data_size(); ++i) {
     auxtrace_info.priv[i] = sample.unparsed_binary_blob_priv_data(i);
   }
   return true;
@@ -1510,7 +1510,7 @@ const SampleInfoReader* PerfSerializer::GetSampleInfoReaderForEvent(
       const u64* array =
           reinterpret_cast<const u64*>(&event) + (offset / sizeof(u64));
       // Find the length of the sample info array.
-      size_t array_size = (event.header.size - offset) / sizeof(u64);
+      ssize_t array_size = (event.header.size - offset) / sizeof(u64);
       if (event.header.type == PERF_RECORD_SAMPLE) {
         if (array_size <= event_id_pos) {
           LOG(ERROR) << "Sample info array of size " << array_size