From 14b2306ec453d30306d26b0ac89efb8f4ecbca11 Mon Sep 17 00:00:00 2001
From: Matthieu Gautier <mgautier@kymeria.fr>
Date: Fri, 29 Mar 2024 11:49:58 +0100
Subject: [PATCH] Introduce `BaseFileReader::get_mmap_buffer`.

BaseFileReader is now responsible for trying to get a mmap and then
fallback to reading in file.
---
 src/file_reader.cpp | 60 ++++++++++++++++++++-------------------------
 src/file_reader.h   | 20 ++++++++++++---
 2 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/src/file_reader.cpp b/src/file_reader.cpp
index 3ec57591..63eea132 100644
--- a/src/file_reader.cpp
+++ b/src/file_reader.cpp
@@ -177,24 +177,11 @@ makeMmappedBuffer(int fd, offset_t offset, zsize_t size)
 } // unnamed namespace
 #endif // ENABLE_USE_MMAP
 
-const Buffer MultiPartFileReader::get_buffer(offset_t offset, zsize_t size) const {
+const Buffer BaseFileReader::get_buffer(offset_t offset, zsize_t size) const {
   ASSERT(size, <=, _size);
 #ifdef ENABLE_USE_MMAP
   try {
-    auto found_range = source->locate(_offset+offset, size);
-    auto first_part_containing_it = found_range.first;
-    if (++first_part_containing_it != found_range.second) {
-      throw MMapException();
-    }
-
-    // The range is in only one part
-    auto range = found_range.first->first;
-    auto part = found_range.first->second;
-    auto logical_local_offset = offset + _offset - range.min;
-    ASSERT(size, <=, part->size());
-    int fd = part->fhandle().getNativeHandle();
-    auto physical_local_offset = logical_local_offset + part->offset();
-    return Buffer::makeBuffer(makeMmappedBuffer(fd, physical_local_offset, size), size);
+    return get_mmap_buffer(offset, size);
   } catch(MMapException& e)
 #endif
   {
@@ -210,6 +197,25 @@ const Buffer MultiPartFileReader::get_buffer(offset_t offset, zsize_t size) cons
   }
 }
 
+#ifdef ENABLE_USE_MMAP
+const Buffer MultiPartFileReader::get_mmap_buffer(offset_t offset, zsize_t size) const {
+  auto found_range = source->locate(_offset + offset, size);
+  auto first_part_containing_it = found_range.first;
+  if (++first_part_containing_it != found_range.second) {
+    throw MMapException();
+  }
+
+  // The range is in only one part
+  auto range = found_range.first->first;
+  auto part = found_range.first->second;
+  auto logical_local_offset = offset + _offset - range.min;
+  ASSERT(size, <=, part->size());
+  int fd = part->fhandle().getNativeHandle();
+  auto physical_local_offset = logical_local_offset + part->offset();
+  return Buffer::makeBuffer(makeMmappedBuffer(fd, physical_local_offset, size), size);
+}
+#endif
+
 bool Reader::can_read(offset_t offset, zsize_t size) const
 {
     return (offset.v <= this->size().v && (offset.v+size.v) <= this->size().v);
@@ -273,27 +279,13 @@ void FileReader::read(char* dest, offset_t offset, zsize_t size) const
   };
 }
 
-const Buffer FileReader::get_buffer(offset_t offset, zsize_t size) const
-{
-  ASSERT(size, <=, _size);
 #ifdef ENABLE_USE_MMAP
-  try {
-    auto local_offset = offset + _offset;
-    int fd = _fhandle->getNativeHandle();
-    return Buffer::makeBuffer(makeMmappedBuffer(fd, local_offset, size), size);
-  } catch(MMapException& e)
-#endif
-  {
-    // We cannot do the mmap, for several possible reasons:
-    // - Mmap offset is too big (>4GB on 32 bits)
-    // - We are on Windows.
-    // We will have to do some memory copies :/
-    // [TODO] Use Windows equivalent for mmap.
-    auto ret_buffer = Buffer::makeBuffer(size);
-    read(const_cast<char*>(ret_buffer.data()), offset, size);
-    return ret_buffer;
-  }
+const Buffer FileReader::get_mmap_buffer(offset_t offset, zsize_t size) const {
+  auto local_offset = offset + _offset;
+  int fd = _fhandle->getNativeHandle();
+  return Buffer::makeBuffer(makeMmappedBuffer(fd, local_offset, size), size);
 }
+#endif
 
 std::unique_ptr<const Reader>
 FileReader::sub_reader(offset_t offset, zsize_t size) const
diff --git a/src/file_reader.h b/src/file_reader.h
index a6650c3e..285ceb69 100644
--- a/src/file_reader.h
+++ b/src/file_reader.h
@@ -36,6 +36,12 @@ class BaseFileReader : public Reader {
     zsize_t size() const { return _size; };
     offset_t offset() const { return _offset; };
 
+#ifdef ENABLE_USE_MMAP
+    virtual const Buffer get_mmap_buffer(offset_t offset,
+                                         zsize_t size) const = 0;
+#endif
+    const Buffer get_buffer(offset_t offset, zsize_t size) const;
+
   protected: // data
     offset_t _offset;
     zsize_t _size;
@@ -50,8 +56,11 @@ class FileReader : public BaseFileReader {
     ~FileReader() = default;
 
     char read(offset_t offset) const;
-    void read(char* dest, offset_t offset, zsize_t size) const;
-    const Buffer get_buffer(offset_t offset, zsize_t size) const;
+    void read(char *dest, offset_t offset, zsize_t size) const;
+
+#ifdef ENABLE_USE_MMAP
+    const Buffer get_mmap_buffer(offset_t offset, zsize_t size) const;
+#endif
 
     std::unique_ptr<const Reader> sub_reader(offset_t offset, zsize_t size) const;
 
@@ -68,8 +77,11 @@ class MultiPartFileReader : public BaseFileReader {
     ~MultiPartFileReader() {};
 
     char read(offset_t offset) const;
-    void read(char* dest, offset_t offset, zsize_t size) const;
-    const Buffer get_buffer(offset_t offset, zsize_t size) const;
+    void read(char *dest, offset_t offset, zsize_t size) const;
+
+#ifdef ENABLE_USE_MMAP
+    const Buffer get_mmap_buffer(offset_t offset, zsize_t size) const;
+#endif
 
     std::unique_ptr<const Reader> sub_reader(offset_t offset, zsize_t size) const;