From f000fd0cab7860920b5269c018ea3eaae9f35e2e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 5 Jul 2013 21:08:24 +0900 Subject: [PATCH] MultiDiskAdaptor: Use std::unique_ptr for DiskWriterEntry and DiskWriter MultiFileAllocationIterator is also rewritten so that it does not requre copying DiskWriterEntry objects. --- src/AnonDiskWriterFactory.h | 5 +- src/DefaultDiskWriterFactory.cc | 5 +- src/DefaultDiskWriterFactory.h | 3 +- src/DefaultPieceStorage.cc | 5 +- src/DiskWriterFactory.h | 3 +- src/MultiDiskAdaptor.cc | 203 ++++++++++-------------- src/MultiDiskAdaptor.h | 26 +-- src/MultiFileAllocationIterator.cc | 60 ++++--- src/MultiFileAllocationIterator.h | 8 +- src/UnknownLengthPieceStorage.cc | 5 +- test/MultiDiskAdaptorTest.cc | 80 ++++------ test/MultiFileAllocationIteratorTest.cc | 108 ++++++------- 12 files changed, 236 insertions(+), 275 deletions(-) diff --git a/src/AnonDiskWriterFactory.h b/src/AnonDiskWriterFactory.h index 60146827..4d3c0116 100644 --- a/src/AnonDiskWriterFactory.h +++ b/src/AnonDiskWriterFactory.h @@ -36,6 +36,7 @@ #define D_ANON_DISK_WRITER_FACTORY_H #include "DiskWriterFactory.h" +#include "a2functional.h" namespace aria2 { @@ -47,9 +48,9 @@ public: AnonDiskWriterFactory() {} virtual ~AnonDiskWriterFactory() {} - virtual std::shared_ptr newDiskWriter(const std::string& filename) + virtual std::unique_ptr newDiskWriter(const std::string& filename) { - return std::shared_ptr(new DiskWriterType()); + return make_unique(); } }; diff --git a/src/DefaultDiskWriterFactory.cc b/src/DefaultDiskWriterFactory.cc index b2bd9be2..a0547252 100644 --- a/src/DefaultDiskWriterFactory.cc +++ b/src/DefaultDiskWriterFactory.cc @@ -34,13 +34,14 @@ /* copyright --> */ #include "DefaultDiskWriterFactory.h" #include "DefaultDiskWriter.h" +#include "a2functional.h" namespace aria2 { -std::shared_ptr DefaultDiskWriterFactory::newDiskWriter +std::unique_ptr DefaultDiskWriterFactory::newDiskWriter (const std::string& filename) { - return std::shared_ptr(new DefaultDiskWriter(filename)); + return make_unique(filename); } } // namespace aria2 diff --git a/src/DefaultDiskWriterFactory.h b/src/DefaultDiskWriterFactory.h index f13f07e9..41696a76 100644 --- a/src/DefaultDiskWriterFactory.h +++ b/src/DefaultDiskWriterFactory.h @@ -44,7 +44,8 @@ class DiskWriter; class DefaultDiskWriterFactory:public DiskWriterFactory { public: - virtual std::shared_ptr newDiskWriter(const std::string& filename); + virtual std::unique_ptr newDiskWriter + (const std::string& filename); }; } // namespace aria2 diff --git a/src/DefaultPieceStorage.cc b/src/DefaultPieceStorage.cc index 656c2e23..8af48b8e 100644 --- a/src/DefaultPieceStorage.cc +++ b/src/DefaultPieceStorage.cc @@ -643,9 +643,8 @@ void DefaultPieceStorage::initStorage() (downloadContext_->getFileEntries().begin(), downloadContext_->getFileEntries().end()); - std::shared_ptr writer = - diskWriterFactory_->newDiskWriter(directDiskAdaptor->getFilePath()); - directDiskAdaptor->setDiskWriter(writer); + directDiskAdaptor->setDiskWriter + (diskWriterFactory_->newDiskWriter(directDiskAdaptor->getFilePath())); diskAdaptor_.reset(directDiskAdaptor); } else { A2_LOG_DEBUG("Instantiating MultiDiskAdaptor"); diff --git a/src/DiskWriterFactory.h b/src/DiskWriterFactory.h index 56a8cef0..0677dfad 100644 --- a/src/DiskWriterFactory.h +++ b/src/DiskWriterFactory.h @@ -48,7 +48,8 @@ class DiskWriterFactory { public: virtual ~DiskWriterFactory() {} - virtual std::shared_ptr newDiskWriter(const std::string& filename)=0; + virtual std::unique_ptr newDiskWriter + (const std::string& filename)=0; }; } // namespace aria2 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), - 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) +void DiskWriterEntry::setDiskWriter(std::unique_ptr 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 createDiskWriterEntry -(const std::shared_ptr& fileEntry, - bool needsFileAllocation) +std::unique_ptr createDiskWriterEntry +(const std::shared_ptr& fileEntry) { - std::shared_ptr entry(new DiskWriterEntry(fileEntry)); - entry->needsFileAllocation(needsFileAllocation); + auto entry = make_unique(fileEntry); + entry->needsFileAllocation(fileEntry->isRequested()); return entry; } } // namespace @@ -144,33 +144,28 @@ void MultiDiskAdaptor::resetDiskWriterEntries() return; } - for(std::vector >::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 dwreq; // TODO Currently, pieceLength_ == 0 is used for unit testing only. if(pieceLength_ > 0) { - std::vector >::const_iterator done = - diskWriterEntries_.begin(); - for(std::vector >::const_iterator itr = - diskWriterEntries_.begin(), eoi = diskWriterEntries_.end(); - itr != eoi;) { - const std::shared_ptr& 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 >::const_iterator i = - itr-1; true; --i) { - const std::shared_ptr& 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 >::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& 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 >::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 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 entry, int64_t offset) } // namespace namespace { -ssize_t calculateLength(const std::shared_ptr 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 entry, namespace { class OffsetCompare { public: - bool operator()(int64_t offset, const std::shared_ptr& dwe) + bool operator()(int64_t offset, const std::unique_ptr& 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(offset))); } @@ -353,8 +342,7 @@ DiskWriterEntries::const_iterator findFirstDiskWriterEntry } // namespace namespace { -void throwOnDiskWriterNotOpened(const std::shared_ptr& 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& 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& fent = (*dent)->getFileEntry(); + auto& fent = (*dent)->getFileEntry(); for(; i != eoi;) { if(std::max(fent->getOffset(), static_cast((*i)->goff + celloff)) < std::min(fent->getLastOffset(), static_cast((*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 >::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 MultiDiskAdaptor::fileAllocationIterator() +std::shared_ptr +MultiDiskAdaptor::fileAllocationIterator() { - return std::shared_ptr - (new MultiFileAllocationIterator(this)); + return std::make_shared(this); } void MultiDiskAdaptor::enableReadOnly() @@ -552,10 +528,8 @@ void MultiDiskAdaptor::disableReadOnly() void MultiDiskAdaptor::enableMmap() { - for(std::vector >::const_iterator i = - diskWriterEntries_.begin(), eoi = diskWriterEntries_.end(); - i != eoi; ++i) { - const std::shared_ptr& 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 >::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 >::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; } diff --git a/src/MultiDiskAdaptor.h b/src/MultiDiskAdaptor.h index f778f9d8..59c239cd 100644 --- a/src/MultiDiskAdaptor.h +++ b/src/MultiDiskAdaptor.h @@ -46,9 +46,10 @@ class DiskWriter; class DiskWriterEntry { private: std::shared_ptr fileEntry_; - std::shared_ptr diskWriter_; + std::unique_ptr diskWriter_; bool open_; bool needsFileAllocation_; + bool needsDiskWriter_; public: DiskWriterEntry(const std::shared_ptr& fileEntry); @@ -76,9 +77,9 @@ public: return fileEntry_; } - void setDiskWriter(const std::shared_ptr& diskWriter); + void setDiskWriter(std::unique_ptr diskWriter); - const std::shared_ptr& getDiskWriter() const + const std::unique_ptr& getDiskWriter() const { return diskWriter_; } @@ -95,9 +96,18 @@ public: needsFileAllocation_ = f; } + bool needsDiskWriter() const + { + return needsDiskWriter_; + } + + void needsDiskWriter(bool f) + { + needsDiskWriter_ = f; + } }; -typedef std::vector > DiskWriterEntries; +typedef std::vector> DiskWriterEntries; class MultiDiskAdaptor : public DiskAdaptor { friend class MultiFileAllocationIterator; @@ -105,7 +115,7 @@ private: int32_t pieceLength_; DiskWriterEntries diskWriterEntries_; - std::vector > openedDiskWriterEntries_; + std::vector openedDiskWriterEntries_; int maxOpenFiles_; @@ -113,8 +123,7 @@ private: void resetDiskWriterEntries(); - void openIfNot(const std::shared_ptr& entry, - void (DiskWriterEntry::*f)()); + void openIfNot(DiskWriterEntry* entry, void (DiskWriterEntry::*f)()); static const int DEFAULT_MAX_OPEN_FILES = 100; @@ -168,8 +177,7 @@ public: virtual size_t utime(const Time& actime, const Time& modtime); - const std::vector >& - getDiskWriterEntries() const + const DiskWriterEntries& getDiskWriterEntries() const { return diskWriterEntries_; } diff --git a/src/MultiFileAllocationIterator.cc b/src/MultiFileAllocationIterator.cc index b2561e10..1b234c0a 100644 --- a/src/MultiFileAllocationIterator.cc +++ b/src/MultiFileAllocationIterator.cc @@ -45,51 +45,49 @@ namespace aria2 { MultiFileAllocationIterator::MultiFileAllocationIterator -(MultiDiskAdaptor* diskAdaptor): - diskAdaptor_(diskAdaptor), - entries_(diskAdaptor_->diskWriterEntries_.begin(), - diskAdaptor_->diskWriterEntries_.end()) +(MultiDiskAdaptor* diskAdaptor) + : diskAdaptor_{diskAdaptor}, + entryItr_{std::begin(diskAdaptor_->getDiskWriterEntries())} {} MultiFileAllocationIterator::~MultiFileAllocationIterator() {} void MultiFileAllocationIterator::allocateChunk() { - while(!fileAllocationIterator_ || - fileAllocationIterator_->finished()) { - if(entries_.empty()) { - break; - } - std::shared_ptr entry = entries_.front(); - entries_.pop_front(); - std::shared_ptr fileEntry = entry->getFileEntry(); - // Open file before calling DiskWriterEntry::size() - diskAdaptor_->openIfNot(entry, &DiskWriterEntry::openFile); - if(entry->needsFileAllocation() && entry->size() < fileEntry->getLength()) { - // Calling private function of MultiDiskAdaptor. + while((!fileAllocationIterator_ || + fileAllocationIterator_->finished()) && + entryItr_ != std::end(diskAdaptor_->getDiskWriterEntries())) { + auto& fileEntry = (*entryItr_)->getFileEntry(); + // Open file before calling DiskWriterEntry::size(). Calling + // private function of MultiDiskAdaptor. + diskAdaptor_->openIfNot((*entryItr_).get(), &DiskWriterEntry::openFile); + if((*entryItr_)->needsFileAllocation() && + (*entryItr_)->size() < fileEntry->getLength()) { switch(diskAdaptor_->getFileAllocationMethod()) { #ifdef HAVE_SOME_FALLOCATE case(DiskAdaptor::FILE_ALLOC_FALLOC): - fileAllocationIterator_.reset - (new FallocFileAllocationIterator(entry->getDiskWriter().get(), - entry->size(), - fileEntry->getLength())); + fileAllocationIterator_ = make_unique + ((*entryItr_)->getDiskWriter().get(), + (*entryItr_)->size(), + fileEntry->getLength()); break; #endif // HAVE_SOME_FALLOCATE case(DiskAdaptor::FILE_ALLOC_TRUNC): - fileAllocationIterator_.reset - (new TruncFileAllocationIterator(entry->getDiskWriter().get(), - entry->size(), - fileEntry->getLength())); + fileAllocationIterator_ = make_unique + ((*entryItr_)->getDiskWriter().get(), + (*entryItr_)->size(), + fileEntry->getLength()); break; default: - fileAllocationIterator_.reset - (new AdaptiveFileAllocationIterator(entry->getDiskWriter().get(), - entry->size(), - fileEntry->getLength())); + fileAllocationIterator_ = make_unique + ((*entryItr_)->getDiskWriter().get(), + (*entryItr_)->size(), + fileEntry->getLength()); break; } + break; } + ++entryItr_; } if(finished()) { return; @@ -99,7 +97,7 @@ void MultiFileAllocationIterator::allocateChunk() bool MultiFileAllocationIterator::finished() { - return entries_.empty() && + return entryItr_ == std::end(diskAdaptor_->getDiskWriterEntries()) && (!fileAllocationIterator_ || fileAllocationIterator_->finished()); } @@ -121,10 +119,10 @@ int64_t MultiFileAllocationIterator::getTotalLength() } } -const std::deque >& +const DiskWriterEntries& MultiFileAllocationIterator::getDiskWriterEntries() const { - return entries_; + return diskAdaptor_->getDiskWriterEntries(); } } // namespace aria2 diff --git a/src/MultiFileAllocationIterator.h b/src/MultiFileAllocationIterator.h index 388b891f..2f55d234 100644 --- a/src/MultiFileAllocationIterator.h +++ b/src/MultiFileAllocationIterator.h @@ -40,16 +40,17 @@ #include #include +#include "MultiDiskAdaptor.h" + namespace aria2 { -class MultiDiskAdaptor; class DiskWriterEntry; class MultiFileAllocationIterator:public FileAllocationIterator { private: MultiDiskAdaptor* diskAdaptor_; - std::deque > entries_; + DiskWriterEntries::const_iterator entryItr_; std::unique_ptr fileAllocationIterator_; public: MultiFileAllocationIterator(MultiDiskAdaptor* diskAdaptor); @@ -64,8 +65,7 @@ public: virtual int64_t getTotalLength(); - const std::deque >& - getDiskWriterEntries() const; + const DiskWriterEntries& getDiskWriterEntries() const; }; } // namespace aria2 diff --git a/src/UnknownLengthPieceStorage.cc b/src/UnknownLengthPieceStorage.cc index 00d324c9..dcacccb2 100644 --- a/src/UnknownLengthPieceStorage.cc +++ b/src/UnknownLengthPieceStorage.cc @@ -62,9 +62,8 @@ void UnknownLengthPieceStorage::initStorage() directDiskAdaptor->setFileEntries(downloadContext_->getFileEntries().begin(), downloadContext_->getFileEntries().end()); - std::shared_ptr writer = - diskWriterFactory_->newDiskWriter(directDiskAdaptor->getFilePath()); - directDiskAdaptor->setDiskWriter(writer); + directDiskAdaptor->setDiskWriter + (diskWriterFactory_->newDiskWriter(directDiskAdaptor->getFilePath())); diskAdaptor_.reset(directDiskAdaptor); } diff --git a/test/MultiDiskAdaptorTest.cc b/test/MultiDiskAdaptorTest.cc index a35f105d..001c9a54 100644 --- a/test/MultiDiskAdaptorTest.cc +++ b/test/MultiDiskAdaptorTest.cc @@ -28,10 +28,10 @@ class MultiDiskAdaptorTest:public CppUnit::TestFixture { CPPUNIT_TEST(testWriteCache); CPPUNIT_TEST_SUITE_END(); private: - std::shared_ptr adaptor; + std::unique_ptr adaptor; public: void setUp() { - adaptor.reset(new MultiDiskAdaptor()); + adaptor = make_unique(); adaptor->setPieceLength(2); } @@ -81,13 +81,12 @@ std::vector > createEntries() void MultiDiskAdaptorTest::testResetDiskWriterEntries() { { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); // In openFile(), resetDiskWriterEntries() are called. adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); CPPUNIT_ASSERT(entries[0]->getDiskWriter()); CPPUNIT_ASSERT(entries[1]->getDiskWriter()); CPPUNIT_ASSERT(entries[2]->getDiskWriter()); @@ -98,14 +97,13 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() adaptor->closeFile(); } { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); fileEntries[0]->setRequested(false); adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); // In openFile(), resetDiskWriterEntries() are called. adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); // Because entries[1] spans entries[0] CPPUNIT_ASSERT(entries[0]->getDiskWriter()); CPPUNIT_ASSERT(entries[1]->getDiskWriter()); @@ -117,15 +115,14 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() adaptor->closeFile(); } { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); fileEntries[0]->setRequested(false); fileEntries[1]->setRequested(false); adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); // In openFile(), resetDiskWriterEntries() are called. adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); CPPUNIT_ASSERT(!entries[0]->getDiskWriter()); // Because entries[2] spans entries[1] CPPUNIT_ASSERT(entries[1]->getDiskWriter()); @@ -138,14 +135,13 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() adaptor->closeFile(); } { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); fileEntries[3]->setRequested(false); adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); // In openFile(), resetDiskWriterEntries() are called. adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); CPPUNIT_ASSERT(entries[0]->getDiskWriter()); CPPUNIT_ASSERT(entries[1]->getDiskWriter()); CPPUNIT_ASSERT(entries[2]->getDiskWriter()); @@ -158,14 +154,13 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() adaptor->closeFile(); } { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); fileEntries[4]->setRequested(false); adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); // In openFile(), resetDiskWriterEntries() are called. adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); CPPUNIT_ASSERT(entries[0]->getDiskWriter()); CPPUNIT_ASSERT(entries[1]->getDiskWriter()); CPPUNIT_ASSERT(entries[2]->getDiskWriter()); @@ -177,15 +172,14 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() adaptor->closeFile(); } { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); fileEntries[3]->setRequested(false); fileEntries[4]->setRequested(false); adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); // In openFile(), resetDiskWriterEntries() are called. adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); CPPUNIT_ASSERT(entries[0]->getDiskWriter()); CPPUNIT_ASSERT(entries[1]->getDiskWriter()); CPPUNIT_ASSERT(entries[2]->getDiskWriter()); @@ -196,7 +190,7 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() adaptor->closeFile(); } { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); for(size_t i = 5; i < 9; ++i) { fileEntries[i]->setRequested(false); } @@ -204,8 +198,7 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() // In openFile(), resetDiskWriterEntries() are called. adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); CPPUNIT_ASSERT(entries[0]->getDiskWriter()); CPPUNIT_ASSERT(entries[1]->getDiskWriter()); CPPUNIT_ASSERT(entries[2]->getDiskWriter()); @@ -216,14 +209,13 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() adaptor->closeFile(); } { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); for(size_t i = 1; i < 9; ++i) { fileEntries[i]->setRequested(false); } adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); CPPUNIT_ASSERT(entries[0]->getDiskWriter()); CPPUNIT_ASSERT(!entries[1]->getDiskWriter()); CPPUNIT_ASSERT(!entries[2]->getDiskWriter()); @@ -234,14 +226,13 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() adaptor->closeFile(); } { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); for(size_t i = 2; i < 9; ++i) { fileEntries[i]->setRequested(false); } adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); CPPUNIT_ASSERT(entries[0]->getDiskWriter()); CPPUNIT_ASSERT(entries[1]->getDiskWriter()); // entries[1] spans entries[2] @@ -254,15 +245,14 @@ void MultiDiskAdaptorTest::testResetDiskWriterEntries() adaptor->closeFile(); } { - std::vector > fileEntries = createEntries(); + auto fileEntries = createEntries(); for(size_t i = 0; i < 6; ++i) { fileEntries[i]->setRequested(false); } fileEntries[8]->setRequested(false); adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); adaptor->openFile(); - std::vector > entries = - adaptor->getDiskWriterEntries(); + auto& entries = adaptor->getDiskWriterEntries(); CPPUNIT_ASSERT(!entries[0]->getDiskWriter()); CPPUNIT_ASSERT(!entries[1]->getDiskWriter()); CPPUNIT_ASSERT(!entries[2]->getDiskWriter()); @@ -291,8 +281,8 @@ void readFile(const std::string& filename, char* buf, int bufLength) { } void MultiDiskAdaptorTest::testWriteData() { - std::vector > fileEntries(createEntries()); - adaptor->setFileEntries(fileEntries.begin(), fileEntries.end()); + auto fileEntries = createEntries(); + adaptor->setFileEntries(std::begin(fileEntries), std::end(fileEntries)); adaptor->openFile(); std::string msg = "12345"; @@ -340,13 +330,13 @@ void MultiDiskAdaptorTest::testWriteData() { void MultiDiskAdaptorTest::testReadData() { - std::vector> entries { + auto entries = std::vector>{ std::make_shared(A2_TEST_DIR "/file1r.txt", 15, 0), std::make_shared(A2_TEST_DIR "/file2r.txt", 7, 15), std::make_shared(A2_TEST_DIR "/file3r.txt", 3, 22) }; - adaptor->setFileEntries(entries.begin(), entries.end()); + adaptor->setFileEntries(std::begin(entries), std::end(entries)); adaptor->enableReadOnly(); adaptor->openFile(); unsigned char buf[128]; @@ -369,7 +359,7 @@ void MultiDiskAdaptorTest::testCutTrailingGarbage() { std::string dir = A2_TEST_OUT_DIR; std::string prefix = "aria2_MultiDiskAdaptorTest_testCutTrailingGarbage_"; - std::vector > fileEntries { + auto fileEntries = std::vector>{ std::make_shared(dir+"/"+prefix+"1", 256, 0), std::make_shared(dir+"/"+prefix+"2", 512, 256) }; @@ -378,7 +368,7 @@ void MultiDiskAdaptorTest::testCutTrailingGarbage() } MultiDiskAdaptor adaptor; - adaptor.setFileEntries(fileEntries.begin(), fileEntries.end()); + adaptor.setFileEntries(std::begin(fileEntries), std::end(fileEntries)); adaptor.setMaxOpenFiles(1); adaptor.setPieceLength(1); @@ -396,7 +386,7 @@ void MultiDiskAdaptorTest::testSize() { std::string dir = A2_TEST_OUT_DIR; std::string prefix = "aria2_MultiDiskAdaptorTest_testSize_"; - std::vector> fileEntries { + auto fileEntries = std::vector>{ std::make_shared(dir+"/"+prefix+"1", 1, 0), std::make_shared(dir+"/"+prefix+"2", 1, 1) }; @@ -405,7 +395,7 @@ void MultiDiskAdaptorTest::testSize() } MultiDiskAdaptor adaptor; - adaptor.setFileEntries(fileEntries.begin(), fileEntries.end()); + adaptor.setFileEntries(std::begin(fileEntries), std::end(fileEntries)); adaptor.setMaxOpenFiles(1); adaptor.setPieceLength(1); @@ -417,7 +407,7 @@ void MultiDiskAdaptorTest::testSize() void MultiDiskAdaptorTest::testUtime() { std::string storeDir = A2_TEST_OUT_DIR"/aria2_MultiDiskAdaptorTest_testUtime"; - std::vector > entries { + auto entries = std::vector>{ std::make_shared(storeDir+"/requested", 0, 0), std::make_shared(storeDir+"/notFound", 0, 0), std::make_shared(storeDir+"/notRequested", 0, 0), @@ -432,7 +422,7 @@ void MultiDiskAdaptorTest::testUtime() entries[2]->setRequested(false); MultiDiskAdaptor adaptor; - adaptor.setFileEntries(entries.begin(), entries.end()); + adaptor.setFileEntries(std::begin(entries), std::end(entries)); time_t atime = (time_t) 100000; time_t mtime = (time_t) 200000; @@ -453,16 +443,16 @@ void MultiDiskAdaptorTest::testWriteCache() { std::string storeDir = A2_TEST_OUT_DIR"/aria2_MultiDiskAdaptorTest_testWriteCache"; - std::vector> entries { + auto entries = std::vector>{ std::make_shared(storeDir+"/file1", 16385, 0), std::make_shared(storeDir+"/file2", 4098, 16385) }; for(const auto& i : entries) { File(i->getPath()).remove(); } - std::shared_ptr adaptor(new MultiDiskAdaptor()); + auto adaptor = std::make_shared(); adaptor->setFileEntries(std::begin(entries), std::end(entries)); - WrDiskCacheEntry cache(adaptor); + WrDiskCacheEntry cache{adaptor}; std::string data1(16383, '1'), data2(100, '2'), data3(4000, '3'); cache.cacheData(createDataCell(0, data1.c_str())); cache.cacheData(createDataCell(data1.size(), data2.c_str())); diff --git a/test/MultiFileAllocationIteratorTest.cc b/test/MultiFileAllocationIteratorTest.cc index e061fab5..c1507ede 100644 --- a/test/MultiFileAllocationIteratorTest.cc +++ b/test/MultiFileAllocationIteratorTest.cc @@ -38,18 +38,18 @@ void MultiFileAllocationIteratorTest::testMakeDiskWriterEntries() std::string storeDir = A2_TEST_OUT_DIR"/aria2_MultiFileAllocationIteratorTest" "_testMakeDiskWriterEntries"; - std::shared_ptr fs[] = { - std::shared_ptr(new FileEntry(storeDir+"/file1", 1536, 0)), - std::shared_ptr(new FileEntry(storeDir+"/file2", 2048, 1536)),// req no - std::shared_ptr(new FileEntry(storeDir+"/file3", 1024, 3584)), - std::shared_ptr(new FileEntry(storeDir+"/file4", 1024, 4608)),// req no - std::shared_ptr(new FileEntry(storeDir+"/file5", 1024, 5632)),// req no - std::shared_ptr(new FileEntry(storeDir+"/file6", 1024, 6656)),// req no - std::shared_ptr(new FileEntry(storeDir+"/file7", 256, 7680)),// req no - std::shared_ptr(new FileEntry(storeDir+"/file8", 255, 7936)), - std::shared_ptr(new FileEntry(storeDir+"/file9", 1025, 8191)),// req no - std::shared_ptr(new FileEntry(storeDir+"/fileA", 1024, 9216)),// req no - std::shared_ptr(new FileEntry(storeDir+"/fileB", 1024, 10240)), + auto fs = std::vector>{ + std::make_shared(storeDir+"/file1", 1536, 0), + std::make_shared(storeDir+"/file2", 2048, 1536),// req no + std::make_shared(storeDir+"/file3", 1024, 3584), + std::make_shared(storeDir+"/file4", 1024, 4608),// req no + std::make_shared(storeDir+"/file5", 1024, 5632),// req no + std::make_shared(storeDir+"/file6", 1024, 6656),// req no + std::make_shared(storeDir+"/file7", 256, 7680),// req no + std::make_shared(storeDir+"/file8", 255, 7936), + std::make_shared(storeDir+"/file9", 1025, 8191),// req no + std::make_shared(storeDir+"/fileA", 1024, 9216),// req no + std::make_shared(storeDir+"/fileB", 1024, 10240) }; fs[1]->setRequested(false); // file2 fs[3]->setRequested(false); // file4 @@ -59,21 +59,22 @@ void MultiFileAllocationIteratorTest::testMakeDiskWriterEntries() fs[8]->setRequested(false); // file9 fs[9]->setRequested(false); // fileA + for(auto& fe : fs) { + File{fe->getPath()}.remove(); + } // create empty file4 createFile(storeDir+std::string("/file4"), 0); - std::shared_ptr diskAdaptor(new MultiDiskAdaptor()); - diskAdaptor->setFileEntries(std::begin(fs), std::end(fs)); - diskAdaptor->setPieceLength(1024); - diskAdaptor->openFile(); + MultiDiskAdaptor diskAdaptor; + diskAdaptor.setFileEntries(std::begin(fs), std::end(fs)); + diskAdaptor.setPieceLength(1024); + diskAdaptor.openFile(); - auto itr = std::dynamic_pointer_cast - (diskAdaptor->fileAllocationIterator()); - - const std::deque >& entries = - itr->getDiskWriterEntries(); + auto allocitr = diskAdaptor.fileAllocationIterator(); + auto itr = dynamic_cast(allocitr.get()); + auto& entries = itr->getDiskWriterEntries(); CPPUNIT_ASSERT_EQUAL((size_t)11, entries.size()); // file1 @@ -152,58 +153,49 @@ void MultiFileAllocationIteratorTest::testAllocate() int64_t length6 = 30; try { - std::shared_ptr diskAdaptor(new MultiDiskAdaptor()); - diskAdaptor->setPieceLength(1); + MultiDiskAdaptor diskAdaptor; + diskAdaptor.setPieceLength(1); int64_t offset = 0; - std::shared_ptr fileEntry1(new FileEntry(storeDir+"/"+fname1, - length1, - offset)); + auto fileEntry1 = std::make_shared(storeDir+"/"+fname1, + length1, offset); offset += length1; - std::shared_ptr fileEntry2(new FileEntry(storeDir+"/"+fname2, - length2, - offset)); - + auto fileEntry2 = std::make_shared(storeDir+"/"+fname2, + length2, offset); offset += length2; - std::shared_ptr fileEntry3(new FileEntry(storeDir+"/"+fname3, - length3, - offset)); + auto fileEntry3 = std::make_shared(storeDir+"/"+fname3, + length3, offset); offset += length3; - std::shared_ptr fileEntry4(new FileEntry(storeDir+"/"+fname4, - length4, - offset)); + auto fileEntry4 = std::make_shared(storeDir+"/"+fname4, + length4, offset); fileEntry4->setRequested(false); - offset += length4; - std::shared_ptr fileEntry5(new FileEntry(storeDir+"/"+fname5, - length5, - offset)); - + auto fileEntry5 = std::make_shared(storeDir+"/"+fname5, + length5, offset); offset += length5; - std::shared_ptr fileEntry6(new FileEntry(storeDir+"/"+fname6, - length6, - offset)); + auto fileEntry6 = std::make_shared(storeDir+"/"+fname6, + length6, offset); fileEntry6->setRequested(false); - std::vector > fs; - fs.push_back(fileEntry1); - fs.push_back(fileEntry2); - fs.push_back(fileEntry3); - fs.push_back(fileEntry4); - fs.push_back(fileEntry5); - fs.push_back(fileEntry6); - diskAdaptor->setFileEntries(fs.begin(), fs.end()); + auto fs = std::vector>{ + fileEntry1, + fileEntry2, + fileEntry3, + fileEntry4, + fileEntry5, + fileEntry6 + }; + diskAdaptor.setFileEntries(std::begin(fs), std::end(fs)); - for(std::vector >::const_iterator i = fs.begin(); - i != fs.end(); ++i) { - File((*i)->getPath()).remove(); + for(auto& fe : fs) { + File{fe->getPath()}.remove(); } // we have to open file first. - diskAdaptor->initAndOpenFile(); - auto itr = std::dynamic_pointer_cast - (diskAdaptor->fileAllocationIterator()); + diskAdaptor.initAndOpenFile(); + auto allocitr = diskAdaptor.fileAllocationIterator(); + auto itr = dynamic_cast(allocitr.get()); while(!itr->finished()) { itr->allocateChunk(); }