2007-11-07 Tatsuhiro Tsujikawa <tujikawa at rednoah dot com>

	Reflect the download length of in-flight pieces.
	It makes the download length readout more precise.
	* src/DefaultPieceStorage.{h, cc}
	* test/DefaultPieceStorageTest.cc
	* src/a2functional.h
	* test/a2functionalTest.cc
	
	Lower CPU load when --max-download-limit is used.
	There is up and down in speed indicator when enabling
	http-pipelining but a download goes well. I think the problem is 
that
	because http-pipelining is enabled, DownloadCommand is created 
for
	each segment and in its constructor, PeerStat::downloadStart() 
is
	called. In PeerStat::downloadStart(), speed calculation object 
is
	reseted, which makes download speed zero.
	* src/DownloadCommand.cc

	Rewritten using accumulate.
	* src/RequestGroupMan.cc (calculateStat)
	
	Code clearnup.
	* src/FtpNegotiationCommand.cc
	* src/HttpResponseCommand.cc

ChangeLog

@@ -1,3 +1,28 @@
+2007-11-07  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	Reflect the download length of in-flight pieces.
+	It makes the download length readout more precise.
+	* src/DefaultPieceStorage.{h, cc}
+	* test/DefaultPieceStorageTest.cc
+	* src/a2functional.h
+	* test/a2functionalTest.cc
+	
+	Lower CPU load when --max-download-limit is used.
+	There is up and down in speed indicator when enabling
+	http-pipelining but a download goes well. I think the problem is that
+	because http-pipelining is enabled, DownloadCommand is created for
+	each segment and in its constructor, PeerStat::downloadStart() is
+	called. In PeerStat::downloadStart(), speed calculation object is
+	reseted, which makes download speed zero.
+	* src/DownloadCommand.cc
+
+	Rewritten using accumulate.
+	* src/RequestGroupMan.cc (calculateStat)
+	
+	Code clearnup.
+	* src/FtpNegotiationCommand.cc
+	* src/HttpResponseCommand.cc
+
 2007-11-06  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
 
 	Fixed: Can not resume: aria2 reports download already finished.

src/DefaultPieceStorage.cc

@@ -47,6 +47,8 @@
 #include "DefaultDiskWriterFactory.h"
 #include "DlAbortEx.h"
 #include "Util.h"
+#include "a2functional.h"
+#include <numeric>
 
 DefaultPieceStorage::DefaultPieceStorage(const DownloadContextHandle& downloadContext, const Option* option):
   downloadContext(downloadContext),
@@ -358,12 +360,17 @@ int64_t DefaultPieceStorage::getFilteredTotalLength()
 
 int64_t DefaultPieceStorage::getCompletedLength()
 {
-  return bitfieldMan->getCompletedLength();
+  return bitfieldMan->getCompletedLength()+getInFlightPieceCompletedLength();
 }
 
 int64_t DefaultPieceStorage::getFilteredCompletedLength()
 {
-  return bitfieldMan->getFilteredCompletedLength();
+  return bitfieldMan->getFilteredCompletedLength()+getInFlightPieceCompletedLength();
+}
+
+int32_t DefaultPieceStorage::getInFlightPieceCompletedLength() const
+{
+  return accumulate(usedPieces.begin(), usedPieces.end(), 0, adopt2nd(plus<int32_t>(), mem_fun_sh(&Piece::getCompletedLength)));
 }
 
 // not unittested

src/DefaultPieceStorage.h

@@ -88,6 +88,9 @@ private:
   void reduceUsedPieces(int32_t delMax);
   void deleteUsedPiece(const PieceHandle& piece);
   PieceHandle findUsedPiece(int32_t index) const;
+
+  int32_t getInFlightPieceCompletedLength() const;
+
 public:
   DefaultPieceStorage(const DownloadContextHandle& downloadContext, const Option* option);
   virtual ~DefaultPieceStorage();

src/DownloadCommand.cc

@@ -79,10 +79,11 @@ bool DownloadCommand::executeInternal() {
   // TODO we need to specify the sum of all segmentMan's download speed here.
   if(maxDownloadSpeedLimit > 0 &&
      maxDownloadSpeedLimit < _requestGroup->getSegmentMan()->calculateDownloadSpeed()) {
-  Util::usleep(1);
     e->commands.push_back(this);
+    disableReadCheckSocket();
     return false;
   }
+  setReadCheckSocket(socket);
   SegmentHandle segment = _segments.front();
 
   int32_t BUFSIZE = 16*1024;

src/FtpNegotiationCommand.cc

@@ -200,10 +200,11 @@ bool FtpNegotiationCommand::recvSize() {
 
     initPieceStorage();
 
-    // TODO validate filename and totalsize against hintFilename and hintTotalSize if these are provided.
+    // TODO validate totalsize against hintTotalSize if it is provided.
     _requestGroup->validateTotalLengthByHint(size);
+    // TODO Is this really necessary?
     if(req->getMethod() == Request::METHOD_HEAD) {
-      //_requestGroup->getSegmentMan()->isSplittable = false; // TODO because we don't want segment file to be saved.
+      // TODO because we don't want segment file to be saved.
       sequence = SEQ_HEAD_OK;
       return false;
     }

src/HttpResponseCommand.cc

@@ -75,12 +75,6 @@ bool HttpResponseCommand::executeInternal()
   // check HTTP status number
   httpResponse->validateResponse();
   httpResponse->retrieveCookie();
-  // check whether the server supports persistent connections.
-  /*
-  if(Util::toLower(headers.getFirst("Connection")).find("close") != string::npos) {
-    req->setKeepAlive(false);
-  }
-  */
   // check whether Location header exists. If it does, update request object
   // with redirected URL.
   // then establish a connection to the new host and port
@@ -116,18 +110,13 @@ bool HttpResponseCommand::handleDefaultEncoding(const HttpResponseHandle& httpRe
   if(size == INT64_MAX || size < 0) {
     throw new DlAbortEx(EX_TOO_LARGE_FILE, Util::llitos(size, true).c_str());
   }
-  //_requestGroup->getSegmentMan()->isSplittable = !(size == 0);
-  //_requestGroup->getSegmentMan()->totalSize = size;
-  //_requestGroup->getSegmentMan()->initDownloadContext(size);
-
   SingleFileDownloadContextHandle(_requestGroup->getDownloadContext())->setTotalLength(size);
 
   initPieceStorage();
 
-  // quick hack for method 'head'
+  // quick hack for method 'head',, is it necessary?
   if(httpRequest->getMethod() == Request::METHOD_HEAD) {
     // TODO because we don't want segment file to be saved.
-    //_requestGroup->getSegmentMan()->isSplittable = false;
     return true;
   }
 
@@ -156,11 +145,9 @@ bool HttpResponseCommand::handleDefaultEncoding(const HttpResponseHandle& httpRe
 }
 
 bool HttpResponseCommand::handleOtherEncoding(const HttpResponseHandle& httpResponse) {
-  HttpRequestHandle httpRequest = httpResponse->getHttpRequest();
   // we ignore content-length when transfer-encoding is set
-  //_requestGroup->getSegmentMan()->isSplittable = false;
-  //_requestGroup->getSegmentMan()->totalSize = 0;
-  // quick hack for method 'head'
+  HttpRequestHandle httpRequest = httpResponse->getHttpRequest();
+  // quick hack for method 'head',, is it necessary?
   if(httpRequest->getMethod() == Request::METHOD_HEAD) {
     return true;
   }
@@ -169,11 +156,9 @@ bool HttpResponseCommand::handleOtherEncoding(const HttpResponseHandle& httpResp
 
   initPieceStorage();
 
-  //segment = _requestGroup->getSegmentMan()->getSegment(cuid);
-
   shouldCancelDownloadForSafety();
   // TODO handle file-size unknown case
-  _requestGroup->getPieceStorage()->getDiskAdaptor()->initAndOpenFile();//_requestGroup->getFilePath());
+  _requestGroup->getPieceStorage()->getDiskAdaptor()->initAndOpenFile();
   e->commands.push_back(createHttpDownloadCommand(httpResponse));
   return true;
 }

src/RequestGroupMan.cc

@@ -39,8 +39,10 @@
 #include "LogFactory.h"
 #include "DownloadEngine.h"
 #include "message.h"
+#include "a2functional.h"
 #include <iomanip>
 #include <sstream>
+#include <numeric>
 
 RequestGroupMan::RequestGroupMan(const RequestGroups& requestGroups,
 				 int32_t maxSimultaneousDownloads):
@@ -277,10 +279,6 @@ void RequestGroupMan::halt()
 
 TransferStat RequestGroupMan::calculateStat()
 {
-  TransferStat stat;
-  for(RequestGroups::const_iterator itr = _requestGroups.begin();
-      itr != _requestGroups.end(); ++itr) {
-    stat = stat+(*itr)->calculateStat();
-  }
-  return stat;
+  return accumulate(_requestGroups.begin(), _requestGroups.end(), TransferStat(),
+		    adopt2nd(plus<TransferStat>(), mem_fun_sh(&RequestGroup::calculateStat)));
 }

src/a2functional.h

@@ -0,0 +1,109 @@
+/* <!-- copyright */
+/*
+ * aria2 - The high speed download utility
+ *
+ * Copyright (C) 2006 Tatsuhiro Tsujikawa
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * In addition, as a special exception, the copyright holders give
+ * permission to link the code of portions of this program with the
+ * OpenSSL library under certain conditions as described in each
+ * individual source file, and distribute linked combinations
+ * including the two.
+ * You must obey the GNU General Public License in all respects
+ * for all of the code used other than OpenSSL.  If you modify
+ * file(s) with this exception, you may extend this exception to your
+ * version of the file(s), but you are not obligated to do so.  If you
+ * do not wish to do so, delete this exception statement from your
+ * version.  If you delete this exception statement from all source
+ * files in the program, then also delete it here.
+ */
+/* copyright --> */
+#include <functional>
+#include "SharedHandle.h"
+
+// mem_fun_t for SharedHandle
+template <class ReturnType, typename ClassType>
+class mem_fun_sh_t:public std::unary_function< SharedHandle<ClassType>, ReturnType>
+{
+private:
+  ReturnType (ClassType::*f)();
+
+public:
+  mem_fun_sh_t(ReturnType (ClassType::*f)()):f(f) {}
+
+  ReturnType operator()(const SharedHandle<ClassType>& x) const
+  {
+    return (x.get()->*f)();
+  }
+};
+
+// const_mem_fun_t for SharedHandle
+template <class ReturnType, typename ClassType>
+class const_mem_fun_sh_t:public std::unary_function< SharedHandle<ClassType>, ReturnType>
+{
+private:
+  ReturnType (ClassType::*f)() const;
+
+public:
+  const_mem_fun_sh_t(ReturnType (ClassType::*f)() const):f(f) {}
+
+  ReturnType operator()(const SharedHandle<ClassType>& x) const
+  {
+    return (x.get()->*f)();
+  }
+};
+
+template <class ReturnType, typename ClassType>
+mem_fun_sh_t<ReturnType, ClassType>
+mem_fun_sh(ReturnType (ClassType::*f)())
+{
+  return mem_fun_sh_t<ReturnType, ClassType>(f);
+};
+
+template <class ReturnType, typename ClassType>
+const_mem_fun_sh_t<ReturnType, ClassType>
+mem_fun_sh(ReturnType (ClassType::*f)() const)
+{
+  return const_mem_fun_sh_t<ReturnType, ClassType>(f);
+};
+
+template<class BinaryOp, class UnaryOp>
+class adopt2nd_t:public std::binary_function<typename BinaryOp::first_argument_type,
+					     typename UnaryOp::argument_type,
+					     typename BinaryOp::result_type> {
+private:
+  BinaryOp _binaryOp;
+  UnaryOp _unaryOp;
+public:
+  adopt2nd_t(const BinaryOp& b, const UnaryOp& u):
+    _binaryOp(b), _unaryOp(u) {}
+
+  typename BinaryOp::result_type
+  operator()(const typename BinaryOp::first_argument_type& x,
+	     const typename UnaryOp::argument_type& y)
+  {
+    return _binaryOp(x, _unaryOp(y));
+  }
+
+};
+
+template <class BinaryOp, class UnaryOp>
+inline adopt2nd_t<BinaryOp, UnaryOp>
+adopt2nd(const BinaryOp& binaryOp, const UnaryOp& unaryOp)
+{
+  return adopt2nd_t<BinaryOp, UnaryOp>(binaryOp, unaryOp);
+};

test/DefaultPieceStorageTest.cc

@@ -119,20 +119,24 @@ void DefaultPieceStorageTest::testCompletePiece() {
   DefaultPieceStorage pss(btContext, option);
   pss.setEndGamePieceNum(0);
 
-    peer->setAllBitfield();
+  peer->setAllBitfield();
 
   PieceHandle piece = pss.getMissingPiece(peer);
   CPPUNIT_ASSERT_EQUAL(string("piece: index=0, length=128"),
 		       piece->toString());
 
-  CPPUNIT_ASSERT_EQUAL((long long int)0,
+  CPPUNIT_ASSERT_EQUAL((int64_t)0,
 		       pss.getCompletedLength());
 
   pss.completePiece(piece);
 
-  CPPUNIT_ASSERT_EQUAL((long long int)128,
+  CPPUNIT_ASSERT_EQUAL((int64_t)128,
 		       pss.getCompletedLength());
 
+  PieceHandle incompletePiece = pss.getMissingPiece(peer);
+  incompletePiece->completeBlock(0);
+  CPPUNIT_ASSERT_EQUAL((int64_t)256,
+		       pss.getCompletedLength());
 }
 
 void DefaultPieceStorageTest::testGetPiece() {

test/Makefile.am

@@ -1,6 +1,7 @@
 TESTS = aria2c
 check_PROGRAMS = $(TESTS)
 aria2c_SOURCES = AllTest.cc\
+	a2functionalTest.cc\
 	FileEntryTest.cc\
 	PieceTest.cc\
 	DefaultPieceStorageTest.cc\

test/Makefile.in

@@ -112,8 +112,9 @@ mkinstalldirs = $(SHELL) $(top_srcdir)/mkinstalldirs
 CONFIG_HEADER = $(top_builddir)/config.h
 CONFIG_CLEAN_FILES =
 am__EXEEXT_1 = aria2c$(EXEEXT)
-am__aria2c_SOURCES_DIST = AllTest.cc FileEntryTest.cc PieceTest.cc \
-	DefaultPieceStorageTest.cc SegmentTest.cc GrowSegmentTest.cc \
+am__aria2c_SOURCES_DIST = AllTest.cc a2functionalTest.cc \
+	FileEntryTest.cc PieceTest.cc DefaultPieceStorageTest.cc \
+	SegmentTest.cc GrowSegmentTest.cc \
 	SingleFileAllocationIteratorTest.cc \
 	DefaultBtProgressInfoFileTest.cc \
 	SingleFileDownloadContextTest.cc RequestGroupTest.cc \
@@ -203,9 +204,10 @@ am__aria2c_SOURCES_DIST = AllTest.cc FileEntryTest.cc PieceTest.cc \
 @ENABLE_METALINK_TRUE@	Metalink2RequestGroupTest.$(OBJEXT) \
 @ENABLE_METALINK_TRUE@	MetalinkPostDownloadHandlerTest.$(OBJEXT) \
 @ENABLE_METALINK_TRUE@	MetalinkHelperTest.$(OBJEXT)
-am_aria2c_OBJECTS = AllTest.$(OBJEXT) FileEntryTest.$(OBJEXT) \
-	PieceTest.$(OBJEXT) DefaultPieceStorageTest.$(OBJEXT) \
-	SegmentTest.$(OBJEXT) GrowSegmentTest.$(OBJEXT) \
+am_aria2c_OBJECTS = AllTest.$(OBJEXT) a2functionalTest.$(OBJEXT) \
+	FileEntryTest.$(OBJEXT) PieceTest.$(OBJEXT) \
+	DefaultPieceStorageTest.$(OBJEXT) SegmentTest.$(OBJEXT) \
+	GrowSegmentTest.$(OBJEXT) \
 	SingleFileAllocationIteratorTest.$(OBJEXT) \
 	DefaultBtProgressInfoFileTest.$(OBJEXT) \
 	SingleFileDownloadContextTest.$(OBJEXT) \
@@ -414,9 +416,9 @@ target_cpu = @target_cpu@
 target_os = @target_os@
 target_vendor = @target_vendor@
 TESTS = aria2c
-aria2c_SOURCES = AllTest.cc FileEntryTest.cc PieceTest.cc \
-	DefaultPieceStorageTest.cc SegmentTest.cc GrowSegmentTest.cc \
-	SingleFileAllocationIteratorTest.cc \
+aria2c_SOURCES = AllTest.cc a2functionalTest.cc FileEntryTest.cc \
+	PieceTest.cc DefaultPieceStorageTest.cc SegmentTest.cc \
+	GrowSegmentTest.cc SingleFileAllocationIteratorTest.cc \
 	DefaultBtProgressInfoFileTest.cc \
 	SingleFileDownloadContextTest.cc RequestGroupTest.cc \
 	PStringBuildVisitorTest.cc ParameterizedStringParserTest.cc \
@@ -584,6 +586,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/TimeSeedCriteriaTest.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/UtilTest.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/Xml2MetalinkProcessorTest.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/a2functionalTest.Po@am__quote@
 
 .cc.o:
 @am__fastdepCXX_TRUE@	if $(CXXCOMPILE) -MT $@ -MD -MP -MF "$(DEPDIR)/$*.Tpo" -c -o $@ $<; \

test/a2functionalTest.cc

@@ -0,0 +1,64 @@
+#include "a2functional.h"
+#include <string>
+#include <numeric>
+#include <cppunit/extensions/HelperMacros.h>
+
+using namespace std;
+
+class a2functionalTest:public CppUnit::TestFixture {
+
+  CPPUNIT_TEST_SUITE(a2functionalTest);
+  CPPUNIT_TEST(testMemFunSh);
+  CPPUNIT_TEST(testAdopt2nd);
+  CPPUNIT_TEST_SUITE_END();
+public:
+  void testMemFunSh();
+  void testAdopt2nd();
+
+  class Greeting {
+  public:
+    virtual ~Greeting() {}
+
+    virtual string sayGreeting() = 0;
+
+    virtual string sayGreetingConst() const = 0;
+  };
+
+  typedef SharedHandle<Greeting> GreetingHandle;
+
+  class JapaneseGreeting:public Greeting
+  {
+    virtual string sayGreeting()
+    {
+      return "HAROO WAARUDO";
+    }
+
+    virtual string sayGreetingConst() const
+    {
+      return "HAROO WAARUDO";
+    }
+
+  };
+
+};
+
+
+CPPUNIT_TEST_SUITE_REGISTRATION(a2functionalTest);
+
+void a2functionalTest::testMemFunSh()
+{
+  GreetingHandle greeting = new JapaneseGreeting();
+
+  CPPUNIT_ASSERT_EQUAL(string("HAROO WAARUDO"), mem_fun_sh(&Greeting::sayGreeting)(greeting));
+
+  CPPUNIT_ASSERT_EQUAL(string("HAROO WAARUDO"), mem_fun_sh(&Greeting::sayGreetingConst)(greeting));
+
+}
+
+void a2functionalTest::testAdopt2nd()
+{
+  GreetingHandle greeting = new JapaneseGreeting();
+
+  CPPUNIT_ASSERT_EQUAL(string("A Japanese said:HAROO WAARUDO"),
+		       adopt2nd(plus<string>(), mem_fun_sh(&Greeting::sayGreeting))("A Japanese said:", greeting));
+}