[mc] Cleanup RegionSnapshot buffer code

diff --git a/src/mc/RegionSnapshot.cpp b/src/mc/RegionSnapshot.cpp
index 7e96951..a045b2f 100644 (file)
--- a/src/mc/RegionSnapshot.cpp
+++ b/src/mc/RegionSnapshot.cpp
@@ -39,36 +39,63 @@ const char* to_cstr(RegionType region)
   }
 }
 
   }
 }
 

-void data_deleter::operator()(void* p) const
+buffer::buffer(std::size_t size, Type type) : size_(size), type_(type)

 {
   switch(type_) {
 {
   switch(type_) {

-  case Free:
-    std::free(p);
+  case Type::Malloc:
+    data_ = malloc(size_);

     break;
     break;

-  case Munmap:
-    munmap(p, size_);
+  case Type::Mmap:
+    data_ = mmap(nullptr, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_POPULATE, -1, 0);
+    if (data_ == MAP_FAILED) {
+      data_ = nullptr;
+      size_ = 0;
+      type_ = Type::Malloc;
+      throw std::bad_alloc();
+    }
+    break;
+  default:
+    abort();
+  }
+}
+
+void buffer::clear() noexcept
+{
+  switch(type_) {
+  case Type::Malloc:
+    std::free(data_);

     break;
     break;

+  case Type::Mmap:
+    if (munmap(data_, size_) != 0)
+      abort();
+    break;
+  default:
+    abort();

   }
   }

+  data_ = nullptr;
+  size_ = 0;
+  type_ = Type::Malloc;

 }
 
 RegionSnapshot dense_region(
   RegionType region_type,
   void *start_addr, void* permanent_addr, size_t size)
 {
 }
 
 RegionSnapshot dense_region(
   RegionType region_type,
   void *start_addr, void* permanent_addr, size_t size)
 {

-  simgrid::mc::RegionSnapshot::flat_data_ptr data;
-  if (!_sg_mc_ksm)
-    data = simgrid::mc::RegionSnapshot::flat_data_ptr((char*) std::malloc(size));
-  else {
-    char* ptr = (char*) mmap(nullptr, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_POPULATE, -1, 0);
-    if (ptr == MAP_FAILED)
-      throw std::bad_alloc();
-    simgrid::mc::data_deleter deleter(
-      simgrid::mc::data_deleter::Munmap, size);
-    data = simgrid::mc::RegionSnapshot::flat_data_ptr(ptr, deleter);
-  }
+  simgrid::mc::buffer::Type buffer_type;
+  if (_sg_mc_ksm)
+    // We use mmap to allocate the memory in order to madvise it.
+    // We don't want to madvise the main heap.
+    // Moreover we get aligned pgaes which is merge-friendly.
+    buffer_type = simgrid::mc::buffer::Type::Mmap;
+  else
+    buffer_type = simgrid::mc::buffer::Type::Malloc;
+
+  simgrid::mc::buffer data(size, buffer_type);
+

   mc_model_checker->process().read_bytes(data.get(), size,
     remote(permanent_addr),
     simgrid::mc::ProcessIndexDisabled);
   mc_model_checker->process().read_bytes(data.get(), size,
     remote(permanent_addr),
     simgrid::mc::ProcessIndexDisabled);

+

   if (_sg_mc_ksm)
     // Mark the region as mergeable *after* we have written into it.
     // There no point to let KSM do the hard work before that.
   if (_sg_mc_ksm)
     // Mark the region as mergeable *after* we have written into it.
     // There no point to let KSM do the hard work before that.


@@ -79,7 +106,7 @@ RegionSnapshot dense_region(
   region.flat_data(std::move(data));
 
   XBT_DEBUG("New region : type : %s, data : %p (real addr %p), size : %zu",
   region.flat_data(std::move(data));
 
   XBT_DEBUG("New region : type : %s, data : %p (real addr %p), size : %zu",

-            to_cstr(region_type), region.flat_data(), permanent_addr, size);
+            to_cstr(region_type), region.flat_data().get(), permanent_addr, size);

   return std::move(region);
 }
 
   return std::move(region);
 }
 

diff --git a/src/mc/RegionSnapshot.hpp b/src/mc/RegionSnapshot.hpp
index a003191..f1007ac 100644 (file)
--- a/src/mc/RegionSnapshot.hpp
+++ b/src/mc/RegionSnapshot.hpp
@@ -37,22 +37,53 @@ enum class StorageType {
   Privatized = 3
 };
 
   Privatized = 3
 };
 

-class data_deleter {
+class buffer {

 public:
 public:

-  enum Type {
-    Free,
-    Munmap
+  enum class Type {
+    Malloc,
+    Mmap

   };
 private:
   };
 private:

-  Type type_;
+  void* data_ = nullptr;

   std::size_t size_;
   std::size_t size_;

+  Type type_ = Type::Malloc;

 public:
 public:

-  data_deleter() : type_(Free) {}
-  data_deleter(Type type, std::size_t size) : type_(type), size_(size) {}
-  void operator()(void* p) const;
-};
+  buffer() {}
+  buffer(std::size_t size, Type type = Type::Malloc);
+  buffer(void* data, std::size_t size, Type type = Type::Malloc) :
+    data_(data), size_(size), type_(type) {}
+  void clear() noexcept;
+  ~buffer() noexcept { clear(); }
+
+  // No copy
+  buffer(buffer const& buffer) = delete;
+  buffer& operator=(buffer const& buffer) = delete;
+
+  // Move
+  buffer(buffer&& that) noexcept
+    : data_(that.data_), size_(that.size_), type_(that.type_)
+  {
+    that.data_ = nullptr;
+    that.size_ = 0;
+    that.type_ = Type::Malloc;
+  }
+  buffer& operator=(buffer&& that) noexcept
+  {
+    clear();
+    data_ = that.data_;
+    size_ = that.size_;
+    type_ = that.type_;
+    that.data_ = nullptr;
+    that.size_ = 0;
+    that.type_ = Type::Malloc;
+    return *this;
+  }

 
 

-typedef std::unique_ptr<char[], data_deleter> unique_data_ptr;
+  void* get()              { return data_; }
+  const void* get()  const { return data_; }
+  std::size_t size() const { return size_; }
+  Type type()        const { return type_; }
+};

 
 /** A copy/snapshot of a given memory region
  *
 
 /** A copy/snapshot of a given memory region
  *


@@ -77,8 +108,6 @@ public:
   static const RegionType UnknownRegion = RegionType::Unknown;
   static const RegionType HeapRegion = RegionType::Heap;
   static const RegionType DataRegion = RegionType::Data;
   static const RegionType UnknownRegion = RegionType::Unknown;
   static const RegionType HeapRegion = RegionType::Heap;
   static const RegionType DataRegion = RegionType::Data;

-public:
-  typedef unique_data_ptr flat_data_ptr;

 private:
   RegionType region_type_;
   StorageType storage_type_;
 private:
   RegionType region_type_;
   StorageType storage_type_;


@@ -101,7 +130,7 @@ private:
    * */
   void *permanent_addr_;
 
    * */
   void *permanent_addr_;
 

-  flat_data_ptr flat_data_;
+  buffer flat_data_;

   ChunkedData page_numbers_;
   std::vector<RegionSnapshot> privatized_regions_;
 public:
   ChunkedData page_numbers_;
   std::vector<RegionSnapshot> privatized_regions_;
 public:


@@ -160,7 +189,7 @@ public:
     storage_type_ = StorageType::NoData;
     privatized_regions_.clear();
     page_numbers_.clear();
     storage_type_ = StorageType::NoData;
     privatized_regions_.clear();
     page_numbers_.clear();

-    flat_data_.reset();
+    flat_data_.clear();

     object_info_ = nullptr;
     start_addr_ = nullptr;
     size_ = 0;
     object_info_ = nullptr;
     start_addr_ = nullptr;
     size_ = 0;


@@ -170,24 +199,25 @@ public:
   void clear_data()
   {
     storage_type_ = StorageType::NoData;
   void clear_data()
   {
     storage_type_ = StorageType::NoData;

-    flat_data_.reset();
+    flat_data_.clear();

     page_numbers_.clear();
     privatized_regions_.clear();
   }
   
     page_numbers_.clear();
     privatized_regions_.clear();
   }
   

-  void flat_data(flat_data_ptr data)
+  void flat_data(buffer data)

   {
     storage_type_ = StorageType::Flat;
     flat_data_ = std::move(data);
     page_numbers_.clear();
     privatized_regions_.clear();
   }
   {
     storage_type_ = StorageType::Flat;
     flat_data_ = std::move(data);
     page_numbers_.clear();
     privatized_regions_.clear();
   }

-  const char* flat_data() const { return flat_data_.get(); }
+  const buffer& flat_data() const { return flat_data_; }
+  buffer& flat_data()             { return flat_data_; }

 
   void page_data(ChunkedData page_data)
   {
     storage_type_ = StorageType::Chunked;
 
   void page_data(ChunkedData page_data)
   {
     storage_type_ = StorageType::Chunked;

-    flat_data_.reset();
+    flat_data_.clear();

     page_numbers_ = std::move(page_data);
     privatized_regions_.clear();
   }
     page_numbers_ = std::move(page_data);
     privatized_regions_.clear();
   }


@@ -196,7 +226,7 @@ public:
   void privatized_data(std::vector<RegionSnapshot> data)
   {
     storage_type_ = StorageType::Privatized;
   void privatized_data(std::vector<RegionSnapshot> data)
   {
     storage_type_ = StorageType::Privatized;

-    flat_data_.reset();
+    flat_data_.clear();

     page_numbers_.clear();
     privatized_regions_ = std::move(data);
   }
     page_numbers_.clear();
     privatized_regions_ = std::move(data);
   }

diff --git a/src/mc/mc_checkpoint.cpp b/src/mc/mc_checkpoint.cpp
index baf90a2..07fa867 100644 (file)
--- a/src/mc/mc_checkpoint.cpp
+++ b/src/mc/mc_checkpoint.cpp
@@ -66,7 +66,7 @@ static void restore(mc_mem_region_t region)
     break;
 
   case simgrid::mc::StorageType::Flat:
     break;
 
   case simgrid::mc::StorageType::Flat:

-    mc_model_checker->process().write_bytes(region->flat_data(),
+    mc_model_checker->process().write_bytes(region->flat_data().get(),

       region->size(), region->permanent_address());
     break;
 
       region->size(), region->permanent_address());
     break;
 

diff --git a/src/mc/mc_snapshot.h b/src/mc/mc_snapshot.h
index 45db54c..a1e7399 100644 (file)
--- a/src/mc/mc_snapshot.h
+++ b/src/mc/mc_snapshot.h
@@ -55,7 +55,7 @@ void* mc_translate_address_region(uintptr_t addr, mc_mem_region_t region, int pr
   case simgrid::mc::StorageType::Flat:
     {
       uintptr_t offset = (uintptr_t) addr - (uintptr_t) region->start().address();
   case simgrid::mc::StorageType::Flat:
     {
       uintptr_t offset = (uintptr_t) addr - (uintptr_t) region->start().address();

-      return (void *) ((uintptr_t) region->flat_data() + offset);
+      return (void *) ((uintptr_t) region->flat_data().get() + offset);

     }
 
   case simgrid::mc::StorageType::Chunked:
     }
 
   case simgrid::mc::StorageType::Chunked:


@@ -243,7 +243,7 @@ const void* MC_region_read(
     xbt_die("Storage type not supported");
 
   case simgrid::mc::StorageType::Flat:
     xbt_die("Storage type not supported");
 
   case simgrid::mc::StorageType::Flat:

-    return (char*) region->flat_data() + offset;
+    return (char*) region->flat_data().get() + offset;

 
   case simgrid::mc::StorageType::Chunked:
     {
 
   case simgrid::mc::StorageType::Chunked:
     {