Merge "simpleperf: move to file2 feature section."

9 files changed, 210 insertions, 46 deletions

diff --git a/simpleperf/cmd_debug_unwind.cpp b/simpleperf/cmd_debug_unwind.cpp
index 3306e486..560a1834 100644
--- a/simpleperf/cmd_debug_unwind.cpp
+++ b/simpleperf/cmd_debug_unwind.cpp
@@ -443,7 +443,8 @@ class TestFileGenerator : public RecordFileProcessor {
         if (!writer_->WriteDebugUnwindFeature(feature)) {
           return false;
         }
-      } else if (feat_type == PerfFileFormat::FEAT_FILE) {
+      } else if (feat_type == PerfFileFormat::FEAT_FILE ||
+                 feat_type == PerfFileFormat::FEAT_FILE2) {
         size_t read_pos = 0;
         FileFeature file_feature;
         while (reader_->ReadFileFeature(read_pos, &file_feature)) {
diff --git a/simpleperf/cmd_dumprecord.cpp b/simpleperf/cmd_dumprecord.cpp
index f67c411c..9a5586bb 100644
--- a/simpleperf/cmd_dumprecord.cpp
+++ b/simpleperf/cmd_dumprecord.cpp
@@ -471,7 +471,7 @@ bool DumpRecordCommand::DumpFeatureSection() {
     } else if (feature == FEAT_CMDLINE) {
       std::vector<std::string> cmdline = record_file_reader_->ReadCmdlineFeature();
       PrintIndented(1, "cmdline: %s\n", android::base::Join(cmdline, ' ').c_str());
-    } else if (feature == FEAT_FILE) {
+    } else if (feature == FEAT_FILE || feature == FEAT_FILE2) {
       FileFeature file;
       size_t read_pos = 0;
       PrintIndented(1, "file:\n");
diff --git a/simpleperf/cmd_merge.cpp b/simpleperf/cmd_merge.cpp
index d1ac6dca..e80dd953 100644
--- a/simpleperf/cmd_merge.cpp
+++ b/simpleperf/cmd_merge.cpp
@@ -338,7 +338,7 @@ class MergeCommand : public Command {
         }
       } else if (feature == PerfFileFormat::FEAT_BUILD_ID) {
         WriteBuildIdFeature();
-      } else if (feature == PerfFileFormat::FEAT_FILE) {
+      } else if (feature == PerfFileFormat::FEAT_FILE || feature == PerfFileFormat::FEAT_FILE2) {
         WriteFileFeature();
       } else {
         LOG(WARNING) << "Drop feature " << feature << ", which isn't supported in the merge cmd.";
diff --git a/simpleperf/record_file.h b/simpleperf/record_file.h
index 76f70436..8733c277 100644
--- a/simpleperf/record_file.h
+++ b/simpleperf/record_file.h
@@ -43,7 +43,7 @@ struct FileFeature {
   std::string path;
   DsoType type;
   uint64_t min_vaddr;
-  uint64_t file_offset_of_min_vaddr;
+  uint64_t file_offset_of_min_vaddr;       // for DSO_ELF_FILE or DSO_KERNEL_MODULE
   std::vector<Symbol> symbols;             // used for reading symbols
   std::vector<const Symbol*> symbol_ptrs;  // used for writing symbols
   std::vector<uint64_t> dex_file_offsets;
@@ -196,6 +196,8 @@ class RecordFileReader {
   bool ReadAttrSection();
   bool ReadIdsForAttr(const PerfFileFormat::FileAttr& attr, std::vector<uint64_t>* ids);
   bool ReadFeatureSectionDescriptors();
+  bool ReadFileV1Feature(size_t& read_pos, FileFeature* file);
+  bool ReadFileV2Feature(size_t& read_pos, FileFeature* file);
   bool ReadMetaInfoFeature();
   void UseRecordingEnvironment();
   std::unique_ptr<Record> ReadRecord();
diff --git a/simpleperf/record_file.proto b/simpleperf/record_file.proto
index 752c1f6b..5b100a74 100644
--- a/simpleperf/record_file.proto
+++ b/simpleperf/record_file.proto
@@ -29,3 +29,32 @@ message DebugUnwindFeature {
 
   repeated File file = 1;
 }
+
+message FileFeature {
+  string path = 1;
+  uint32 type = 2;
+  uint64 min_vaddr = 3;
+
+  message Symbol {
+    uint64 vaddr = 1;
+    uint32 len = 2;
+    string name = 3;
+  }
+  repeated Symbol symbol = 4;
+
+  message DexFile {
+    repeated uint64 dex_file_offset = 1;
+  }
+  message ElfFile {
+    uint64 file_offset_of_min_vaddr = 1;
+  }
+  message KernelModule {
+    uint64 memory_offset_of_min_vaddr = 1;
+  }
+
+  oneof type_specific_msg {
+    DexFile dex_file = 5;  // Only when type = DSO_DEX_FILE
+    ElfFile elf_file = 6;  // Only when type = DSO_ELF_FILE
+    KernelModule kernel_module = 7;  // Only when type = DSO_KERNEL_MODULE
+  }
+}
diff --git a/simpleperf/record_file_format.h b/simpleperf/record_file_format.h
index 07a489c8..53c6719b 100644
--- a/simpleperf/record_file_format.h
+++ b/simpleperf/record_file_format.h
@@ -67,7 +67,7 @@ meta_info feature section:
     simpleperf_version,
 
 debug_unwind feature section:
-  message DebugUnwindSection from record_file.proto
+  message DebugUnwindFeature from record_file.proto
 
 debug_unwind_file feature section:
   data for file 1
@@ -75,6 +75,13 @@ debug_unwind_file feature section:
   ...
 
   The file list is stored in debug_unwind feature section.
+
+file2 feature section (used to replace file feature section):
+  uint32_t file_msg1_size;
+  FileFeature file_msg1;  // FileFeature from record_file.proto
+  uint32_t file_msg2_size;
+  FileFeature file_msg2;
+  ...
 */
 
 namespace simpleperf {
@@ -108,6 +115,7 @@ enum {
   FEAT_META_INFO,
   FEAT_DEBUG_UNWIND,
   FEAT_DEBUG_UNWIND_FILE,
+  FEAT_FILE2,
   FEAT_MAX_NUM = 256,
 };
 
diff --git a/simpleperf/record_file_reader.cpp b/simpleperf/record_file_reader.cpp
index bba96830..22277d40 100644
--- a/simpleperf/record_file_reader.cpp
+++ b/simpleperf/record_file_reader.cpp
@@ -57,6 +57,7 @@ static const std::map<int, std::string> feature_name_map = {
     {FEAT_META_INFO, "meta_info"},
     {FEAT_DEBUG_UNWIND, "debug_unwind"},
     {FEAT_DEBUG_UNWIND_FILE, "debug_unwind_file"},
+    {FEAT_FILE2, "file2"},
 };
 
 std::string GetFeatureName(int feature_id) {
@@ -487,6 +488,16 @@ std::vector<uint64_t> RecordFileReader::ReadAuxTraceFeature() {
 }
 
 bool RecordFileReader::ReadFileFeature(size_t& read_pos, FileFeature* file) {
+  if (HasFeature(FEAT_FILE)) {
+    return ReadFileV1Feature(read_pos, file);
+  }
+  if (HasFeature(FEAT_FILE2)) {
+    return ReadFileV2Feature(read_pos, file);
+  }
+  return false;
+}
+
+bool RecordFileReader::ReadFileV1Feature(size_t& read_pos, FileFeature* file) {
   auto it = feature_section_descriptors_.find(FEAT_FILE);
   if (it == feature_section_descriptors_.end()) {
     return false;
@@ -551,6 +562,57 @@ bool RecordFileReader::ReadFileFeature(size_t& read_pos, FileFeature* file) {
   return true;
 }
 
+bool RecordFileReader::ReadFileV2Feature(size_t& read_pos, FileFeature* file) {
+  auto it = feature_section_descriptors_.find(FEAT_FILE2);
+  if (it == feature_section_descriptors_.end()) {
+    return false;
+  }
+  if (read_pos >= it->second.size) {
+    return false;
+  }
+  if (read_pos == 0) {
+    if (fseek(record_fp_, it->second.offset, SEEK_SET) != 0) {
+      PLOG(ERROR) << "fseek() failed";
+      return false;
+    }
+  }
+  uint32_t size;
+  if (!Read(&size, 4)) {
+    return false;
+  }
+  read_pos += 4 + size;
+  std::string s(size, '\0');
+  if (!Read(s.data(), size)) {
+    return false;
+  }
+  proto::FileFeature proto_file;
+  if (!proto_file.ParseFromString(s)) {
+    return false;
+  }
+  file->path = proto_file.path();
+  file->type = static_cast<DsoType>(proto_file.type());
+  file->min_vaddr = proto_file.min_vaddr();
+  file->symbols.clear();
+  file->symbols.reserve(proto_file.symbol_size());
+  for (size_t i = 0; i < proto_file.symbol_size(); i++) {
+    const auto& proto_symbol = proto_file.symbol(i);
+    file->symbols.emplace_back(proto_symbol.name(), proto_symbol.vaddr(), proto_symbol.len());
+  }
+  if (file->type == DSO_DEX_FILE) {
+    CHECK(proto_file.has_dex_file());
+    const auto& dex_file_offsets = proto_file.dex_file().dex_file_offset();
+    file->dex_file_offsets.insert(file->dex_file_offsets.end(), dex_file_offsets.begin(),
+                                  dex_file_offsets.end());
+  } else if (file->type == DSO_ELF_FILE) {
+    CHECK(proto_file.has_elf_file());
+    file->file_offset_of_min_vaddr = proto_file.elf_file().file_offset_of_min_vaddr();
+  } else if (file->type == DSO_KERNEL_MODULE) {
+    CHECK(proto_file.has_kernel_module());
+    file->file_offset_of_min_vaddr = proto_file.kernel_module().memory_offset_of_min_vaddr();
+  }
+  return true;
+}
+
 bool RecordFileReader::ReadMetaInfoFeature() {
   if (feature_section_descriptors_.count(FEAT_META_INFO)) {
     std::vector<char> buf;
@@ -596,7 +658,7 @@ void RecordFileReader::LoadBuildIdAndFileFeatures(ThreadTree& thread_tree) {
   }
   Dso::SetBuildIds(build_ids);
 
-  if (HasFeature(PerfFileFormat::FEAT_FILE)) {
+  if (HasFeature(PerfFileFormat::FEAT_FILE) || HasFeature(PerfFileFormat::FEAT_FILE2)) {
     FileFeature file_feature;
     size_t read_pos = 0;
     while (ReadFileFeature(read_pos, &file_feature)) {
diff --git a/simpleperf/record_file_test.cpp b/simpleperf/record_file_test.cpp
index 20bacaa9..31c942c2 100644
--- a/simpleperf/record_file_test.cpp
+++ b/simpleperf/record_file_test.cpp
@@ -19,6 +19,7 @@
 #include <string.h>
 
 #include <memory>
+#include <vector>
 
 #include <android-base/file.h>
 
@@ -180,3 +181,75 @@ TEST_F(RecordFileTest, write_debug_unwind_feature_section) {
     ASSERT_EQ(opt_debug_unwind.value()[i].size, debug_unwind[i].size);
   }
 }
+
+TEST_F(RecordFileTest, write_file2_feature_section) {
+  // Write to a record file.
+  std::unique_ptr<RecordFileWriter> writer = RecordFileWriter::CreateInstance(tmpfile_.path);
+  ASSERT_TRUE(writer != nullptr);
+  AddEventType("cpu-cycles");
+  ASSERT_TRUE(writer->WriteAttrSection(attr_ids_));
+
+  // Write file2 feature section.
+  ASSERT_TRUE(writer->BeginWriteFeatures(1));
+  std::vector<FileFeature> files(3);
+  files[0].path = "fake_dex_file";
+  files[0].type = DSO_DEX_FILE;
+  files[0].min_vaddr = 0x1000;
+  files[0].symbols.emplace_back("dex_symbol", 0x1001, 0x1002);
+  files[0].dex_file_offsets.assign(0x1003, 0x1004);
+  files[1].path = "fake_elf_file";
+  files[1].type = DSO_ELF_FILE;
+  files[1].min_vaddr = 0x2000;
+  Symbol symbol("elf_symbol", 0x2001, 0x2002);
+  files[1].symbol_ptrs.emplace_back(&symbol);
+  files[1].file_offset_of_min_vaddr = 0x2003;
+  files[2].path = "fake_kernel_module";
+  files[2].type = DSO_KERNEL_MODULE;
+  files[2].min_vaddr = 0x3000;
+  files[2].symbols.emplace_back("kernel_module_symbol", 0x3001, 0x3002);
+  files[2].file_offset_of_min_vaddr = 0x3003;
+
+  for (const auto& file : files) {
+    ASSERT_TRUE(writer->WriteFileFeature(file));
+  }
+  ASSERT_TRUE(writer->EndWriteFeatures());
+  ASSERT_TRUE(writer->Close());
+
+  // Read from a record file.
+  std::unique_ptr<RecordFileReader> reader = RecordFileReader::CreateInstance(tmpfile_.path);
+  ASSERT_TRUE(reader != nullptr);
+  size_t read_pos = 0;
+  FileFeature file;
+
+  auto check_symbol = [](const Symbol& sym1, const Symbol& sym2) {
+    return sym1.addr == sym2.addr && sym1.len == sym2.len && strcmp(sym1.Name(), sym2.Name()) == 0;
+  };
+
+  size_t file_id = 0;
+  while (reader->ReadFileFeature(read_pos, &file)) {
+    ASSERT_LT(file_id, files.size());
+    const FileFeature& expected_file = files[file_id++];
+    ASSERT_EQ(file.path, expected_file.path);
+    ASSERT_EQ(file.type, expected_file.type);
+    ASSERT_EQ(file.min_vaddr, expected_file.min_vaddr);
+    if (!expected_file.symbols.empty()) {
+      ASSERT_EQ(file.symbols.size(), expected_file.symbols.size());
+      for (size_t i = 0; i < file.symbols.size(); i++) {
+        ASSERT_TRUE(check_symbol(file.symbols[i], expected_file.symbols[i]));
+      }
+    } else {
+      ASSERT_EQ(file.symbols.size(), expected_file.symbol_ptrs.size());
+      for (size_t i = 0; i < file.symbols.size(); i++) {
+        ASSERT_TRUE(check_symbol(file.symbols[i], *expected_file.symbol_ptrs[i]));
+      }
+    }
+    if (file.type == DSO_DEX_FILE) {
+      ASSERT_EQ(file.dex_file_offsets, expected_file.dex_file_offsets);
+    } else if (file.type == DSO_ELF_FILE) {
+      ASSERT_EQ(file.file_offset_of_min_vaddr, expected_file.file_offset_of_min_vaddr);
+    } else if (file.type == DSO_KERNEL_MODULE) {
+      ASSERT_EQ(file.file_offset_of_min_vaddr, expected_file.file_offset_of_min_vaddr);
+    }
+  }
+  ASSERT_EQ(file_id, files.size());
+}
\ No newline at end of file
diff --git a/simpleperf/record_file_writer.cpp b/simpleperf/record_file_writer.cpp
index 9fe2f70c..ce0e69bb 100644
--- a/simpleperf/record_file_writer.cpp
+++ b/simpleperf/record_file_writer.cpp
@@ -355,52 +355,41 @@ bool RecordFileWriter::WriteFileFeatures(const std::vector<Dso*>& dsos) {
 }
 
 bool RecordFileWriter::WriteFileFeature(const FileFeature& file) {
-  uint32_t symbol_count = file.symbols.size() + file.symbol_ptrs.size();
-  uint32_t size = file.path.size() + 1 + sizeof(uint32_t) * 2 + sizeof(uint64_t) +
-                  symbol_count * (sizeof(uint64_t) + sizeof(uint32_t));
-  for (const auto& symbol : file.symbols) {
-    size += strlen(symbol.Name()) + 1;
-  }
-  for (const auto& symbol : file.symbol_ptrs) {
-    size += strlen(symbol->Name()) + 1;
-  }
-  if (file.type == DSO_DEX_FILE) {
-    size += sizeof(uint32_t) + sizeof(uint64_t) * file.dex_file_offsets.size();
-  }
-  if (file.type == DSO_ELF_FILE || file.type == DSO_KERNEL_MODULE) {
-    size += sizeof(uint64_t);
-  }
-  std::vector<char> buf(sizeof(uint32_t) + size);
-  char* p = buf.data();
-  MoveToBinaryFormat(size, p);
-  MoveToBinaryFormat(file.path.c_str(), file.path.size() + 1, p);
-  MoveToBinaryFormat(static_cast<uint32_t>(file.type), p);
-  MoveToBinaryFormat(file.min_vaddr, p);
-  MoveToBinaryFormat(symbol_count, p);
-
-  auto write_symbol = [&](const Symbol* symbol) {
-    MoveToBinaryFormat(symbol->addr, p);
-    uint32_t len = symbol->len;
-    MoveToBinaryFormat(len, p);
-    MoveToBinaryFormat(symbol->Name(), strlen(symbol->Name()) + 1, p);
+  proto::FileFeature proto_file;
+  proto_file.set_path(file.path);
+  proto_file.set_type(static_cast<uint32_t>(file.type));
+  proto_file.set_min_vaddr(file.min_vaddr);
+  auto write_symbol = [&](const Symbol& symbol) {
+    proto::FileFeature::Symbol* proto_symbol = proto_file.add_symbol();
+    proto_symbol->set_vaddr(symbol.addr);
+    proto_symbol->set_len(symbol.len);
+    proto_symbol->set_name(symbol.Name());
   };
-  for (const auto& symbol : file.symbols) {
-    write_symbol(&symbol);
-  }
-  for (const auto& symbol : file.symbol_ptrs) {
+  for (const Symbol& symbol : file.symbols) {
     write_symbol(symbol);
   }
+  for (const Symbol* symbol_ptr : file.symbol_ptrs) {
+    write_symbol(*symbol_ptr);
+  }
   if (file.type == DSO_DEX_FILE) {
-    uint32_t offset_count = file.dex_file_offsets.size();
-    MoveToBinaryFormat(offset_count, p);
-    MoveToBinaryFormat(file.dex_file_offsets.data(), offset_count, p);
+    proto::FileFeature::DexFile* proto_dex_file = proto_file.mutable_dex_file();
+    proto_dex_file->mutable_dex_file_offset()->Add(file.dex_file_offsets.begin(),
+                                                   file.dex_file_offsets.end());
+  } else if (file.type == DSO_ELF_FILE) {
+    proto::FileFeature::ElfFile* proto_elf_file = proto_file.mutable_elf_file();
+    proto_elf_file->set_file_offset_of_min_vaddr(file.file_offset_of_min_vaddr);
+  } else if (file.type == DSO_KERNEL_MODULE) {
+    proto::FileFeature::KernelModule* proto_kernel_module = proto_file.mutable_kernel_module();
+    proto_kernel_module->set_memory_offset_of_min_vaddr(file.file_offset_of_min_vaddr);
   }
-  if (file.type == DSO_ELF_FILE || file.type == DSO_KERNEL_MODULE) {
-    MoveToBinaryFormat(file.file_offset_of_min_vaddr, p);
+  std::string s;
+  if (!proto_file.SerializeToString(&s)) {
+    LOG(ERROR) << "SerializeToString() failed";
+    return false;
   }
-  CHECK_EQ(buf.size(), static_cast<size_t>(p - buf.data()));
-
-  return WriteFeature(FEAT_FILE, buf.data(), buf.size());
+  uint32_t msg_size = s.size();
+  return WriteFeatureBegin(FEAT_FILE2) && Write(&msg_size, sizeof(uint32_t)) &&
+         Write(s.data(), s.size()) && WriteFeatureEnd(FEAT_FILE2);
 }
 
 bool RecordFileWriter::WriteMetaInfoFeature(