From cd13647abe5210735d08bb5c99247d9c544c902f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 17 Jul 2010 14:36:18 +0000 Subject: [PATCH] 2010-07-17 Tatsuhiro Tsujikawa Pass maxSplitSize as an argument, instead of member variable of DefaultPieceStorage. SegmentMan::getSegment(cuid_t,size_t index) was renamed to SegmentMan::getSegmentWithIndex(...) * src/AbstractCommand.cc * src/AbstractCommand.h * src/DefaultPieceStorage.cc * src/DefaultPieceStorage.h * src/DownloadCommand.cc * src/FtpNegotiationCommand.cc * src/HttpResponseCommand.cc * src/PieceStorage.h * src/RequestGroup.cc * src/SegmentMan.cc * src/SegmentMan.h * src/UnknownLengthPieceStorage.cc * src/UnknownLengthPieceStorage.h * test/MockPieceStorage.h * test/SegmentManTest.cc --- ChangeLog | 21 ++++++++++++ src/AbstractCommand.cc | 56 +++++++++++++++++++------------- src/AbstractCommand.h | 2 ++ src/DefaultPieceStorage.cc | 7 ++-- src/DefaultPieceStorage.h | 9 +---- src/DownloadCommand.cc | 2 +- src/FtpNegotiationCommand.cc | 4 +-- src/HttpResponseCommand.cc | 5 +-- src/PieceStorage.h | 2 +- src/RequestGroup.cc | 1 - src/SegmentMan.cc | 15 ++++++--- src/SegmentMan.h | 5 +-- src/UnknownLengthPieceStorage.cc | 4 +-- src/UnknownLengthPieceStorage.h | 2 +- test/MockPieceStorage.h | 2 +- test/SegmentManTest.cc | 42 ++++++++++++------------ 16 files changed, 106 insertions(+), 73 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2c06433a..d9fda9da 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,24 @@ +2010-07-17 Tatsuhiro Tsujikawa + + Pass maxSplitSize as an argument, instead of member variable of + DefaultPieceStorage. SegmentMan::getSegment(cuid_t,size_t index) + was renamed to SegmentMan::getSegmentWithIndex(...) + * src/AbstractCommand.cc + * src/AbstractCommand.h + * src/DefaultPieceStorage.cc + * src/DefaultPieceStorage.h + * src/DownloadCommand.cc + * src/FtpNegotiationCommand.cc + * src/HttpResponseCommand.cc + * src/PieceStorage.h + * src/RequestGroup.cc + * src/SegmentMan.cc + * src/SegmentMan.h + * src/UnknownLengthPieceStorage.cc + * src/UnknownLengthPieceStorage.h + * test/MockPieceStorage.h + * test/SegmentManTest.cc + 2010-07-17 Tatsuhiro Tsujikawa Removed prepareForRetry from CreateRequestCommand diff --git a/src/AbstractCommand.cc b/src/AbstractCommand.cc index 29e9beb5..9438d8a9 100644 --- a/src/AbstractCommand.cc +++ b/src/AbstractCommand.cc @@ -153,27 +153,27 @@ bool AbstractCommand::execute() { } return prepareForRetry(0); } - } - // TODO it is not needed to check other PeerStats every time. - // Find faster Request when no segment is available. - if(!req_.isNull() && fileEntry_->countPooledRequest() > 0 && - !getPieceStorage()->hasMissingUnusedPiece()) { - SharedHandle fasterRequest = fileEntry_->findFasterRequest(req_); - if(!fasterRequest.isNull()) { - if(getLogger()->info()) { - getLogger()->info("CUID#%s - Use faster Request hostname=%s, port=%u", - util::itos(getCuid()).c_str(), - fasterRequest->getHost().c_str(), - fasterRequest->getPort()); + // TODO it is not needed to check other PeerStats every time. + // Find faster Request when no segment is available. + if(!req_.isNull() && fileEntry_->countPooledRequest() > 0 && + !getPieceStorage()->hasMissingUnusedPiece()) { + SharedHandle fasterRequest = fileEntry_->findFasterRequest(req_); + if(!fasterRequest.isNull()) { + if(getLogger()->info()) { + getLogger()->info("CUID#%s - Use faster Request hostname=%s, port=%u", + util::itos(getCuid()).c_str(), + fasterRequest->getHost().c_str(), + fasterRequest->getPort()); + } + // Cancel current Request object and use faster one. + fileEntry_->removeRequest(req_); + Command* command = + InitiateConnectionCommandFactory::createInitiateConnectionCommand + (getCuid(), fasterRequest, fileEntry_, requestGroup_, e_); + e_->setNoWait(true); + e_->addCommand(command); + return true; } - // Cancel current Request object and use faster one. - fileEntry_->removeRequest(req_); - Command* command = - InitiateConnectionCommandFactory::createInitiateConnectionCommand - (getCuid(), fasterRequest, fileEntry_, requestGroup_, e_); - e_->setNoWait(true); - e_->addCommand(command); - return true; } } if((checkSocketIsReadable_ && readEventEnabled()) || @@ -192,9 +192,10 @@ bool AbstractCommand::execute() { // is more efficient. getDownloadContext()->getFileEntries().size() == 1) { size_t maxSegments = req_.isNull()?1:req_->getMaxPipelinedRequest(); + size_t minSplitSize = calculateMinSplitSize(); while(segments_.size() < maxSegments) { SharedHandle segment = - getSegmentMan()->getSegment(getCuid()); + getSegmentMan()->getSegment(getCuid(), minSplitSize); if(segment.isNull()) { break; } else { @@ -221,10 +222,12 @@ bool AbstractCommand::execute() { } } } else { + // For multi-file downloads + size_t minSplitSize = calculateMinSplitSize(); size_t maxSegments = req_->getMaxPipelinedRequest(); if(segments_.size() < maxSegments) { getSegmentMan()->getSegment - (segments_, getCuid(), fileEntry_, maxSegments); + (segments_, getCuid(), minSplitSize, fileEntry_, maxSegments); } if(segments_.empty()) { return prepareForRetry(0); @@ -837,4 +840,13 @@ void AbstractCommand::createSocket() socket_.reset(new SocketCore()); } +size_t AbstractCommand::calculateMinSplitSize() const +{ + if(!req_.isNull() && req_->isPipeliningEnabled()) { + return getDownloadContext()->getPieceLength(); + } else { + return getOption()->getAsInt(PREF_MIN_SPLIT_SIZE); + } +} + } // namespace aria2 diff --git a/src/AbstractCommand.h b/src/AbstractCommand.h index 8dd5a15b..75d50d57 100644 --- a/src/AbstractCommand.h +++ b/src/AbstractCommand.h @@ -77,6 +77,8 @@ private: bool incNumConnection_; + size_t calculateMinSplitSize() const; + #ifdef ENABLE_ASYNC_DNS void setNameResolverCheck(const SharedHandle& resolver); diff --git a/src/DefaultPieceStorage.cc b/src/DefaultPieceStorage.cc index 29aa36d6..5801ead6 100644 --- a/src/DefaultPieceStorage.cc +++ b/src/DefaultPieceStorage.cc @@ -75,8 +75,7 @@ DefaultPieceStorage::DefaultPieceStorage logger_(LogFactory::getInstance()), option_(option), pieceStatMan_(new PieceStatMan(downloadContext->getNumPieces(), true)), - pieceSelector_(new RarestPieceSelector(pieceStatMan_)), - minSplitSize_(1024*1024) + pieceSelector_(new RarestPieceSelector(pieceStatMan_)) {} DefaultPieceStorage::~DefaultPieceStorage() { @@ -273,11 +272,11 @@ bool DefaultPieceStorage::hasMissingUnusedPiece() } SharedHandle DefaultPieceStorage::getSparseMissingUnusedPiece -(const unsigned char* ignoreBitfield, size_t length) +(size_t minSplitSize, const unsigned char* ignoreBitfield, size_t length) { size_t index; if(bitfieldMan_->getSparseMissingUnusedIndex - (index, minSplitSize_, ignoreBitfield, length)) { + (index, minSplitSize, ignoreBitfield, length)) { return checkOutPiece(index); } else { return SharedHandle(); diff --git a/src/DefaultPieceStorage.h b/src/DefaultPieceStorage.h index dce56c5f..879f13c7 100644 --- a/src/DefaultPieceStorage.h +++ b/src/DefaultPieceStorage.h @@ -87,8 +87,6 @@ private: SharedHandle pieceSelector_; - size_t minSplitSize_; - bool getMissingPieceIndex(size_t& index, const unsigned char* bitfield, size_t length); @@ -140,7 +138,7 @@ public: virtual bool hasMissingUnusedPiece(); virtual SharedHandle getSparseMissingUnusedPiece - (const unsigned char* ignoreBitfield, size_t length); + (size_t minSplitSize, const unsigned char* ignoreBitfield, size_t length); virtual SharedHandle getMissingPiece(size_t index); @@ -253,11 +251,6 @@ public: { return pieceSelector_; } - - void setMinSplitSize(size_t minSplitSize) - { - minSplitSize_ = minSplitSize; - } }; typedef SharedHandle DefaultPieceStorageHandle; diff --git a/src/DownloadCommand.cc b/src/DownloadCommand.cc index 4b08183c..11c0dc25 100644 --- a/src/DownloadCommand.cc +++ b/src/DownloadCommand.cc @@ -341,7 +341,7 @@ bool DownloadCommand::prepareForNextSegment() { (tempSegment->getPosition()+tempSegment->getLength())) { return prepareForRetry(0); } - SharedHandle nextSegment = getSegmentMan()->getSegment + SharedHandle nextSegment = getSegmentMan()->getSegmentWithIndex (getCuid(), tempSegment->getIndex()+1); if(nextSegment.isNull()) { nextSegment = getSegmentMan()->getCleanSegmentIfOwnerIsIdle diff --git a/src/FtpNegotiationCommand.cc b/src/FtpNegotiationCommand.cc index 7a43b273..2c070274 100644 --- a/src/FtpNegotiationCommand.cc +++ b/src/FtpNegotiationCommand.cc @@ -428,7 +428,7 @@ bool FtpNegotiationCommand::onFileSizeDetermined(uint64_t totalLength) // We have to make sure that command that has Request object must // have segment after PieceStorage is initialized. See // AbstractCommand::execute() - getSegmentMan()->getSegment(getCuid(), 0); + getSegmentMan()->getSegmentWithIndex(getCuid(), 0); return true; } else { getRequestGroup()->adjustFilename @@ -466,7 +466,7 @@ bool FtpNegotiationCommand::onFileSizeDetermined(uint64_t totalLength) // We have to make sure that command that has Request object must // have segment after PieceStorage is initialized. See // AbstractCommand::execute() - getSegmentMan()->getSegment(getCuid(), 0); + getSegmentMan()->getSegmentWithIndex(getCuid(), 0); prepareForNextAction(this); diff --git a/src/HttpResponseCommand.cc b/src/HttpResponseCommand.cc index 3a24a97d..a1c8c2aa 100644 --- a/src/HttpResponseCommand.cc +++ b/src/HttpResponseCommand.cc @@ -263,7 +263,8 @@ bool HttpResponseCommand::handleDefaultEncoding // We have to make sure that command that has Request object must // have segment after PieceStorage is initialized. See // AbstractCommand::execute() - SharedHandle segment = getSegmentMan()->getSegment(getCuid(), 0); + SharedHandle segment = + getSegmentMan()->getSegmentWithIndex(getCuid(), 0); // pipelining requires implicit range specified. But the request for // this response most likely dones't contains range header. This means // we can't continue to use this socket because server sends all entity @@ -365,7 +366,7 @@ bool HttpResponseCommand::handleOtherEncoding // We have to make sure that command that has Request object must // have segment after PieceStorage is initialized. See // AbstractCommand::execute() - getSegmentMan()->getSegment(getCuid(), 0); + getSegmentMan()->getSegmentWithIndex(getCuid(), 0); getDownloadEngine()->addCommand (createHttpDownloadCommand(httpResponse, diff --git a/src/PieceStorage.h b/src/PieceStorage.h index cc14bed7..02167432 100644 --- a/src/PieceStorage.h +++ b/src/PieceStorage.h @@ -108,7 +108,7 @@ public: * If ignoreBitfield is set, indexes of true bit are excluded. */ virtual SharedHandle getSparseMissingUnusedPiece - (const unsigned char* ignoreBitfield, size_t length) = 0; + (size_t minSplitSize, const unsigned char* ignoreBitfield, size_t length) = 0; /** * Returns a missing piece whose index is index. diff --git a/src/RequestGroup.cc b/src/RequestGroup.cc index da3e677c..42d65bd0 100644 --- a/src/RequestGroup.cc +++ b/src/RequestGroup.cc @@ -535,7 +535,6 @@ void RequestGroup::initPieceStorage() if(!diskWriterFactory_.isNull()) { ps->setDiskWriterFactory(diskWriterFactory_); } - ps->setMinSplitSize(option_->getAsInt(PREF_MIN_SPLIT_SIZE)); tempPieceStorage = ps; } else { UnknownLengthPieceStorageHandle ps diff --git a/src/SegmentMan.cc b/src/SegmentMan.cc index ea790bcf..82dedce5 100644 --- a/src/SegmentMan.cc +++ b/src/SegmentMan.cc @@ -165,16 +165,19 @@ void SegmentMan::getInFlightSegment } } -SharedHandle SegmentMan::getSegment(cuid_t cuid) { +SharedHandle SegmentMan::getSegment(cuid_t cuid, size_t minSplitSize) +{ SharedHandle piece = pieceStorage_->getSparseMissingUnusedPiece - (ignoreBitfield_.getFilterBitfield(),ignoreBitfield_.getBitfieldLength()); + (minSplitSize, + ignoreBitfield_.getFilterBitfield(), ignoreBitfield_.getBitfieldLength()); return checkoutSegment(cuid, piece); } void SegmentMan::getSegment (std::vector >& segments, cuid_t cuid, + size_t minSplitSize, const SharedHandle& fileEntry, size_t maxSegments) { @@ -186,7 +189,8 @@ void SegmentMan::getSegment SharedHandle segment = checkoutSegment(cuid, pieceStorage_->getSparseMissingUnusedPiece - (filter.getFilterBitfield(), filter.getBitfieldLength())); + (minSplitSize, + filter.getFilterBitfield(), filter.getBitfieldLength())); if(segment.isNull()) { break; } @@ -203,7 +207,8 @@ void SegmentMan::getSegment } } -SharedHandle SegmentMan::getSegment(cuid_t cuid, size_t index) { +SharedHandle SegmentMan::getSegmentWithIndex +(cuid_t cuid, size_t index) { if(index > 0 && downloadContext_->getNumPieces() <= index) { return SharedHandle(); } @@ -230,7 +235,7 @@ SharedHandle SegmentMan::getCleanSegmentIfOwnerIsIdle SharedHandle ps = getPeerStat(owner); if(ps.isNull() || (!ps.isNull() && ps->getStatus() == PeerStat::IDLE)) { cancelSegment(owner); - return getSegment(cuid, index); + return getSegmentWithIndex(cuid, index); } else { return SharedHandle(); } diff --git a/src/SegmentMan.h b/src/SegmentMan.h index 0cdaa349..50fcf8b3 100644 --- a/src/SegmentMan.h +++ b/src/SegmentMan.h @@ -138,12 +138,13 @@ public: void getInFlightSegment(std::vector >& segments, cuid_t cuid); - SharedHandle getSegment(cuid_t cuid); + SharedHandle getSegment(cuid_t cuid, size_t minSplitSize); // Checkouts segments in the range of fileEntry and push back to // segments until segments.size() < maxSegments holds false void getSegment(std::vector >& segments, cuid_t cuid, + size_t minSplitSize, const SharedHandle& fileEntry, size_t maxSegments); @@ -153,7 +154,7 @@ public: * to another cuid or has been downloaded, then returns a segment instance * whose isNull call is true. */ - SharedHandle getSegment(cuid_t cuid, size_t index); + SharedHandle getSegmentWithIndex(cuid_t cuid, size_t index); // Returns a currently used segment whose index is index and written // length is 0. The current owner(in idle state) of segment cancels diff --git a/src/UnknownLengthPieceStorage.cc b/src/UnknownLengthPieceStorage.cc index 678c6942..456772b6 100644 --- a/src/UnknownLengthPieceStorage.cc +++ b/src/UnknownLengthPieceStorage.cc @@ -108,7 +108,7 @@ bool UnknownLengthPieceStorage::hasMissingUnusedPiece() } SharedHandle UnknownLengthPieceStorage::getSparseMissingUnusedPiece -(const unsigned char* ignoreBitfield, size_t length) +(size_t minSplitSize, const unsigned char* ignoreBitfield, size_t length) { if(downloadFinished_) { return SharedHandle(); @@ -124,7 +124,7 @@ SharedHandle UnknownLengthPieceStorage::getSparseMissingUnusedPiece SharedHandle UnknownLengthPieceStorage::getMissingPiece(size_t index) { if(index == 0) { - return getSparseMissingUnusedPiece(0, 0); + return getSparseMissingUnusedPiece(0, 0, 0); } else { return SharedHandle(); } diff --git a/src/UnknownLengthPieceStorage.h b/src/UnknownLengthPieceStorage.h index d90f5644..01bd94e6 100644 --- a/src/UnknownLengthPieceStorage.h +++ b/src/UnknownLengthPieceStorage.h @@ -104,7 +104,7 @@ public: * Returns a missing piece if available. Otherwise returns 0; */ virtual SharedHandle getSparseMissingUnusedPiece - (const unsigned char* ignoreBitfield, size_t length); + (size_t minSplitSize, const unsigned char* ignoreBitfield, size_t length); /** * Returns a missing piece whose index is index. diff --git a/test/MockPieceStorage.h b/test/MockPieceStorage.h index 16a30fa7..6d1a50cf 100644 --- a/test/MockPieceStorage.h +++ b/test/MockPieceStorage.h @@ -72,7 +72,7 @@ public: } virtual SharedHandle getSparseMissingUnusedPiece - (const unsigned char* ignoreBitfield, size_t length) + (size_t minSplitSize, const unsigned char* ignoreBitfield, size_t length) { return SharedHandle(new Piece()); } diff --git a/test/SegmentManTest.cc b/test/SegmentManTest.cc index f32ab520..167802cc 100644 --- a/test/SegmentManTest.cc +++ b/test/SegmentManTest.cc @@ -59,19 +59,20 @@ void SegmentManTest::testNullBitfield() SharedHandle ps (new UnknownLengthPieceStorage(dctx, &op)); SegmentMan segmentMan(&op, dctx, ps); + size_t minSplitSize = dctx->getPieceLength(); - SharedHandle segment = segmentMan.getSegment(1); + SharedHandle segment = segmentMan.getSegment(1, minSplitSize); CPPUNIT_ASSERT(!segment.isNull()); CPPUNIT_ASSERT_EQUAL((size_t)0, segment->getIndex()); CPPUNIT_ASSERT_EQUAL((size_t)0, segment->getLength()); CPPUNIT_ASSERT_EQUAL((size_t)0, segment->getSegmentLength()); CPPUNIT_ASSERT_EQUAL((size_t)0, segment->getWrittenLength()); - SharedHandle segment2 = segmentMan.getSegment(2); + SharedHandle segment2 = segmentMan.getSegment(2, minSplitSize); CPPUNIT_ASSERT(segment2.isNull()); segmentMan.cancelSegment(1); - CPPUNIT_ASSERT(!segmentMan.getSegment(2).isNull()); + CPPUNIT_ASSERT(!segmentMan.getSegment(2, minSplitSize).isNull()); } void SegmentManTest::testCompleteSegment() @@ -85,10 +86,10 @@ void SegmentManTest::testCompleteSegment() SegmentMan segmentMan(&op, dctx, ps); - CPPUNIT_ASSERT(!segmentMan.getSegment(1, 0).isNull()); - SharedHandle seg = segmentMan.getSegment(1, 1); + CPPUNIT_ASSERT(!segmentMan.getSegmentWithIndex(1, 0).isNull()); + SharedHandle seg = segmentMan.getSegmentWithIndex(1, 1); CPPUNIT_ASSERT(!seg.isNull()); - CPPUNIT_ASSERT(!segmentMan.getSegment(1, 2).isNull()); + CPPUNIT_ASSERT(!segmentMan.getSegmentWithIndex(1, 2).isNull()); seg->updateWrittenLength(pieceLength); segmentMan.completeSegment(1, seg); @@ -112,33 +113,32 @@ void SegmentManTest::testGetSegment_sameFileEntry() }; dctx->setFileEntries(&fileEntries[0], &fileEntries[3]); SharedHandle ps(new DefaultPieceStorage(dctx, &op)); - ps->setMinSplitSize(dctx->getPieceLength()); SegmentMan segman(&op, dctx, ps); - + size_t minSplitSize =dctx->getPieceLength(); std::vector > segments; - segman.getSegment(segments, 1, fileEntries[1], 4); + segman.getSegment(segments, 1, minSplitSize, fileEntries[1], 4); // See 3 segments are returned, not 4 because the part of file1 is // not filled in segment#1 CPPUNIT_ASSERT_EQUAL((size_t)3, segments.size()); - SharedHandle segmentNo1 = segman.getSegment(2, 1); + SharedHandle segmentNo1 = segman.getSegmentWithIndex(2, 1); // Fill the part of file1 in segment#1 segmentNo1->updateWrittenLength(1); segman.cancelSegment(2); segman.cancelSegment(1); segments.clear(); - segman.getSegment(segments, 1, fileEntries[1], 4); + segman.getSegment(segments, 1, minSplitSize, fileEntries[1], 4); CPPUNIT_ASSERT_EQUAL((size_t)4, segments.size()); segman.cancelSegment(1); - SharedHandle segmentNo4 = segman.getSegment(1, 4); + SharedHandle segmentNo4 = segman.getSegmentWithIndex(1, 4); // Fill the part of file2 in segment#4 segmentNo4->updateWrittenLength(1); segman.cancelSegment(1); segments.clear(); - segman.getSegment(segments, 1, fileEntries[1], 4); + segman.getSegment(segments, 1, minSplitSize, fileEntries[1], 4); // segment#4 is not returned because the part of file2 is filled. CPPUNIT_ASSERT_EQUAL((size_t)3, segments.size()); } @@ -163,13 +163,13 @@ void SegmentManTest::testRegisterPeerStat() void SegmentManTest::testCancelAllSegments() { - segmentMan_->getSegment(1, 0); - segmentMan_->getSegment(2, 1); - CPPUNIT_ASSERT(segmentMan_->getSegment(3, 0).isNull()); - CPPUNIT_ASSERT(segmentMan_->getSegment(4, 1).isNull()); + segmentMan_->getSegmentWithIndex(1, 0); + segmentMan_->getSegmentWithIndex(2, 1); + CPPUNIT_ASSERT(segmentMan_->getSegmentWithIndex(3, 0).isNull()); + CPPUNIT_ASSERT(segmentMan_->getSegmentWithIndex(4, 1).isNull()); segmentMan_->cancelAllSegments(); - CPPUNIT_ASSERT(!segmentMan_->getSegment(3, 0).isNull()); - CPPUNIT_ASSERT(!segmentMan_->getSegment(4, 1).isNull()); + CPPUNIT_ASSERT(!segmentMan_->getSegmentWithIndex(3, 0).isNull()); + CPPUNIT_ASSERT(!segmentMan_->getSegmentWithIndex(4, 1).isNull()); } void SegmentManTest::testGetPeerStat() @@ -181,8 +181,8 @@ void SegmentManTest::testGetPeerStat() void SegmentManTest::testGetCleanSegmentIfOwnerIsIdle() { - SharedHandle seg1 = segmentMan_->getSegment(1, 0); - SharedHandle seg2 = segmentMan_->getSegment(2, 1); + SharedHandle seg1 = segmentMan_->getSegmentWithIndex(1, 0); + SharedHandle seg2 = segmentMan_->getSegmentWithIndex(2, 1); seg2->updateWrittenLength(100); CPPUNIT_ASSERT(!segmentMan_->getCleanSegmentIfOwnerIsIdle(3, 0).isNull()); SharedHandle peerStat3(new PeerStat(3));