diff --git a/ChangeLog b/ChangeLog index 71ae3ab7..3c011d35 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2007-11-07 Tatsuhiro Tsujikawa + + 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 Fixed: Can not resume: aria2 reports download already finished. diff --git a/src/DefaultPieceStorage.cc b/src/DefaultPieceStorage.cc index e75570f6..d7e7a51d 100644 --- a/src/DefaultPieceStorage.cc +++ b/src/DefaultPieceStorage.cc @@ -47,6 +47,8 @@ #include "DefaultDiskWriterFactory.h" #include "DlAbortEx.h" #include "Util.h" +#include "a2functional.h" +#include 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(), mem_fun_sh(&Piece::getCompletedLength))); } // not unittested diff --git a/src/DefaultPieceStorage.h b/src/DefaultPieceStorage.h index 9ef68a3e..e1ec4713 100644 --- a/src/DefaultPieceStorage.h +++ b/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(); diff --git a/src/DownloadCommand.cc b/src/DownloadCommand.cc index 9e4cb943..9252911b 100644 --- a/src/DownloadCommand.cc +++ b/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; diff --git a/src/FtpNegotiationCommand.cc b/src/FtpNegotiationCommand.cc index 8e227d88..1ad72ed1 100644 --- a/src/FtpNegotiationCommand.cc +++ b/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; } diff --git a/src/HttpResponseCommand.cc b/src/HttpResponseCommand.cc index dba8007f..d4a02bea 100644 --- a/src/HttpResponseCommand.cc +++ b/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; } diff --git a/src/RequestGroupMan.cc b/src/RequestGroupMan.cc index 9e5e93a3..653be569 100644 --- a/src/RequestGroupMan.cc +++ b/src/RequestGroupMan.cc @@ -39,8 +39,10 @@ #include "LogFactory.h" #include "DownloadEngine.h" #include "message.h" +#include "a2functional.h" #include #include +#include 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(), mem_fun_sh(&RequestGroup::calculateStat))); } diff --git a/src/a2functional.h b/src/a2functional.h new file mode 100644 index 00000000..963399be --- /dev/null +++ b/src/a2functional.h @@ -0,0 +1,109 @@ +/* */ +#include +#include "SharedHandle.h" + +// mem_fun_t for SharedHandle +template +class mem_fun_sh_t:public std::unary_function< SharedHandle, ReturnType> +{ +private: + ReturnType (ClassType::*f)(); + +public: + mem_fun_sh_t(ReturnType (ClassType::*f)()):f(f) {} + + ReturnType operator()(const SharedHandle& x) const + { + return (x.get()->*f)(); + } +}; + +// const_mem_fun_t for SharedHandle +template +class const_mem_fun_sh_t:public std::unary_function< SharedHandle, ReturnType> +{ +private: + ReturnType (ClassType::*f)() const; + +public: + const_mem_fun_sh_t(ReturnType (ClassType::*f)() const):f(f) {} + + ReturnType operator()(const SharedHandle& x) const + { + return (x.get()->*f)(); + } +}; + +template +mem_fun_sh_t +mem_fun_sh(ReturnType (ClassType::*f)()) +{ + return mem_fun_sh_t(f); +}; + +template +const_mem_fun_sh_t +mem_fun_sh(ReturnType (ClassType::*f)() const) +{ + return const_mem_fun_sh_t(f); +}; + +template +class adopt2nd_t:public std::binary_function { +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 +inline adopt2nd_t +adopt2nd(const BinaryOp& binaryOp, const UnaryOp& unaryOp) +{ + return adopt2nd_t(binaryOp, unaryOp); +}; diff --git a/test/DefaultPieceStorageTest.cc b/test/DefaultPieceStorageTest.cc index 0a529788..cee9480b 100644 --- a/test/DefaultPieceStorageTest.cc +++ b/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() { diff --git a/test/Makefile.am b/test/Makefile.am index b9f8688d..8c21447e 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -1,6 +1,7 @@ TESTS = aria2c check_PROGRAMS = $(TESTS) aria2c_SOURCES = AllTest.cc\ + a2functionalTest.cc\ FileEntryTest.cc\ PieceTest.cc\ DefaultPieceStorageTest.cc\ diff --git a/test/Makefile.in b/test/Makefile.in index 767f6869..961c76a8 100644 --- a/test/Makefile.in +++ b/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 $@ $<; \ diff --git a/test/a2functionalTest.cc b/test/a2functionalTest.cc new file mode 100644 index 00000000..0a936e82 --- /dev/null +++ b/test/a2functionalTest.cc @@ -0,0 +1,64 @@ +#include "a2functional.h" +#include +#include +#include + +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 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(), mem_fun_sh(&Greeting::sayGreeting))("A Japanese said:", greeting)); +}