2009-07-03 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>

	Rewritten PeerStats handling in SegmentMan.cc.  Now we have 2 list
	of PeerStat in SegmentMan.  peerStats is used for calculating
	download speed. Therefore all active PeerStats should be in
	there. Another one is _fastestPeerStats and it only contains
	fastest PeerStat for each hostname/protocol pair. They are used
	for updating ServerStat.
	* src/DownloadCommand.cc
	* src/Request.h
	* src/RequestGroupMan.cc
	* src/SegmentMan.cc
	* src/SegmentMan.h
	* test/SegmentManTest.cc

ChangeLog

@@ -1,3 +1,18 @@
+2009-07-03  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	Rewritten PeerStats handling in SegmentMan.cc.  Now we have 2 list
+	of PeerStat in SegmentMan.  peerStats is used for calculating
+	download speed. Therefore all active PeerStats should be in
+	there. Another one is _fastestPeerStats and it only contains
+	fastest PeerStat for each hostname/protocol pair. They are used
+	for updating ServerStat.
+	* src/DownloadCommand.cc
+	* src/Request.h
+	* src/RequestGroupMan.cc
+	* src/SegmentMan.cc
+	* src/SegmentMan.h
+	* test/SegmentManTest.cc
+
 2009-07-03  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
 
 	Try all available addresses returned by DNS until it gets

src/DownloadCommand.cc

@@ -89,18 +89,15 @@ DownloadCommand::DownloadCommand(int cuid,
     }
   }
 #endif // ENABLE_MESSAGE_DIGEST
-  peerStat = req->getPeerStat();
-  if(peerStat.isNull()) {
-    peerStat = req->initPeerStat();
-    _requestGroup->getSegmentMan()->registerPeerStat(peerStat);
-  }
+
+  peerStat = req->initPeerStat();
   peerStat->downloadStart();
+  _requestGroup->getSegmentMan()->registerPeerStat(peerStat);
 }
 
 DownloadCommand::~DownloadCommand() {
-  assert(peerStat.get());
   peerStat->downloadStop();
-  req->purgePeerStat();
+  _requestGroup->getSegmentMan()->updateFastestPeerStat(peerStat);
 }
 
 bool DownloadCommand::executeInternal() {

src/Request.h

@@ -185,8 +185,6 @@ public:
 
   const SharedHandle<PeerStat>& initPeerStat();
 
-  void purgePeerStat() { _peerStat.reset(); }
-
   static const std::string METHOD_GET;
   static const std::string METHOD_HEAD;
 

src/RequestGroupMan.cc

@@ -329,34 +329,6 @@ public:
   }
 };
 
-class PeerStatFaster {
-public:
-  bool operator()(const SharedHandle<PeerStat>& lhs,
-		  const SharedHandle<PeerStat>& rhs) const
-  {
-    int r;
-    r = lhs->getHostname().compare(rhs->getHostname());
-    if(r != 0) {
-      return r < 0;
-    }
-    r = lhs->getProtocol().compare(rhs->getProtocol());
-    if(r != 0) {
-      return r < 0;
-    }
-    return lhs->getAvgDownloadSpeed() > rhs->getAvgDownloadSpeed();
-  }
-};
-
-class PeerStatHostProtoEqual {
-public:
-  bool operator()(const SharedHandle<PeerStat>& lhs,
-		  const SharedHandle<PeerStat>& rhs) const
-  {
-    return lhs->getHostname() == rhs->getHostname() &&
-      lhs->getProtocol() == rhs->getProtocol();
-  }
-};
-
 class CollectServerStat {
 private:
   RequestGroupMan* _requestGroupMan;
@@ -370,16 +342,12 @@ public:
       // Collect statistics during download in PeerStats and update/register
       // ServerStatMan
       if(!group->getSegmentMan().isNull()) {
-
-	std::deque<SharedHandle<PeerStat> > peerStats =
-	  group->getSegmentMan()->getPeerStats();
-	std::sort(peerStats.begin(), peerStats.end(), PeerStatFaster());
-	// Use fastest PeerStat for each hostname/protocol pair.
+	bool singleConnection =
+	  group->getSegmentMan()->getPeerStats().size() == 1;
+	const std::deque<SharedHandle<PeerStat> >& peerStats =
+	  group->getSegmentMan()->getFastestPeerStats();
 	for(std::deque<SharedHandle<PeerStat> >::const_iterator i =
-	      peerStats.begin(),
-	      last = std::unique(peerStats.begin(), peerStats.end(),
-				 PeerStatHostProtoEqual());
-	    i != last; ++i) {
+	      peerStats.begin(); i != peerStats.end(); ++i) {
 	  if((*i)->getHostname().empty() || (*i)->getProtocol().empty()) {
 	    continue;
 	  }
@@ -391,7 +359,7 @@ public:
 						    (*i)->getProtocol());
           ss->increaseCounter();
 	  ss->updateDownloadSpeed(speed);
-          if(peerStats.size() == 1) {
+          if(singleConnection) {
             ss->updateSingleConnectionAvgSpeed(speed);
           }
           else {

src/SegmentMan.cc

@@ -282,9 +282,42 @@ uint64_t SegmentMan::getDownloadLength() const {
 
 void SegmentMan::registerPeerStat(const SharedHandle<PeerStat>& peerStat)
 {
+  for(std::deque<SharedHandle<PeerStat> >::iterator i = peerStats.begin();
+      i != peerStats.end(); ++i) {
+    if((*i)->getStatus() == PeerStat::IDLE) {
+      *i = peerStat;
+      return;
+    }
+  }
   peerStats.push_back(peerStat);
 }
 
+class PeerStatHostProtoEqual {
+private:
+  const SharedHandle<PeerStat>& _peerStat;
+public:
+  PeerStatHostProtoEqual(const SharedHandle<PeerStat>& peerStat):
+    _peerStat(peerStat) {}
+
+  bool operator()(const SharedHandle<PeerStat>& p) const
+  {
+    return _peerStat->getHostname() == p->getHostname() &&
+      _peerStat->getProtocol() == p->getProtocol();
+  }
+};
+
+void SegmentMan::updateFastestPeerStat(const SharedHandle<PeerStat>& peerStat)
+{
+  std::deque<SharedHandle<PeerStat> >::iterator i =
+    std::find_if(_fastestPeerStats.begin(), _fastestPeerStats.end(),
+		 PeerStatHostProtoEqual(peerStat));
+  if(i == _fastestPeerStats.end()) {
+    _fastestPeerStats.push_back(peerStat);
+  } else if((*i)->getAvgDownloadSpeed() < peerStat->getAvgDownloadSpeed()) {
+    *i = peerStat;
+  }
+}
+
 unsigned int SegmentMan::calculateDownloadSpeed()
 {
   unsigned int speed = 0;

src/SegmentMan.h

@@ -88,8 +88,12 @@ private:
   // segment. The value is writtenLength for that segment.
   std::map<size_t, size_t> _segmentWrittenLengthMemo;
 
+  // Used for calculating download speed.
   std::deque<SharedHandle<PeerStat> > peerStats;
 
+  // Keep track of fastest PeerStat for each server
+  std::deque<SharedHandle<PeerStat> > _fastestPeerStats;
+
   // key: PeerStat's cuid, value: its download speed
   std::map<cuid_t, unsigned int> _peerStatDlspdMap;
 
@@ -188,10 +192,10 @@ public:
    */
   uint64_t getDownloadLength() const;
 
-  /**
-   * Registers given peerStat if it has not been registerd and returns true.
-   * Otherwise does nothing and returns false.
-   */
+
+  // If there is inactive PeerStat in peerStats, it is replaced with
+  // given peerStat. If no such PeerStat exist, the given peerStat is
+  // inserted.
   void registerPeerStat(const SharedHandle<PeerStat>& peerStat);
 
   const std::deque<SharedHandle<PeerStat> >& getPeerStats() const
@@ -199,6 +203,17 @@ public:
     return peerStats;
   }
 
+  // If there is slower PeerStat than given peerStat for the same
+  // hostname and protocol in _fastestPeerStats, the former is
+  // replaced with latter. If there are no PeerStat with same hostname
+  // and protocol with given peerStat, given peerStat is inserted.
+  void updateFastestPeerStat(const SharedHandle<PeerStat>& peerStat);
+
+  const std::deque<SharedHandle<PeerStat> >& getFastestPeerStats() const
+  {
+    return _fastestPeerStats;
+  }
+
   /**
    * Returns current download speed in bytes per sec. 
    */

test/SegmentManTest.cc

@@ -17,6 +17,7 @@ class SegmentManTest:public CppUnit::TestFixture {
   CPPUNIT_TEST(testNullBitfield);
   CPPUNIT_TEST(testCompleteSegment);
   CPPUNIT_TEST(testGetSegment_sameFileEntry);
+  CPPUNIT_TEST(testRegisterPeerStat);
   CPPUNIT_TEST_SUITE_END();
 private:
 
@@ -28,6 +29,7 @@ public:
   void testCompleteSegment();
   void testGetPeerStat();
   void testGetSegment_sameFileEntry();
+  void testRegisterPeerStat();
 };
 
 
@@ -124,4 +126,22 @@ void SegmentManTest::testGetSegment_sameFileEntry()
   CPPUNIT_ASSERT_EQUAL((size_t)3, segments.size());
 }
 
+void SegmentManTest::testRegisterPeerStat()
+{
+  Option op;
+  SharedHandle<DownloadContext> dctx(new DownloadContext());
+  SharedHandle<DefaultPieceStorage> ps(new DefaultPieceStorage(dctx, &op));
+  SegmentMan segman(&op, dctx, ps);
+  
+  SharedHandle<PeerStat> p1(new PeerStat(0, "host1", "http"));
+  segman.registerPeerStat(p1);
+  CPPUNIT_ASSERT_EQUAL((size_t)1, segman.getPeerStats().size());
+  SharedHandle<PeerStat> p2(new PeerStat(0, "host2", "http"));
+  segman.registerPeerStat(p2);
+  CPPUNIT_ASSERT_EQUAL((size_t)1, segman.getPeerStats().size());
+  p2->downloadStart();
+  segman.registerPeerStat(p1);
+  CPPUNIT_ASSERT_EQUAL((size_t)2, segman.getPeerStats().size());
+}
+
 } // namespace aria2