From d4b1ff0b784a2fce61a0eaa89199a41b586c8c15 Mon Sep 17 00:00:00 2001 From: Wesley Elias Ribeiro Date: Thu, 6 Jan 2022 17:19:37 -0800 Subject: [PATCH] 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 --- cpp/mmapbuf/JBuffer.cpp | 16 +++++++------- cpp/mmapbuf/JBuffer.h | 4 ++-- cpp/mmapbuf/header/MmapBufferHeader.h | 6 +++--- cpp/mmapbuf/writer/MmapBufferTraceWriter.cpp | 11 ++++------ .../writer/MmapBufferTraceWriterTest.cpp | 8 +++---- .../facebook/profilo/mmapbuf/core/Buffer.java | 21 ++++++------------- 6 files changed, 27 insertions(+), 39 deletions(-) diff --git a/cpp/mmapbuf/JBuffer.cpp b/cpp/mmapbuf/JBuffer.cpp index 7194949c..fcba0ae3 100644 --- a/cpp/mmapbuf/JBuffer.cpp +++ b/cpp/mmapbuf/JBuffer.cpp @@ -59,7 +59,7 @@ 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; @@ -67,9 +67,9 @@ void JBuffer::updateMemoryMappingFilename(const std::string& maps_file_path) { // 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 JBuffer::getFilePath() { @@ -80,13 +80,13 @@ fbjni::local_ref JBuffer::getFilePath() { return fbjni::make_jstring(buffer->path); } -fbjni::local_ref JBuffer::getMemoryMappingFileName() { +fbjni::local_ref 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 index a8cd9cd2..26ad8d27 100644 --- a/cpp/mmapbuf/JBuffer.h +++ b/cpp/mmapbuf/JBuffer.h @@ -46,11 +46,11 @@ class JBuffer final : public fbjni::HybridClass { 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 getFilePath(); - fbjni::local_ref getMemoryMappingFileName(); + fbjni::local_ref getMemoryMappingFilePath(); static fbjni::local_ref makeJBuffer( std::weak_ptr ptr) { diff --git a/cpp/mmapbuf/header/MmapBufferHeader.h b/cpp/mmapbuf/header/MmapBufferHeader.h index 1d7f0ae0..dbf09766 100644 --- 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 index 58484ded..94be1955 100644 --- 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 index ef85d172..07371155 100644 --- 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 index 17c8cf8a..fb277939 100644 --- 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();