2010-07-17 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>

	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

@@ -1,3 +1,24 @@
+2010-07-17  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	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  <t-tujikawa@users.sourceforge.net>
 
 	Removed prepareForRetry from CreateRequestCommand

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<Request> 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<Request> 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> 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

src/AbstractCommand.h

@@ -77,6 +77,8 @@ private:
 
   bool incNumConnection_;
 
+  size_t calculateMinSplitSize() const;
+
 #ifdef ENABLE_ASYNC_DNS
   void setNameResolverCheck(const SharedHandle<AsyncNameResolver>& resolver);
 

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<Piece> 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<Piece>();

src/DefaultPieceStorage.h

@@ -87,8 +87,6 @@ private:
 
   SharedHandle<PieceSelector> 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<Piece> getSparseMissingUnusedPiece
-  (const unsigned char* ignoreBitfield, size_t length);
+  (size_t minSplitSize, const unsigned char* ignoreBitfield, size_t length);
 
   virtual SharedHandle<Piece> getMissingPiece(size_t index);
 
@@ -253,11 +251,6 @@ public:
   {
     return pieceSelector_;
   }
-
-  void setMinSplitSize(size_t minSplitSize)
-  {
-    minSplitSize_ = minSplitSize;
-  }
 };
 
 typedef SharedHandle<DefaultPieceStorage> DefaultPieceStorageHandle;

src/DownloadCommand.cc

@@ -341,7 +341,7 @@ bool DownloadCommand::prepareForNextSegment() {
          (tempSegment->getPosition()+tempSegment->getLength())) {
         return prepareForRetry(0);
       }
-      SharedHandle<Segment> nextSegment = getSegmentMan()->getSegment
+      SharedHandle<Segment> nextSegment = getSegmentMan()->getSegmentWithIndex
         (getCuid(), tempSegment->getIndex()+1);
       if(nextSegment.isNull()) {
         nextSegment = getSegmentMan()->getCleanSegmentIfOwnerIsIdle

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);
 

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> segment = getSegmentMan()->getSegment(getCuid(), 0);
+  SharedHandle<Segment> 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,

src/PieceStorage.h

@@ -108,7 +108,7 @@ public:
    * If ignoreBitfield is set, indexes of true bit are excluded.
    */
   virtual SharedHandle<Piece> 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.

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

src/SegmentMan.cc

@@ -165,16 +165,19 @@ void SegmentMan::getInFlightSegment
   }
 }
 
-SharedHandle<Segment> SegmentMan::getSegment(cuid_t cuid) {
+SharedHandle<Segment> SegmentMan::getSegment(cuid_t cuid, size_t minSplitSize)
+{
   SharedHandle<Piece> piece =
     pieceStorage_->getSparseMissingUnusedPiece
-    (ignoreBitfield_.getFilterBitfield(),ignoreBitfield_.getBitfieldLength());
+    (minSplitSize,
+     ignoreBitfield_.getFilterBitfield(), ignoreBitfield_.getBitfieldLength());
   return checkoutSegment(cuid, piece);
 }
 
 void SegmentMan::getSegment
 (std::vector<SharedHandle<Segment> >& segments,
  cuid_t cuid,
+ size_t minSplitSize,
  const SharedHandle<FileEntry>& fileEntry,
  size_t maxSegments)
 {
@@ -186,7 +189,8 @@ void SegmentMan::getSegment
     SharedHandle<Segment> 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<Segment> SegmentMan::getSegment(cuid_t cuid, size_t index) {
+SharedHandle<Segment> SegmentMan::getSegmentWithIndex
+(cuid_t cuid, size_t index) {
   if(index > 0 && downloadContext_->getNumPieces() <= index) {
     return SharedHandle<Segment>();
   }
@@ -230,7 +235,7 @@ SharedHandle<Segment> SegmentMan::getCleanSegmentIfOwnerIsIdle
       SharedHandle<PeerStat> 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<Segment>();
       }

src/SegmentMan.h

@@ -138,12 +138,13 @@ public:
   void getInFlightSegment(std::vector<SharedHandle<Segment> >& segments,
                           cuid_t cuid);
 
-  SharedHandle<Segment> getSegment(cuid_t cuid);
+  SharedHandle<Segment> 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<SharedHandle<Segment> >& segments,
                   cuid_t cuid,
+                  size_t minSplitSize,
                   const SharedHandle<FileEntry>& 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<Segment> getSegment(cuid_t cuid, size_t index);
+  SharedHandle<Segment> 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

src/UnknownLengthPieceStorage.cc

@@ -108,7 +108,7 @@ bool UnknownLengthPieceStorage::hasMissingUnusedPiece()
 }
 
 SharedHandle<Piece> UnknownLengthPieceStorage::getSparseMissingUnusedPiece
-(const unsigned char* ignoreBitfield, size_t length)
+(size_t minSplitSize, const unsigned char* ignoreBitfield, size_t length)
 {
   if(downloadFinished_) {
     return SharedHandle<Piece>();
@@ -124,7 +124,7 @@ SharedHandle<Piece> UnknownLengthPieceStorage::getSparseMissingUnusedPiece
 SharedHandle<Piece> UnknownLengthPieceStorage::getMissingPiece(size_t index)
 {
   if(index == 0) {
-    return getSparseMissingUnusedPiece(0, 0);
+    return getSparseMissingUnusedPiece(0, 0, 0);
   } else {
     return SharedHandle<Piece>();
   }

src/UnknownLengthPieceStorage.h

@@ -104,7 +104,7 @@ public:
    * Returns a missing piece if available. Otherwise returns 0;
    */
   virtual SharedHandle<Piece> 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.

test/MockPieceStorage.h

@@ -72,7 +72,7 @@ public:
   }
 
   virtual SharedHandle<Piece> getSparseMissingUnusedPiece
-  (const unsigned char* ignoreBitfield, size_t length)
+  (size_t minSplitSize, const unsigned char* ignoreBitfield, size_t length)
   {
     return SharedHandle<Piece>(new Piece());
   }

test/SegmentManTest.cc

@@ -59,19 +59,20 @@ void SegmentManTest::testNullBitfield()
   SharedHandle<UnknownLengthPieceStorage> ps
     (new UnknownLengthPieceStorage(dctx, &op));
   SegmentMan segmentMan(&op, dctx, ps);
+  size_t minSplitSize = dctx->getPieceLength();
 
-  SharedHandle<Segment> segment = segmentMan.getSegment(1);
+  SharedHandle<Segment> 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<Segment> segment2 = segmentMan.getSegment(2);
+  SharedHandle<Segment> 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<Segment> seg = segmentMan.getSegment(1, 1);
+  CPPUNIT_ASSERT(!segmentMan.getSegmentWithIndex(1, 0).isNull());
+  SharedHandle<Segment> 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<DefaultPieceStorage> ps(new DefaultPieceStorage(dctx, &op));
-  ps->setMinSplitSize(dctx->getPieceLength());
   SegmentMan segman(&op, dctx, ps);
-
+  size_t minSplitSize =dctx->getPieceLength();
   std::vector<SharedHandle<Segment> > 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<Segment> segmentNo1 = segman.getSegment(2, 1);
+  SharedHandle<Segment> 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<Segment> segmentNo4 = segman.getSegment(1, 4);
+  SharedHandle<Segment> 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<Segment> seg1 = segmentMan_->getSegment(1, 0);
-  SharedHandle<Segment> seg2 = segmentMan_->getSegment(2, 1);
+  SharedHandle<Segment> seg1 = segmentMan_->getSegmentWithIndex(1, 0);
+  SharedHandle<Segment> seg2 = segmentMan_->getSegmentWithIndex(2, 1);
   seg2->updateWrittenLength(100);
   CPPUNIT_ASSERT(!segmentMan_->getCleanSegmentIfOwnerIsIdle(3, 0).isNull());
   SharedHandle<PeerStat> peerStat3(new PeerStat(3));