Store full mmap file path

Summary: This changes the MmapBufferHeader class to store the full path of the mapping file, instead of only the file name. Reviewed By: aandreyeu Differential Revision: D33246595 fbshipit-source-id: e72a363e332e27f753625edb3535458bc157043d
facebookarchive · Jan 7, 2022 · d4b1ff0 · d4b1ff0
1 parent dc799bc
commit d4b1ff0
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 39 deletions.
diff --git a/cpp/mmapbuf/JBuffer.cpp b/cpp/mmapbuf/JBuffer.cpp
@@ -59,17 +59,17 @@ void JBuffer::updateFilePath(const std::string& file_path) {
   buffer->rename(file_path);
 }
 
-void JBuffer::updateMemoryMappingFilename(const std::string& maps_file_path) {
+void JBuffer::updateMemoryMappingFilePath(const std::string& maps_file_path) {
   auto buffer = buffer_.lock();
   if (buffer == nullptr) {
     return;
   }
   // Compare and if session id has not changed exit
   auto sz = std::min(
       maps_file_path.size(),
-      (size_t)header::MmapBufferHeader::kMemoryMapsFilenameLength - 1);
-  maps_file_path.copy(buffer->prefix->header.memoryMapsFilename, sz);
-  buffer->prefix->header.memoryMapsFilename[sz] = 0;
+      (size_t)header::MmapBufferHeader::kMemoryMapsFilePathLength - 1);
+  maps_file_path.copy(buffer->prefix->header.memoryMapsFilePath, sz);
+  buffer->prefix->header.memoryMapsFilePath[sz] = 0;
 }
 
 fbjni::local_ref<fbjni::JString> JBuffer::getFilePath() {
@@ -80,13 +80,13 @@ fbjni::local_ref<fbjni::JString> JBuffer::getFilePath() {
   return fbjni::make_jstring(buffer->path);
 }
 
-fbjni::local_ref<fbjni::JString> JBuffer::getMemoryMappingFileName() {
+fbjni::local_ref<fbjni::JString> JBuffer::getMemoryMappingFilePath() {
   auto buffer = buffer_.lock();
   if (buffer == nullptr) {
     return nullptr;
   }
   auto memoryMappingFile =
-      std::string{buffer->prefix->header.memoryMapsFilename};
+      std::string{buffer->prefix->header.memoryMapsFilePath};
   if (memoryMappingFile.empty()) {
     return nullptr;
   }
@@ -99,9 +99,9 @@ void JBuffer::registerNatives() {
       makeNativeMethod("nativeUpdateId", JBuffer::updateId),
       makeNativeMethod("updateFilePath", JBuffer::updateFilePath),
       makeNativeMethod(
-          "updateMemoryMappingFilename", JBuffer::updateMemoryMappingFilename),
+          "updateMemoryMappingFilePath", JBuffer::updateMemoryMappingFilePath),
       makeNativeMethod(
-          "getMemoryMappingFilename", JBuffer::getMemoryMappingFileName),
+          "getMemoryMappingFilePath", JBuffer::getMemoryMappingFilePath),
       makeNativeMethod("getFilePath", JBuffer::getFilePath),
   });
 }

diff --git a/cpp/mmapbuf/JBuffer.h b/cpp/mmapbuf/JBuffer.h
@@ -46,11 +46,11 @@ class JBuffer final : public fbjni::HybridClass<JBuffer> {
 
   void updateFilePath(const std::string& file_path);
 
-  void updateMemoryMappingFilename(const std::string& maps_file_path);
+  void updateMemoryMappingFilePath(const std::string& maps_file_path);
 
   fbjni::local_ref<fbjni::JString> getFilePath();
 
-  fbjni::local_ref<fbjni::JString> getMemoryMappingFileName();
+  fbjni::local_ref<fbjni::JString> getMemoryMappingFilePath();
 
   static fbjni::local_ref<jhybridobject> makeJBuffer(
       std::weak_ptr<Buffer> ptr) {

diff --git a/cpp/mmapbuf/header/MmapBufferHeader.h b/cpp/mmapbuf/header/MmapBufferHeader.h
@@ -26,7 +26,7 @@ namespace mmapbuf {
 namespace header {
 
 constexpr static uint64_t kMagic = 0x306c3166307270; // pr0f1l0
-constexpr static uint64_t kVersion = 7;
+constexpr static uint64_t kVersion = 8;
 
 //
 // Static header for primary buffer verification.
@@ -47,7 +47,7 @@ struct __attribute__((packed)) MmapStaticHeader {
 //
 struct __attribute__((packed)) alignas(8) MmapBufferHeader {
   constexpr static auto kSessionIdLength = 40;
-  constexpr static auto kMemoryMapsFilenameLength = 64;
+  constexpr static auto kMemoryMapsFilePathLength = 512;
   uint16_t bufferVersion;
   int64_t configId;
   int32_t versionCode;
@@ -58,7 +58,7 @@ struct __attribute__((packed)) alignas(8) MmapBufferHeader {
   int64_t traceId;
   pid_t pid;
   char sessionId[kSessionIdLength];
-  char memoryMapsFilename[kMemoryMapsFilenameLength];
+  char memoryMapsFilePath[kMemoryMapsFilePathLength];
 };
 
 //

diff --git a/cpp/mmapbuf/writer/MmapBufferTraceWriter.cpp b/cpp/mmapbuf/writer/MmapBufferTraceWriter.cpp
@@ -148,7 +148,7 @@ bool copyBufferEntries(TraceBuffer& source, TraceBuffer& dest) {
 
 void processMemoryMappingsFile(
     Logger& logger,
-    std::string& file_path,
+    const char* file_path,
     int64_t timestamp) {
   std::ifstream mappingsFile(file_path);
   if (!mappingsFile.is_open()) {
@@ -302,12 +302,9 @@ void MmapBufferTraceWriter::writeTrace(
         logger, qpl_marker_id, "collection_method", "persistent", timestamp);
   }
 
-  const char* mapsFilename = mapBufferPrefix->header.memoryMapsFilename;
-  if (mapsFilename[0] != '\0') {
-    auto lastSlashIdx = dump_path_.rfind("/");
-    std::string mapsPath =
-        dump_path_.substr(0, lastSlashIdx + 1) + mapsFilename;
-    processMemoryMappingsFile(logger, mapsPath, timestamp);
+  const char* mapsFilePath = mapBufferPrefix->header.memoryMapsFilePath;
+  if (mapsFilePath[0] != '\0') {
+    processMemoryMappingsFile(logger, mapsFilePath, timestamp);
   }
 
   loggerWrite(logger, EntryType::TRACE_END, 0, 0, trace_id_, timestamp);

diff --git a/cpp/test/mmapbuf/writer/MmapBufferTraceWriterTest.cpp b/cpp/test/mmapbuf/writer/MmapBufferTraceWriterTest.cpp
@@ -153,11 +153,11 @@ class MmapBufferTraceWriterTest : public ::testing::Test {
     buffer->prefix->header.longContext = kQplId;
     buffer->prefix->header.traceId = kTraceId;
     if (set_mappings_file) {
-      auto path = temp_mappings_file_.path().filename().generic_string();
+      auto path = temp_mappings_file_.path().generic_string();
       auto sz = std::min(
-          path.size(), sizeof(buffer->prefix->header.memoryMapsFilename) - 1);
-      ::memcpy(buffer->prefix->header.memoryMapsFilename, path.c_str(), sz);
-      buffer->prefix->header.memoryMapsFilename[sz] = 0;
+          path.size(), sizeof(buffer->prefix->header.memoryMapsFilePath) - 1);
+      ::memcpy(buffer->prefix->header.memoryMapsFilePath, path.c_str(), sz);
+      buffer->prefix->header.memoryMapsFilePath[sz] = 0;
     }
     writeRandomEntries(buffer->ringBuffer(), records_count);
     int msync_res = msync(buffer->prefix, buffer->totalByteSize, MS_SYNC);

diff --git a/java/main/com/facebook/profilo/mmapbuf/core/Buffer.java b/java/main/com/facebook/profilo/mmapbuf/core/Buffer.java
@@ -50,28 +50,19 @@ public synchronized String generateMemoryMappingFilePath() {
     if (!isFileBacked()) {
       return null;
     }
-    String filePath = getCurrentMemoryMappingFilePath();
+    String filePath = getMemoryMappingFilePath();
     if (filePath == null) {
       MmapBufferFileHelper helper = new MmapBufferFileHelper(getBufferContainingFolder());
       String fileName = MmapBufferFileHelper.getMemoryMappingFilename(UUID.randomUUID().toString());
-      updateMemoryMappingFilename(fileName);
       filePath = helper.ensureFilePath(fileName);
+      if (filePath != null) {
+        updateMemoryMappingFilePath(filePath);
+      }
     }
 
     return filePath;
   }
 
-  @Nullable
-  public synchronized String getCurrentMemoryMappingFilePath() {
-    MmapBufferFileHelper helper = new MmapBufferFileHelper(getBufferContainingFolder());
-    String memoryMappingFilename = getMemoryMappingFilename();
-    String filePath = null;
-    if (memoryMappingFilename != null) {
-      filePath = helper.ensureFilePath(memoryMappingFilename);
-    }
-    return filePath;
-  }
-
   public synchronized void updateId(String id) {
     if (!isFileBacked()) {
       return;
@@ -102,10 +93,10 @@ public synchronized native void updateHeader(
   public synchronized native void updateFilePath(String filePath);
 
   @DoNotStrip
-  public synchronized native void updateMemoryMappingFilename(String mappingFilePath);
+  public synchronized native void updateMemoryMappingFilePath(String mappingFilePath);
 
   @DoNotStrip
-  public synchronized native String getMemoryMappingFilename();
+  public synchronized native String getMemoryMappingFilePath();
 
   @DoNotStrip
   public synchronized native String getFilePath();