From b14f86d5373eb7a4a8a40c9a23c3a9faf2732eeb Mon Sep 17 00:00:00 2001
From: Mark Rowe <mrowe@bdash.net.nz>
Date: Mon, 13 Jan 2025 22:19:17 -0800
Subject: [SharedCache] Avoid copying strings on each call to
 VM::MappingAtAddress

`PageMapping` was storing the path to the file it points within. This
was causing unnecessary work within `VM::MappingAtAddress` as the
`PageMapping`, and thus the path, is copied into the return value. This
copying was more expensive than the map lookup.

The file path is now stored within a `LazyMappedFileAccessor` class that
wraps the `SelfAllocatingWeakPtr`. This means the path is still
available via the `PageMapping`, but it does not need to be copied as
often.

This includes two additional improvements / optimizations while I was
touching the code in question:
1. `MMappedFileAccessor::Open` no longer performs two hash lookups when
   a file accessor already exists.
2. `VM::MapPages` takes the path by const reference to avoid an
   unnecessary copy.
---
 view/sharedcache/core/VM.h | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

(limited to 'view/sharedcache/core/VM.h')

diff --git a/view/sharedcache/core/VM.h b/view/sharedcache/core/VM.h
index e47cf15e..955dcbec 100644
--- a/view/sharedcache/core/VM.h
+++ b/view/sharedcache/core/VM.h
@@ -104,12 +104,25 @@ class MMAP {
     void Unmap();
 };
 
+class LazyMappedFileAccessor : public SelfAllocatingWeakPtr<MMappedFileAccessor> {
+public:
+    LazyMappedFileAccessor(std::string filePath, std::function<std::shared_ptr<MMappedFileAccessor>()> allocator,
+            std::function<void(std::shared_ptr<MMappedFileAccessor>)> postAlloc)
+        : SelfAllocatingWeakPtr(std::move(allocator), std::move(postAlloc)), m_filePath(std::move(filePath)) {
+    }
+
+    std::string_view filePath() const { return m_filePath; }
+
+private:
+    std::string m_filePath;
+};
+
 static uint64_t maxFPLimit;
 static std::mutex fileAccessorDequeMutex;
 static std::unordered_map<uint64_t, std::deque<std::shared_ptr<MMappedFileAccessor>>> fileAccessorReferenceHolder;
 static std::set<uint64_t> blockedSessionIDs;
 static std::mutex fileAccessorsMutex;
-static std::unordered_map<std::string, std::shared_ptr<SelfAllocatingWeakPtr<MMappedFileAccessor>>> fileAccessors;
+static std::unordered_map<std::string, std::shared_ptr<LazyMappedFileAccessor>> fileAccessors;
 static counting_semaphore fileAccessorSemaphore(0);
 
 static std::atomic<uint64_t> mmapCount = 0;
@@ -123,7 +136,7 @@ public:
 	MMappedFileAccessor(const std::string &path);
 	~MMappedFileAccessor();
 
-	static std::shared_ptr<SelfAllocatingWeakPtr<MMappedFileAccessor>> Open(BinaryNinja::Ref<BinaryNinja::BinaryView> dscView, const uint64_t sessionID, const std::string &path, std::function<void(std::shared_ptr<MMappedFileAccessor>)> postAllocationRoutine = nullptr);
+	static std::shared_ptr<LazyMappedFileAccessor> Open(BinaryNinja::Ref<BinaryNinja::BinaryView> dscView, const uint64_t sessionID, const std::string &path, std::function<void(std::shared_ptr<MMappedFileAccessor>)> postAllocationRoutine = nullptr);
 
 	static void CloseAll(const uint64_t sessionID);
 
@@ -179,11 +192,10 @@ public:
 
 
 struct PageMapping {
-    std::string filePath;
-	std::shared_ptr<SelfAllocatingWeakPtr<MMappedFileAccessor>> fileAccessor;
+    std::shared_ptr<LazyMappedFileAccessor> fileAccessor;
     size_t fileOffset;
-	PageMapping(std::string filePath, std::shared_ptr<SelfAllocatingWeakPtr<MMappedFileAccessor>> fileAccessor, size_t fileOffset)
-		: filePath(std::move(filePath)), fileAccessor(std::move(fileAccessor)), fileOffset(fileOffset) {}
+    PageMapping(std::shared_ptr<LazyMappedFileAccessor> fileAccessor, size_t fileOffset)
+        : fileAccessor(std::move(fileAccessor)), fileOffset(fileOffset) {}
 };
 
 
@@ -249,7 +261,7 @@ public:
 
     ~VM();
 
-    void MapPages(BinaryNinja::Ref<BinaryNinja::BinaryView> dscView, uint64_t sessionID, size_t vm_address, size_t fileoff, size_t size, std::string filePath, std::function<void(std::shared_ptr<MMappedFileAccessor>)> postAllocationRoutine);
+    void MapPages(BinaryNinja::Ref<BinaryNinja::BinaryView> dscView, uint64_t sessionID, size_t vm_address, size_t fileoff, size_t size, const std::string& filePath, std::function<void(std::shared_ptr<MMappedFileAccessor>)> postAllocationRoutine);
 
     bool AddressIsMapped(uint64_t address);
 
-- 
cgit v1.3.1