Remove PeerStorage::getActivePeers() and add getUsedPeers() instead

PeerStorage::getUsedPeers() returns non-active peers, so caller must
call Peer::isActive() to get active peers.

src/BtLeecherStateChoke.cc

@@ -121,11 +121,6 @@ void BtLeecherStateChoke::PeerEntry::disableOptUnchoking()
   peer_->optUnchoking(false);
 }
 
-bool BtLeecherStateChoke::PeerEntry::isSnubbing() const
-{
-  return peer_->snubbing();
-}
-
 bool BtLeecherStateChoke::PeerEntry::operator<(const PeerEntry& peerEntry) const
 {
   return downloadSpeed_ > peerEntry.downloadSpeed_;
@@ -203,24 +198,19 @@ void BtLeecherStateChoke::regularUnchoke(std::vector<PeerEntry>& peerEntries)
   }
 }
 
-void
-BtLeecherStateChoke::executeChoke
-(const std::vector<SharedHandle<Peer> >& peerSet)
+void BtLeecherStateChoke::executeChoke(const PeerSet& peerSet)
 {
   A2_LOG_INFO(fmt("Leecher state, %d choke round started", round_));
   lastRound_ = global::wallclock();
 
   std::vector<PeerEntry> peerEntries;
-  std::transform(peerSet.begin(), peerSet.end(),
-                 std::back_inserter(peerEntries),
-                 BtLeecherStateChokeGenPeerEntry());
-
-  peerEntries.erase(std::remove_if(peerEntries.begin(), peerEntries.end(),
-                                   std::mem_fun_ref(&PeerEntry::isSnubbing)),
-                    peerEntries.end());
-
-  std::for_each(peerEntries.begin(), peerEntries.end(),
-                std::mem_fun_ref(&PeerEntry::enableChokingRequired));
+  for(PeerSet::const_iterator i = peerSet.begin(), eoi = peerSet.end();
+      i != eoi; ++i) {
+    if((*i)->isActive() && !(*i)->snubbing()) {
+      (*i)->chokingRequired(true);
+      peerEntries.push_back(PeerEntry(*i));
+    }
+  }
 
   // planned optimistic unchoke
   if(round_ == 0) {

src/BtLeecherStateChoke.h

@@ -41,6 +41,7 @@
 
 #include "SharedHandle.h"
 #include "TimerA2.h"
+#include "PeerStorage.h"
 
 namespace aria2 {
 
@@ -74,8 +75,6 @@ private:
 
     bool isRegularUnchoker() const;
 
-    bool isSnubbing() const;
-
     void enableChokingRequired();
 
     void disableChokingRequired();
@@ -100,20 +99,12 @@ private:
 
     bool operator()(const PeerEntry& peerEntry) const;
   };
-
-  class BtLeecherStateChokeGenPeerEntry {
-  public:
-    PeerEntry operator()(const SharedHandle<Peer>& peer) const
-    {
-      return PeerEntry(peer);
-    }
-  };
 public:
   BtLeecherStateChoke();
 
   ~BtLeecherStateChoke();
 
-  void executeChoke(const std::vector<SharedHandle<Peer> >& peerSet);
+  void executeChoke(const PeerSet& peerSet);
 
   const Timer& getLastRound() const;
 

src/BtSeederStateChoke.cc

@@ -117,12 +117,6 @@ void BtSeederStateChoke::PeerEntry::disableOptUnchoking()
   peer_->optUnchoking(false);
 }
 
-bool BtSeederStateChoke::NotInterestedPeer::operator()
-  (const PeerEntry& peerEntry) const
-{
-  return !peerEntry.getPeer()->peerInterested();
-}
-
 void BtSeederStateChoke::unchoke
 (std::vector<BtSeederStateChoke::PeerEntry>& peers)
 {
@@ -151,33 +145,19 @@ void BtSeederStateChoke::unchoke
   }
 }
 
-namespace {
-class ChokingRequired {
-public:
-  void operator()(const SharedHandle<Peer>& peer) const
-  {
-    peer->chokingRequired(true);
-  }
-};
-} // namespace
-
-void
-BtSeederStateChoke::executeChoke
-(const std::vector<SharedHandle<Peer> >& peerSet)
+void BtSeederStateChoke::executeChoke(const PeerSet& peerSet)
 {
   A2_LOG_INFO(fmt("Seeder state, %d choke round started", round_));
   lastRound_ = global::wallclock();
 
   std::vector<PeerEntry> peerEntries;
-
-  std::for_each(peerSet.begin(), peerSet.end(), ChokingRequired());
-
-  std::transform(peerSet.begin(), peerSet.end(),
-                 std::back_inserter(peerEntries), GenPeerEntry());
-
-  peerEntries.erase(std::remove_if(peerEntries.begin(), peerEntries.end(),
-                                   NotInterestedPeer()),
-                    peerEntries.end());
+  for(PeerSet::const_iterator i = peerSet.begin(), eoi = peerSet.end();
+      i != eoi; ++i) {
+    if((*i)->isActive() && (*i)->peerInterested()) {
+      (*i)->chokingRequired(true);
+      peerEntries.push_back(PeerEntry(*i));
+    }
+  }
 
   unchoke(peerEntries);
 

src/BtSeederStateChoke.h

@@ -41,6 +41,7 @@
 
 #include "SharedHandle.h"
 #include "TimerA2.h"
+#include "PeerStorage.h"
 
 namespace aria2 {
 
@@ -80,25 +81,12 @@ private:
   };
 
   void unchoke(std::vector<PeerEntry>& peers);
-
-  class GenPeerEntry {
-  public:
-    PeerEntry operator()(const SharedHandle<Peer>& peer) const
-    {
-      return PeerEntry(peer);
-    }
-  };
-
-  class NotInterestedPeer {
-  public:
-    bool operator()(const PeerEntry& peerEntry) const;
-  };
 public:
   BtSeederStateChoke();
 
   ~BtSeederStateChoke();
 
-  void executeChoke(const std::vector<SharedHandle<Peer> >& peerSet);
+  void executeChoke(const PeerSet& peerSet);
 
   const Timer& getLastRound() const { return lastRound_; }
 

src/ConsoleStatCalc.cc

@@ -173,8 +173,7 @@ void printProgress
 #ifdef ENABLE_BITTORRENT
   const SharedHandle<BtObject>& btObj = e->getBtRegistry()->get(rg->getGID());
   if(btObj) {
-    std::vector<SharedHandle<Peer> > peers;
-    btObj->peerStorage->getActivePeers(peers);
+    const PeerSet& peers = btObj->peerStorage->getUsedPeers();
     o << " SD:"
       << countSeeder(peers.begin(), peers.end());
   }

src/DefaultBtInteractive.cc

@@ -123,11 +123,11 @@ SharedHandle<BtMessage> DefaultBtInteractive::receiveHandshake(bool quickReply)
       (fmt("CUID#%" PRId64 " - Drop connection from the same Peer ID",
            cuid_));
   }
-  std::vector<SharedHandle<Peer> > activePeers;
-  peerStorage_->getActivePeers(activePeers);
-  for(std::vector<SharedHandle<Peer> >::const_iterator i = activePeers.begin(),
-        eoi = activePeers.end(); i != eoi; ++i) {
-    if(memcmp((*i)->getPeerId(), message->getPeerId(), PEER_ID_LENGTH) == 0) {
+  const PeerSet& usedPeers = peerStorage_->getUsedPeers();
+  for(PeerSet::const_iterator i = usedPeers.begin(), eoi = usedPeers.end();
+      i != eoi; ++i) {
+    if((*i)->isActive() &&
+       memcmp((*i)->getPeerId(), message->getPeerId(), PEER_ID_LENGTH) == 0) {
       throw DL_ABORT_EX
         (fmt("CUID#%" PRId64 " - Same Peer ID has been already seen.",
              cuid_));
@@ -485,12 +485,10 @@ void DefaultBtInteractive::addPeerExchangeMessage()
       (new UTPexExtensionMessage(peer_->getExtensionMessageID
                                  (ExtensionMessageRegistry::UT_PEX)));
 
-    std::vector<SharedHandle<Peer> > activePeers;
-    peerStorage_->getActivePeers(activePeers);
-    for(std::vector<SharedHandle<Peer> >::const_iterator i =
-          activePeers.begin(), eoi = activePeers.end();
+    const PeerSet& usedPeers = peerStorage_->getUsedPeers();
+    for(PeerSet::const_iterator i = usedPeers.begin(), eoi = usedPeers.end();
         i != eoi && !m->freshPeersAreFull(); ++i) {
-      if(peer_->getIPAddress() != (*i)->getIPAddress()) {
+      if((*i)->isActive() && peer_->getIPAddress() != (*i)->getIPAddress()) {
         m->addFreshPeer(*i);
       }
     }

src/DefaultPeerStorage.cc

@@ -172,30 +172,6 @@ bool DefaultPeerStorage::isPeerAvailable() {
   return !unusedPeers_.empty();
 }
 
-namespace {
-class CollectActivePeer {
-private:
-  std::vector<SharedHandle<Peer> >& activePeers_;
-public:
-  CollectActivePeer(std::vector<SharedHandle<Peer> >& activePeers):
-    activePeers_(activePeers) {}
-
-  void operator()(const SharedHandle<Peer>& peer)
-  {
-    if(peer->isActive()) {
-      activePeers_.push_back(peer);
-    }
-  }
-};
-} // namespace
-
-void DefaultPeerStorage::getActivePeers
-(std::vector<SharedHandle<Peer> >& activePeers)
-{
-  std::for_each(usedPeers_.begin(), usedPeers_.end(),
-                CollectActivePeer(activePeers));
-}
-
 bool DefaultPeerStorage::isBadPeer(const std::string& ipaddr)
 {
   std::map<std::string, time_t>::iterator i = badPeers_.find(ipaddr);
@@ -302,12 +278,10 @@ bool DefaultPeerStorage::chokeRoundIntervalElapsed()
 
 void DefaultPeerStorage::executeChoke()
 {
-  std::vector<SharedHandle<Peer> > activePeers;
-  getActivePeers(activePeers);
   if(pieceStorage_->downloadFinished()) {
-    return seederStateChoke_->executeChoke(activePeers);
+    return seederStateChoke_->executeChoke(usedPeers_);
   } else {
-    return leecherStateChoke_->executeChoke(activePeers);
+    return leecherStateChoke_->executeChoke(usedPeers_);
   }
 }
 

src/DefaultPeerStorage.h

@@ -39,10 +39,8 @@
 
 #include <string>
 #include <map>
-#include <set>
 
 #include "TimerA2.h"
-#include "a2functional.h"
 
 namespace aria2 {
 
@@ -51,8 +49,6 @@ class BtSeederStateChoke;
 class BtLeecherStateChoke;
 class PieceStorage;
 
-typedef std::set<SharedHandle<Peer>, RefLess<Peer> > PeerSet;
-
 class DefaultPeerStorage : public PeerStorage {
 private:
   SharedHandle<BtRuntime> btRuntime_;
@@ -98,14 +94,12 @@ public:
 
   const std::deque<SharedHandle<Peer> >& getUnusedPeers();
 
-  const PeerSet& getUsedPeers();
+  virtual const PeerSet& getUsedPeers();
 
   virtual const std::deque<SharedHandle<Peer> >& getDroppedPeers();
 
   virtual bool isPeerAvailable();
 
-  virtual void getActivePeers(std::vector<SharedHandle<Peer> >& peers);
-
   virtual bool isBadPeer(const std::string& ipaddr);
 
   virtual void addBadPeer(const std::string& ipaddr);

src/Peer.h

@@ -317,7 +317,13 @@ public:
 template<typename InputIterator>
 size_t countSeeder(InputIterator first, InputIterator last)
 {
-  return std::count_if(first, last, mem_fun_sh(&Peer::isSeeder));
+  size_t res = 0;
+  for(; first != last; ++first) {
+    if((*first)->isActive() && (*first)->isSeeder()) {
+      ++res;
+    }
+  }
+  return res;
 }
 
 } // namespace aria2

src/PeerStorage.h

@@ -39,15 +39,19 @@
 
 #include <deque>
 #include <vector>
+#include <set>
 
 #include "SharedHandle.h"
 #include "TransferStat.h"
 #include "Command.h"
+#include "a2functional.h"
 
 namespace aria2 {
 
 class Peer;
 
+typedef std::set<SharedHandle<Peer>, RefLess<Peer> > PeerSet;
+
 class PeerStorage {
 public:
   virtual ~PeerStorage() {}
@@ -80,9 +84,9 @@ public:
   virtual bool isPeerAvailable() = 0;
 
   /**
-   * Returns the list of peers which are currently connected from localhost.
+   * Returns used peer set.
    */
-  virtual void getActivePeers(std::vector<SharedHandle<Peer> >& peers) = 0;
+  virtual const PeerSet& getUsedPeers() = 0;
 
   /**
    * Returns true if peer with ipaddr should be ignored because, for

src/RpcMethodImpl.cc

@@ -775,8 +775,7 @@ void gatherProgressBitTorrent
     } else {
       const SharedHandle<PeerStorage>& peerStorage = btObject->peerStorage;
       assert(peerStorage);
-      std::vector<SharedHandle<Peer> > peers;
-      peerStorage->getActivePeers(peers);
+      const PeerSet& peers = peerStorage->getUsedPeers();
       entryDict->put(KEY_NUM_SEEDERS,
                      util::uitos(countSeeder(peers.begin(), peers.end())));
     }
@@ -788,10 +787,12 @@ namespace {
 void gatherPeer
 (const SharedHandle<List>& peers, const SharedHandle<PeerStorage>& ps)
 {
-  std::vector<SharedHandle<Peer> > activePeers;
-  ps->getActivePeers(activePeers);
-  for(std::vector<SharedHandle<Peer> >::const_iterator i =
-        activePeers.begin(), eoi = activePeers.end(); i != eoi; ++i) {
+  const PeerSet& usedPeers = ps->getUsedPeers();
+  for(PeerSet::const_iterator i = usedPeers.begin(), eoi = usedPeers.end();
+      i != eoi; ++i) {
+    if(!(*i)->isActive()) {
+      continue;
+    }
     SharedHandle<Dict> peerEntry = Dict::g();
     peerEntry->put(KEY_PEER_ID, util::torrentPercentEncode((*i)->getPeerId(),
                                                            PEER_ID_LENGTH));

test/DefaultPeerStorageTest.cc

@@ -20,7 +20,6 @@ class DefaultPeerStorageTest:public CppUnit::TestFixture {
   CPPUNIT_TEST(testDeleteUnusedPeer);
   CPPUNIT_TEST(testAddPeer);
   CPPUNIT_TEST(testIsPeerAvailable);
-  CPPUNIT_TEST(testGetActivePeers);
   CPPUNIT_TEST(testCheckoutPeer);
   CPPUNIT_TEST(testReturnPeer);
   CPPUNIT_TEST(testOnErasingPeer);
@@ -43,7 +42,6 @@ public:
   void testDeleteUnusedPeer();
   void testAddPeer();
   void testIsPeerAvailable();
-  void testGetActivePeers();
   void testCheckoutPeer();
   void testReturnPeer();
   void testOnErasingPeer();
@@ -135,33 +133,6 @@ void DefaultPeerStorageTest::testIsPeerAvailable() {
   CPPUNIT_ASSERT(!ps.isPeerAvailable());
 }
 
-void DefaultPeerStorageTest::testGetActivePeers()
-{
-  DefaultPeerStorage ps;
-  {
-    std::vector<SharedHandle<Peer> > peers;
-    ps.getActivePeers(peers);
-    CPPUNIT_ASSERT_EQUAL((size_t)0, peers.size());
-  }
-
-  SharedHandle<Peer> peer1(new Peer("192.168.0.1", 6889));
-  ps.addPeer(peer1);
-  {
-    std::vector<SharedHandle<Peer> > activePeers;
-    ps.getActivePeers(activePeers);
-    CPPUNIT_ASSERT_EQUAL((size_t)0, activePeers.size());
-  }
-  CPPUNIT_ASSERT(ps.checkoutPeer(1));
-  {
-    peer1->allocateSessionResource(1024*1024, 1024*1024*10);
-
-    std::vector<SharedHandle<Peer> > activePeers;
-    ps.getActivePeers(activePeers);
-
-    CPPUNIT_ASSERT_EQUAL((size_t)1, activePeers.size());
-  }
-}
-
 void DefaultPeerStorageTest::testCheckoutPeer()
 {
   DefaultPeerStorage ps;

test/MockPeerStorage.h

@@ -12,7 +12,7 @@ namespace aria2 {
 class MockPeerStorage : public PeerStorage {
 private:
   std::deque<SharedHandle<Peer> > unusedPeers;
-  std::deque<SharedHandle<Peer> > usedPeers;
+  PeerSet usedPeers;
   std::deque<SharedHandle<Peer> > droppedPeers;
   std::vector<SharedHandle<Peer> > activePeers;
   int numChokeExecuted_;
@@ -57,10 +57,15 @@ public:
     this->activePeers = activePeers;
   }
 
-  virtual void getActivePeers(std::vector<SharedHandle<Peer> >& peers) {
+  void getActivePeers(std::vector<SharedHandle<Peer> >& peers) {
     peers.insert(peers.end(), activePeers.begin(), activePeers.end());
   }
 
+  virtual const PeerSet& getUsedPeers()
+  {
+    return usedPeers;
+  }
+
   virtual bool isBadPeer(const std::string& ipaddr)
   {
     return false;