MultiDiskAdaptor: Use std::unique_ptr for DiskWriterEntry and DiskWriter

MultiFileAllocationIterator is also rewritten so that it does not
requre copying DiskWriterEntry objects.

1 files changed, 87 insertions, 116 deletions

diff --git a/src/MultiDiskAdaptor.cc b/src/MultiDiskAdaptor.cc
index 7e537be2..a432a9be 100644
--- a/src/MultiDiskAdaptor.cc
+++ b/src/MultiDiskAdaptor.cc
@@ -55,9 +55,10 @@
 namespace aria2 {
 
 DiskWriterEntry::DiskWriterEntry(const std::shared_ptr<FileEntry>& fileEntry)
-  : fileEntry_(fileEntry),
-    open_(false),
-    needsFileAllocation_(false)
+  : fileEntry_{fileEntry},
+    open_{false},
+    needsFileAllocation_{false},
+    needsDiskWriter_{false}
 {}
 
 const std::string& DiskWriterEntry::getFilePath() const
@@ -107,9 +108,9 @@ int64_t DiskWriterEntry::size() const
   return File(getFilePath()).size();
 }
 
-void DiskWriterEntry::setDiskWriter(const std::shared_ptr<DiskWriter>& diskWriter)
+void DiskWriterEntry::setDiskWriter(std::unique_ptr<DiskWriter> diskWriter)
 {
-  diskWriter_ = diskWriter;
+  diskWriter_ = std::move(diskWriter);
 }
 
 bool DiskWriterEntry::operator<(const DiskWriterEntry& entry) const
@@ -118,20 +119,19 @@ bool DiskWriterEntry::operator<(const DiskWriterEntry& entry) const
 }
 
 MultiDiskAdaptor::MultiDiskAdaptor()
-  : pieceLength_(0),
-    maxOpenFiles_(DEFAULT_MAX_OPEN_FILES),
-    readOnly_(false)
+  : pieceLength_{0},
+    maxOpenFiles_{DEFAULT_MAX_OPEN_FILES},
+    readOnly_{false}
 {}
 
 MultiDiskAdaptor::~MultiDiskAdaptor() {}
 
 namespace {
-std::shared_ptr<DiskWriterEntry> createDiskWriterEntry
-(const std::shared_ptr<FileEntry>& fileEntry,
- bool needsFileAllocation)
+std::unique_ptr<DiskWriterEntry> createDiskWriterEntry
+(const std::shared_ptr<FileEntry>& fileEntry)
 {
-  std::shared_ptr<DiskWriterEntry> entry(new DiskWriterEntry(fileEntry));
-  entry->needsFileAllocation(needsFileAllocation);
+  auto entry = make_unique<DiskWriterEntry>(fileEntry);
+  entry->needsFileAllocation(fileEntry->isRequested());
   return entry;
 }
 } // namespace
@@ -144,33 +144,28 @@ void MultiDiskAdaptor::resetDiskWriterEntries()
     return;
   }
 
-  for(std::vector<std::shared_ptr<FileEntry> >::const_iterator i =
-        getFileEntries().begin(), eoi = getFileEntries().end(); i != eoi; ++i) {
-    diskWriterEntries_.push_back
-      (createDiskWriterEntry(*i, (*i)->isRequested()));
+  for(auto& fileEntry: getFileEntries()) {
+    diskWriterEntries_.push_back(createDiskWriterEntry(fileEntry));
   }
-  std::map<std::string, bool> dwreq;
   // TODO Currently, pieceLength_ == 0 is used for unit testing only.
   if(pieceLength_ > 0) {
-    std::vector<std::shared_ptr<DiskWriterEntry> >::const_iterator done =
-      diskWriterEntries_.begin();
-    for(std::vector<std::shared_ptr<DiskWriterEntry> >::const_iterator itr =
-          diskWriterEntries_.begin(), eoi = diskWriterEntries_.end();
-        itr != eoi;) {
-      const std::shared_ptr<FileEntry>& fileEntry = (*itr)->getFileEntry();
-
+    auto done = std::begin(diskWriterEntries_);
+    for(auto itr = std::begin(diskWriterEntries_),
+          eoi = std::end(diskWriterEntries_); itr != eoi;) {
+      auto& fileEntry = (*itr)->getFileEntry();
       if(!fileEntry->isRequested()) {
         ++itr;
         continue;
       }
       int64_t pieceStartOffset =
         (fileEntry->getOffset()/pieceLength_)*pieceLength_;
-      if(itr != diskWriterEntries_.begin()) {
-        for(std::vector<std::shared_ptr<DiskWriterEntry> >::const_iterator i =
-              itr-1; true; --i) {
-          const std::shared_ptr<FileEntry>& fileEntry = (*i)->getFileEntry();
+      if(itr != std::begin(diskWriterEntries_)) {
+        for(auto i = itr-1;; --i) {
+          auto& fileEntry = (*i)->getFileEntry();
           if(pieceStartOffset <= fileEntry->getOffset() ||
              pieceStartOffset < fileEntry->getLastOffset()) {
+            A2_LOG_DEBUG(fmt("%s needs file allocation",
+                             (*i)->getFileEntry()->getPath().c_str()));
             (*i)->needsFileAllocation(true);
           } else {
             break;
@@ -205,7 +200,7 @@ void MultiDiskAdaptor::resetDiskWriterEntries()
             A2_LOG_DEBUG
               (fmt("%s needs diskwriter",
                    (*itr)->getFileEntry()->getPath().c_str()));
-            dwreq[(*itr)->getFileEntry()->getPath()] = true;
+            (*itr)->needsDiskWriter(true);
           } else {
             break;
           }
@@ -218,17 +213,15 @@ void MultiDiskAdaptor::resetDiskWriterEntries()
     }
   }
   DefaultDiskWriterFactory dwFactory;
-  for(std::vector<std::shared_ptr<DiskWriterEntry> >::const_iterator i =
-        diskWriterEntries_.begin(), eoi = diskWriterEntries_.end();
-      i != eoi; ++i) {
-    if((*i)->needsFileAllocation() ||
-       dwreq.find((*i)->getFileEntry()->getPath()) != dwreq.end() ||
-       (*i)->fileExists()) {
+  for(auto& dwent : diskWriterEntries_) {
+    if(dwent->needsFileAllocation() ||
+       dwent->needsDiskWriter() ||
+       dwent->fileExists()) {
       A2_LOG_DEBUG(fmt("Creating DiskWriter for filename=%s",
-                       (*i)->getFilePath().c_str()));
-      (*i)->setDiskWriter(dwFactory.newDiskWriter((*i)->getFilePath()));
+                       dwent->getFilePath().c_str()));
+      dwent->setDiskWriter(dwFactory.newDiskWriter(dwent->getFilePath()));
       if(readOnly_) {
-        (*i)->getDiskWriter()->enableReadOnly();
+        dwent->getDiskWriter()->enableReadOnly();
       }
       // TODO mmap is not enabled at this moment. Call enableMmap()
       // after this function call.
@@ -237,24 +230,22 @@ void MultiDiskAdaptor::resetDiskWriterEntries()
 }
 
 void MultiDiskAdaptor::openIfNot
-(const std::shared_ptr<DiskWriterEntry>& entry, void (DiskWriterEntry::*open)())
+(DiskWriterEntry* entry, void (DiskWriterEntry::*open)())
 {
   if(!entry->isOpen()) {
     //     A2_LOG_DEBUG(fmt("DiskWriterEntry: Cache MISS. offset=%s",
     //            util::itos(entry->getFileEntry()->getOffset()).c_str()));
-
     int numOpened = openedDiskWriterEntries_.size();
-    (entry.get()->*open)();
+    (entry->*open)();
     if(numOpened >= maxOpenFiles_) {
       // Cache is full.
       // Choose one DiskWriterEntry randomly and close it.
       size_t index =
         SimpleRandomizer::getInstance()->getRandomNumber(numOpened);
-      std::vector<std::shared_ptr<DiskWriterEntry> >::iterator i =
-        openedDiskWriterEntries_.begin();
+      auto i = std::begin(openedDiskWriterEntries_);
       std::advance(i, index);
       (*i)->closeFile();
-      (*i) = entry;
+      *i = entry;
     } else {
       openedDiskWriterEntries_.push_back(entry);
     }
@@ -272,9 +263,8 @@ void MultiDiskAdaptor::openFile()
 
   // Call DiskWriterEntry::openFile to make sure that zero-length files are
   // created.
-  for(DiskWriterEntries::const_iterator itr = diskWriterEntries_.begin(),
-        eoi = diskWriterEntries_.end(); itr != eoi; ++itr) {
-    openIfNot(*itr, &DiskWriterEntry::openFile);
+  for(auto& dwent : diskWriterEntries_) {
+    openIfNot(dwent.get(), &DiskWriterEntry::openFile);
   }
 }
 
@@ -285,9 +275,8 @@ void MultiDiskAdaptor::initAndOpenFile()
   // we don't need to call it here.
 
   // Call DiskWriterEntry::initAndOpenFile to make files truncated.
-  for(DiskWriterEntries::const_iterator itr = diskWriterEntries_.begin(),
-        eoi = diskWriterEntries_.end(); itr != eoi; ++itr) {
-    openIfNot(*itr, &DiskWriterEntry::initAndOpenFile);
+  for(auto& dwent : diskWriterEntries_) {
+    openIfNot(dwent.get(), &DiskWriterEntry::initAndOpenFile);
   }
 }
 
@@ -299,12 +288,14 @@ void MultiDiskAdaptor::openExistingFile()
 
 void MultiDiskAdaptor::closeFile()
 {
-  std::for_each(diskWriterEntries_.begin(), diskWriterEntries_.end(),
-                std::mem_fn(&DiskWriterEntry::closeFile));
+  openedDiskWriterEntries_.clear();
+  for(auto& dwent : diskWriterEntries_) {
+    dwent->closeFile();
+  }
 }
 
 namespace {
-bool isInRange(const std::shared_ptr<DiskWriterEntry> entry, int64_t offset)
+bool isInRange(DiskWriterEntry* entry, int64_t offset)
 {
   return entry->getFileEntry()->getOffset() <= offset &&
     offset < entry->getFileEntry()->getLastOffset();
@@ -312,7 +303,7 @@ bool isInRange(const std::shared_ptr<DiskWriterEntry> entry, int64_t offset)
 } // namespace
 
 namespace {
-ssize_t calculateLength(const std::shared_ptr<DiskWriterEntry> entry,
+ssize_t calculateLength(DiskWriterEntry* entry,
                         int64_t fileOffset, ssize_t rem)
 {
   if(entry->getFileEntry()->getLength() < fileOffset+rem) {
@@ -326,7 +317,7 @@ ssize_t calculateLength(const std::shared_ptr<DiskWriterEntry> entry,
 namespace {
 class OffsetCompare {
 public:
-  bool operator()(int64_t offset, const std::shared_ptr<DiskWriterEntry>& dwe)
+  bool operator()(int64_t offset, const std::unique_ptr<DiskWriterEntry>& dwe)
   {
     return offset < dwe->getFileEntry()->getOffset();
   }
@@ -337,14 +328,12 @@ namespace {
 DiskWriterEntries::const_iterator findFirstDiskWriterEntry
 (const DiskWriterEntries& diskWriterEntries, int64_t offset)
 {
-  DiskWriterEntries::const_iterator first =
-    std::upper_bound(diskWriterEntries.begin(), diskWriterEntries.end(),
-                     offset, OffsetCompare());
-
+  auto first = std::upper_bound(std::begin(diskWriterEntries),
+                                std::end(diskWriterEntries),
+                                offset, OffsetCompare());
   --first;
-
   // In case when offset is out-of-range
-  if(!isInRange(*first, offset)) {
+  if(!isInRange((*first).get(), offset)) {
     throw DL_ABORT_EX
       (fmt(EX_FILE_OFFSET_OUT_OF_RANGE, static_cast<int64_t>(offset)));
   }
@@ -353,8 +342,7 @@ DiskWriterEntries::const_iterator findFirstDiskWriterEntry
 } // namespace
 
 namespace {
-void throwOnDiskWriterNotOpened(const std::shared_ptr<DiskWriterEntry>& e,
-                                int64_t offset)
+void throwOnDiskWriterNotOpened(DiskWriterEntry* e, int64_t offset)
 {
   throw DL_ABORT_EX
     (fmt("DiskWriter for offset=%" PRId64 ", filename=%s is not opened.",
@@ -366,19 +354,14 @@ void throwOnDiskWriterNotOpened(const std::shared_ptr<DiskWriterEntry>& e,
 void MultiDiskAdaptor::writeData(const unsigned char* data, size_t len,
                                  int64_t offset)
 {
-  DiskWriterEntries::const_iterator first =
-    findFirstDiskWriterEntry(diskWriterEntries_, offset);
-
+  auto first = findFirstDiskWriterEntry(diskWriterEntries_, offset);
   ssize_t rem = len;
   int64_t fileOffset = offset-(*first)->getFileEntry()->getOffset();
-  for(DiskWriterEntries::const_iterator i = first,
-        eoi = diskWriterEntries_.end(); i != eoi; ++i) {
-    ssize_t writeLength = calculateLength(*i, fileOffset, rem);
-
-    openIfNot(*i, &DiskWriterEntry::openFile);
-
+  for(auto i = first, eoi = diskWriterEntries_.cend(); i != eoi; ++i) {
+    ssize_t writeLength = calculateLength((*i).get(), fileOffset, rem);
+    openIfNot((*i).get(), &DiskWriterEntry::openFile);
     if(!(*i)->isOpen()) {
-      throwOnDiskWriterNotOpened(*i, offset+(len-rem));
+      throwOnDiskWriterNotOpened((*i).get(), offset+(len-rem));
     }
 
     (*i)->getDiskWriter()->writeData(data+(len-rem), writeLength, fileOffset);
@@ -393,22 +376,16 @@ void MultiDiskAdaptor::writeData(const unsigned char* data, size_t len,
 ssize_t MultiDiskAdaptor::readData
 (unsigned char* data, size_t len, int64_t offset)
 {
-  DiskWriterEntries::const_iterator first =
-    findFirstDiskWriterEntry(diskWriterEntries_, offset);
-
+  auto first = findFirstDiskWriterEntry(diskWriterEntries_, offset);
   ssize_t rem = len;
   ssize_t totalReadLength = 0;
   int64_t fileOffset = offset-(*first)->getFileEntry()->getOffset();
-  for(DiskWriterEntries::const_iterator i = first,
-        eoi = diskWriterEntries_.end(); i != eoi; ++i) {
-    ssize_t readLength = calculateLength(*i, fileOffset, rem);
-
-    openIfNot(*i, &DiskWriterEntry::openFile);
-
+  for(auto i = first, eoi = diskWriterEntries_.cend(); i != eoi; ++i) {
+    ssize_t readLength = calculateLength((*i).get(), fileOffset, rem);
+    openIfNot((*i).get(), &DiskWriterEntry::openFile);
     if(!(*i)->isOpen()) {
-      throwOnDiskWriterNotOpened(*i, offset+(len-rem));
+      throwOnDiskWriterNotOpened((*i).get(), offset+(len-rem));
     }
-
     totalReadLength +=
       (*i)->getDiskWriter()->readData(data+(len-rem), readLength, fileOffset);
     rem -= readLength;
@@ -427,27 +404,26 @@ void MultiDiskAdaptor::writeCache(const WrDiskCacheEntry* entry)
   unsigned char buf[16*1024];
   size_t buflen = 0;
   size_t buffoffset = 0;
-  const WrDiskCacheEntry::DataCellSet& dataSet = entry->getDataSet();
+  auto& dataSet = entry->getDataSet();
   if(dataSet.empty()) {
     return;
   }
-  DiskWriterEntries::const_iterator dent =
+  auto dent =
     findFirstDiskWriterEntry(diskWriterEntries_, (*dataSet.begin())->goff),
-    eod = diskWriterEntries_.end();
-  WrDiskCacheEntry::DataCellSet::const_iterator i = dataSet.begin(),
-    eoi = dataSet.end();
+    eod = diskWriterEntries_.cend();
+  auto i = std::begin(dataSet), eoi = std::end(dataSet);
   size_t celloff = 0;
   for(; dent != eod; ++dent) {
     int64_t lstart = 0, lp = 0;
-    const std::shared_ptr<FileEntry>& fent = (*dent)->getFileEntry();
+    auto& fent = (*dent)->getFileEntry();
     for(; i != eoi;) {
       if(std::max(fent->getOffset(),
                   static_cast<int64_t>((*i)->goff + celloff)) <
          std::min(fent->getLastOffset(),
                   static_cast<int64_t>((*i)->goff + (*i)->len))) {
-        openIfNot(*dent, &DiskWriterEntry::openFile);
+        openIfNot((*dent).get(), &DiskWriterEntry::openFile);
         if(!(*dent)->isOpen()) {
-          throwOnDiskWriterNotOpened(*dent, (*i)->goff + celloff);
+          throwOnDiskWriterNotOpened((*dent).get(), (*i)->goff + celloff);
         }
       } else {
         A2_LOG_DEBUG(fmt("%s Cache flush loff=%" PRId64 ", len=%lu",
@@ -519,25 +495,25 @@ void MultiDiskAdaptor::writeCache(const WrDiskCacheEntry* entry)
 
 bool MultiDiskAdaptor::fileExists()
 {
-  return std::find_if(getFileEntries().begin(), getFileEntries().end(),
+  return std::find_if(std::begin(getFileEntries()),
+                      std::end(getFileEntries()),
                       std::mem_fn(&FileEntry::exists)) !=
-    getFileEntries().end();
+    std::end(getFileEntries());
 }
 
 int64_t MultiDiskAdaptor::size()
 {
   int64_t size = 0;
-  for(std::vector<std::shared_ptr<FileEntry> >::const_iterator i =
-        getFileEntries().begin(), eoi = getFileEntries().end(); i != eoi; ++i) {
-    size += File((*i)->getPath()).size();
+  for(auto& fe : getFileEntries()) {
+    size += File(fe->getPath()).size();
   }
   return size;
 }
 
-std::shared_ptr<FileAllocationIterator> MultiDiskAdaptor::fileAllocationIterator()
+std::shared_ptr<FileAllocationIterator>
+MultiDiskAdaptor::fileAllocationIterator()
 {
-  return std::shared_ptr<FileAllocationIterator>
-    (new MultiFileAllocationIterator(this));
+  return std::make_shared<MultiFileAllocationIterator>(this);
 }
 
 void MultiDiskAdaptor::enableReadOnly()
@@ -552,10 +528,8 @@ void MultiDiskAdaptor::disableReadOnly()
 
 void MultiDiskAdaptor::enableMmap()
 {
-  for(std::vector<std::shared_ptr<DiskWriterEntry> >::const_iterator i =
-        diskWriterEntries_.begin(), eoi = diskWriterEntries_.end();
-      i != eoi; ++i) {
-    const std::shared_ptr<DiskWriter>& dw = (*i)->getDiskWriter();
+  for(auto& dwent : diskWriterEntries_) {
+    auto& dw = dwent->getDiskWriter();
     if(dw) {
       dw->enableMmap();
     }
@@ -564,14 +538,12 @@ void MultiDiskAdaptor::enableMmap()
 
 void MultiDiskAdaptor::cutTrailingGarbage()
 {
-  for(std::vector<std::shared_ptr<DiskWriterEntry> >::const_iterator i =
-        diskWriterEntries_.begin(), eoi = diskWriterEntries_.end();
-      i != eoi; ++i) {
-    int64_t length = (*i)->getFileEntry()->getLength();
-    if(File((*i)->getFilePath()).size() > length) {
+  for(auto& dwent : diskWriterEntries_) {
+    int64_t length = dwent->getFileEntry()->getLength();
+    if(File(dwent->getFilePath()).size() > length) {
       // We need open file before calling DiskWriter::truncate(int64_t)
-      openIfNot(*i, &DiskWriterEntry::openFile);
-      (*i)->getDiskWriter()->truncate(length);
+      openIfNot(dwent.get(), &DiskWriterEntry::openFile);
+      dwent->getDiskWriter()->truncate(length);
     }
   }
 }
@@ -584,10 +556,9 @@ void MultiDiskAdaptor::setMaxOpenFiles(int maxOpenFiles)
 size_t MultiDiskAdaptor::utime(const Time& actime, const Time& modtime)
 {
   size_t numOK = 0;
-  for(std::vector<std::shared_ptr<FileEntry> >::const_iterator i =
-        getFileEntries().begin(), eoi = getFileEntries().end(); i != eoi; ++i) {
-    if((*i)->isRequested()) {
-      File f((*i)->getPath());
+  for(auto& fe : getFileEntries()) {
+    if(fe->isRequested()) {
+      File f{fe->getPath()};
       if(f.isFile() && f.utime(actime, modtime)) {
         ++numOK;
       }