Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
codefactor: reduce complexity in s4u_FileSystem.cpp
authorFrederic Suter <frederic.suter@cc.in2p3.fr>
Wed, 18 Dec 2019 14:27:20 +0000 (15:27 +0100)
committerFrederic Suter <frederic.suter@cc.in2p3.fr>
Wed, 18 Dec 2019 14:31:42 +0000 (15:31 +0100)
(and use a bool when it's a boolean parameter)

include/simgrid/plugins/file_system.h
src/plugins/file_system/s4u_FileSystem.cpp
src/smpi/mpi/smpi_file.cpp

index 179ab9f..e95a56f 100644 (file)
@@ -96,6 +96,16 @@ namespace s4u {
  * For now, you cannot change the mountpoints programmatically, and must declare them from your platform file.
  */
 class XBT_PUBLIC File : public xbt::Extendable<File> {
+  sg_size_t size_;
+  std::string path_;
+  std::string fullpath_;
+  sg_size_t current_position_ = SEEK_SET;
+
+  Storage* find_local_storage_on(Host* host);
+  Disk* find_local_disk_on(Host* host);
+  sg_size_t write_on_storage(sg_size_t size, bool write_inside);
+  sg_size_t write_on_disk(sg_size_t size, bool write_inside);
+
 public:
   File(const std::string& fullpath, void* userdata);
   File(const std::string& fullpath, sg_host_t host, void* userdata);
@@ -110,7 +120,7 @@ public:
   sg_size_t read(sg_size_t size);
 
   /** Simulates a write action. Returns the size of data actually written. */
-  sg_size_t write(sg_size_t size, int write_inside=0);
+  sg_size_t write(sg_size_t size, bool write_inside = false);
 
   /** Allows to store user data on that host */
   XBT_ATTRIB_DEPRECATED_v329("Please use set_data()") void set_userdata(void* data) { set_data(data); }
@@ -134,12 +144,6 @@ public:
   Disk* local_disk_       = nullptr;
   Storage* local_storage_ = nullptr;
   std::string mount_point_;
-
-private:
-  sg_size_t size_;
-  std::string path_;
-  std::string fullpath_;
-  sg_size_t current_position_ = SEEK_SET;
 };
 
 class XBT_PUBLIC FileSystemDiskExt {
index 6b63130..88ec37b 100644 (file)
@@ -26,61 +26,71 @@ simgrid::xbt::Extension<Disk, FileSystemDiskExt> FileSystemDiskExt::EXTENSION_ID
 simgrid::xbt::Extension<Storage, FileSystemStorageExt> FileSystemStorageExt::EXTENSION_ID;
 simgrid::xbt::Extension<Host, FileDescriptorHostExt> FileDescriptorHostExt::EXTENSION_ID;
 
-File::File(const std::string& fullpath, void* userdata) : File(fullpath, Host::current(), userdata){}
-
-File::File(const std::string& fullpath, sg_host_t host, void* userdata) : fullpath_(fullpath)
+Storage* File::find_local_storage_on(Host* host)
 {
-  this->set_data(userdata);
-  // this cannot fail because we get a xbt_die if the mountpoint does not exist
-  if (not host->get_mounted_storages().empty()) {
-    Storage* st                  = nullptr;
-    size_t longest_prefix_length = 0;
-    XBT_DEBUG("Search for storage name for '%s' on '%s'", fullpath_.c_str(), host->get_cname());
+  Storage* st                  = nullptr;
+  size_t longest_prefix_length = 0;
+  XBT_DEBUG("Search for storage name for '%s' on '%s'", fullpath_.c_str(), host->get_cname());
 
-    for (auto const& mnt : host->get_mounted_storages()) {
-      XBT_DEBUG("See '%s'", mnt.first.c_str());
-      mount_point_ = fullpath_.substr(0, mnt.first.length());
+  for (auto const& mnt : host->get_mounted_storages()) {
+    XBT_DEBUG("See '%s'", mnt.first.c_str());
+    mount_point_ = fullpath_.substr(0, mnt.first.length());
 
-      if (mount_point_ == mnt.first && mnt.first.length() > longest_prefix_length) {
-        /* The current mount name is found in the full path and is bigger than the previous*/
-        longest_prefix_length = mnt.first.length();
-        st                    = mnt.second;
-      }
+    if (mount_point_ == mnt.first && mnt.first.length() > longest_prefix_length) {
+      /* The current mount name is found in the full path and is bigger than the previous*/
+      longest_prefix_length = mnt.first.length();
+      st                    = mnt.second;
+    }
+  }
+  if (longest_prefix_length > 0) { /* Mount point found, split fullpath_ into mount_name and path+filename*/
+    mount_point_ = fullpath_.substr(0, longest_prefix_length);
+    path_        = fullpath_.substr(longest_prefix_length, fullpath_.length());
+  } else
+    xbt_die("Can't find mount point for '%s' on '%s'", fullpath_.c_str(), host->get_cname());
+
+  return st;
+}
+
+Disk* File::find_local_disk_on(Host* host)
+{
+  Disk* d                      = nullptr;
+  size_t longest_prefix_length = 0;
+  for (auto const& disk : host->get_disks()) {
+    std::string current_mount;
+    if (disk->get_host() != host)
+      current_mount = disk->extension<FileSystemDiskExt>()->get_mount_point(disk->get_host());
+    else
+      current_mount = disk->extension<FileSystemDiskExt>()->get_mount_point();
+    mount_point_ = fullpath_.substr(0, current_mount.length());
+    if (mount_point_ == current_mount && current_mount.length() > longest_prefix_length) {
+      /* The current mount name is found in the full path and is bigger than the previous*/
+      longest_prefix_length = current_mount.length();
+      d                     = disk;
     }
     if (longest_prefix_length > 0) { /* Mount point found, split fullpath_ into mount_name and path+filename*/
       mount_point_ = fullpath_.substr(0, longest_prefix_length);
-      path_        = fullpath_.substr(longest_prefix_length, fullpath_.length());
+      if (mount_point_ == std::string("/"))
+        path_ = fullpath_;
+      else
+        path_ = fullpath_.substr(longest_prefix_length, fullpath_.length());
+      XBT_DEBUG("%s + %s", mount_point_.c_str(), path_.c_str());
     } else
       xbt_die("Can't find mount point for '%s' on '%s'", fullpath_.c_str(), host->get_cname());
+  }
+  return d;
+}
 
-    local_storage_ = st;
+File::File(const std::string& fullpath, void* userdata) : File(fullpath, Host::current(), userdata) {}
+
+File::File(const std::string& fullpath, sg_host_t host, void* userdata) : fullpath_(fullpath)
+{
+  this->set_data(userdata);
+  // this cannot fail because we get a xbt_die if the mountpoint does not exist
+  if (not host->get_mounted_storages().empty()) {
+    local_storage_ = find_local_storage_on(host);
   }
   if (not host->get_disks().empty()) {
-    Disk* d                      = nullptr;
-    size_t longest_prefix_length = 0;
-    for (auto const& disk : host->get_disks()) {
-      std::string current_mount;
-      if (disk->get_host() != host)
-        current_mount = disk->extension<FileSystemDiskExt>()->get_mount_point(disk->get_host());
-      else
-        current_mount = disk->extension<FileSystemDiskExt>()->get_mount_point();
-      mount_point_              = fullpath_.substr(0, current_mount.length());
-      if (mount_point_ == current_mount && current_mount.length() > longest_prefix_length) {
-        /* The current mount name is found in the full path and is bigger than the previous*/
-        longest_prefix_length = current_mount.length();
-        d                     = disk;
-      }
-      if (longest_prefix_length > 0) { /* Mount point found, split fullpath_ into mount_name and path+filename*/
-        mount_point_ = fullpath_.substr(0, longest_prefix_length);
-        if (mount_point_ == std::string("/"))
-          path_ = fullpath_;
-        else
-          path_ = fullpath_.substr(longest_prefix_length, fullpath_.length());
-        XBT_DEBUG("%s + %s", mount_point_.c_str(), path_.c_str());
-      } else
-        xbt_die("Can't find mount point for '%s' on '%s'", fullpath_.c_str(), host->get_cname());
-    }
-    local_disk_ = d;
+    local_disk_ = find_local_disk_on(host);
   }
 
   // assign a file descriptor id to the newly opened File
@@ -180,77 +190,94 @@ sg_size_t File::read(sg_size_t size)
  * @param size of the file to write
  * @return the number of bytes successfully write or -1 if an error occurred
  */
-sg_size_t File::write(sg_size_t size, int write_inside)
+sg_size_t File::write_on_disk(sg_size_t size, bool write_inside)
 {
-  if (size == 0) /* Nothing to write, return */
-    return 0;
   sg_size_t write_size = 0;
-  Host* host           = nullptr;
-
   /* Find the host where the file is physically located (remote or local)*/
-  if (local_storage_)
-    host = local_storage_->get_host();
-  if (local_disk_)
-    host = local_disk_->get_host();
+  Host* host = local_disk_->get_host();
 
   if (host && host->get_name() != Host::current()->get_name()) {
     /* the file is hosted on a remote host, initiate a communication between src and dest hosts for data transfer */
     XBT_DEBUG("File is on %s remote host, initiate data transfer of %llu bytes.", host->get_cname(), size);
     Host::current()->send_to(host, size);
   }
-
-  if (local_storage_) {
-    XBT_DEBUG("WRITE %s on disk '%s'. size '%llu/%llu' '%llu:%llu'", get_path(), local_storage_->get_cname(), size,
-              size_, sg_storage_get_size_used(local_storage_), sg_storage_get_size(local_storage_));
-    // If the storage is full before even starting to write
-    if (sg_storage_get_size_used(local_storage_) >= sg_storage_get_size(local_storage_))
-      return 0;
-    if (write_inside == 0) {
-      /* Subtract the part of the file that might disappear from the used sized on the storage element */
-      local_storage_->extension<FileSystemStorageExt>()->decr_used_size(size_ - current_position_);
-      write_size = local_storage_->write(size);
-      local_storage_->extension<FileSystemStorageExt>()->incr_used_size(write_size);
-      current_position_ += write_size;
+  XBT_DEBUG("WRITE %s on disk '%s'. size '%llu/%llu' '%llu:%llu'", get_path(), local_disk_->get_cname(), size, size_,
+            sg_disk_get_size_used(local_disk_), sg_disk_get_size(local_disk_));
+  // If the storage is full before even starting to write
+  if (sg_disk_get_size_used(local_disk_) >= sg_disk_get_size(local_disk_))
+    return 0;
+  if (not write_inside) {
+    /* Subtract the part of the file that might disappear from the used sized on the storage element */
+    local_disk_->extension<FileSystemDiskExt>()->decr_used_size(size_ - current_position_);
+    write_size = local_disk_->write(size);
+    local_disk_->extension<FileSystemDiskExt>()->incr_used_size(write_size);
+    current_position_ += write_size;
+    size_ = current_position_;
+  } else {
+    write_size = local_disk_->write(size);
+    current_position_ += write_size;
+    if (current_position_ > size_)
       size_ = current_position_;
-    } else {
-      write_size = local_storage_->write(size);
-      current_position_ += write_size;
-      if (current_position_ > size_)
-        size_ = current_position_;
-    }
-    std::map<std::string, sg_size_t>* content = local_storage_->extension<FileSystemStorageExt>()->get_content();
+  }
+  std::map<std::string, sg_size_t>* content = local_disk_->extension<FileSystemDiskExt>()->get_content();
 
-    content->erase(path_);
-    content->insert({path_, size_});
+  content->erase(path_);
+  content->insert({path_, size_});
+
+  return write_size;
+}
+
+sg_size_t File::write_on_storage(sg_size_t size, bool write_inside)
+{
+  sg_size_t write_size = 0;
+  /* Find the host where the file is physically located (remote or local)*/
+  Host* host = local_storage_->get_host();
+
+  if (host && host->get_name() != Host::current()->get_name()) {
+    /* the file is hosted on a remote host, initiate a communication between src and dest hosts for data transfer */
+    XBT_DEBUG("File is on %s remote host, initiate data transfer of %llu bytes.", host->get_cname(), size);
+    Host::current()->send_to(host, size);
   }
 
-  if (local_disk_) {
-    XBT_DEBUG("WRITE %s on disk '%s'. size '%llu/%llu' '%llu:%llu'", get_path(), local_disk_->get_cname(), size, size_,
-              sg_disk_get_size_used(local_disk_), sg_disk_get_size(local_disk_));
-    // If the storage is full before even starting to write
-    if (sg_disk_get_size_used(local_disk_) >= sg_disk_get_size(local_disk_))
-      return 0;
-    if (write_inside == 0) {
-      /* Subtract the part of the file that might disappear from the used sized on the storage element */
-      local_disk_->extension<FileSystemDiskExt>()->decr_used_size(size_ - current_position_);
-      write_size = local_disk_->write(size);
-      local_disk_->extension<FileSystemDiskExt>()->incr_used_size(write_size);
-      current_position_ += write_size;
+  XBT_DEBUG("WRITE %s on disk '%s'. size '%llu/%llu' '%llu:%llu'", get_path(), local_storage_->get_cname(), size, size_,
+            sg_storage_get_size_used(local_storage_), sg_storage_get_size(local_storage_));
+  // If the storage is full before even starting to write
+  if (sg_storage_get_size_used(local_storage_) >= sg_storage_get_size(local_storage_))
+    return 0;
+  if (not write_inside) {
+    /* Subtract the part of the file that might disappear from the used sized on the storage element */
+    local_storage_->extension<FileSystemStorageExt>()->decr_used_size(size_ - current_position_);
+    write_size = local_storage_->write(size);
+    local_storage_->extension<FileSystemStorageExt>()->incr_used_size(write_size);
+    current_position_ += write_size;
+    size_ = current_position_;
+  } else {
+    write_size = local_storage_->write(size);
+    current_position_ += write_size;
+    if (current_position_ > size_)
       size_ = current_position_;
-    } else {
-      write_size = local_disk_->write(size);
-      current_position_ += write_size;
-      if (current_position_ > size_)
-        size_ = current_position_;
-    }
-    std::map<std::string, sg_size_t>* content = local_disk_->extension<FileSystemDiskExt>()->get_content();
-
-    content->erase(path_);
-    content->insert({path_, size_});
   }
+  std::map<std::string, sg_size_t>* content = local_storage_->extension<FileSystemStorageExt>()->get_content();
+
+  content->erase(path_);
+  content->insert({path_, size_});
+
   return write_size;
 }
 
+sg_size_t File::write(sg_size_t size, bool write_inside)
+{
+  if (size == 0) /* Nothing to write, return */
+    return 0;
+
+  if (local_disk_)
+    return write_on_disk(size, write_inside);
+  if (local_storage_)
+    return write_on_storage(size, write_inside);
+
+  return 0;
+}
+
 sg_size_t File::size()
 {
   return size_;
@@ -367,10 +394,10 @@ int File::remote_copy(sg_host_t host, const char* fullpath)
   current_position_ += read_size;
 
   Host* dst_host = host;
+  size_t longest_prefix_length = 0;
   if (local_storage_) {
     /* Find the host that owns the storage where the file has to be copied */
     Storage* storage_dest        = nullptr;
-    size_t longest_prefix_length = 0;
 
     for (auto const& elm : host->get_mounted_storages()) {
       std::string mount_point = std::string(fullpath).substr(0, elm.first.size());
@@ -391,7 +418,6 @@ int File::remote_copy(sg_host_t host, const char* fullpath)
   }
 
   if (local_disk_) {
-    size_t longest_prefix_length = 0;
     Disk* dst_disk               = nullptr;
 
     for (auto const& disk : host->get_disks()) {
index 4585a58..4c73765 100644 (file)
@@ -206,7 +206,7 @@ namespace smpi{
     MPI_Offset movesize = datatype->get_extent()*count;
     MPI_Offset writesize = datatype->size()*count;
     XBT_DEBUG("Position before write in MPI_File %s : %llu",fh->file_->get_path(),fh->file_->tell());
-    MPI_Offset write = fh->file_->write(writesize, 1);
+    MPI_Offset write = fh->file_->write(writesize, true);
     XBT_VERB("Write in MPI_File %s, %lld bytes written, readsize %lld bytes, movesize %lld", fh->file_->get_path(), write, writesize, movesize);
     if(writesize!=movesize){
       fh->file_->seek(position+movesize, SEEK_SET);