Browse Source

Use RequestSlot as pointer to avoid copying

Tatsuhiro Tsujikawa 12 years ago
parent
commit
03ae308faa

+ 3 - 3
src/BtMessageDispatcher.h

@@ -79,12 +79,12 @@ public:
 
   virtual bool isOutstandingRequest(size_t index, size_t blockIndex) = 0;
 
-  virtual RequestSlot getOutstandingRequest
+  virtual const RequestSlot* getOutstandingRequest
   (size_t index, int32_t begin, int32_t length) = 0;
 
-  virtual void removeOutstandingRequest(const RequestSlot& slot) = 0;
+  virtual void removeOutstandingRequest(const RequestSlot* slot) = 0;
 
-  virtual void addOutstandingRequest(const RequestSlot& slot) = 0;
+  virtual void addOutstandingRequest(std::unique_ptr<RequestSlot> slot) = 0;
 
   virtual size_t countOutstandingUpload() = 0;
 };

+ 5 - 5
src/BtPieceMessage.cc

@@ -103,11 +103,11 @@ void BtPieceMessage::doReceivedAction()
   if(isMetadataGetMode()) {
     return;
   }
-  RequestSlot slot = getBtMessageDispatcher()->getOutstandingRequest
+  auto slot = getBtMessageDispatcher()->getOutstandingRequest
     (index_, begin_, blockLength_);
   getPeer()->updateDownloadLength(blockLength_);
   downloadContext_->updateDownloadLength(blockLength_);
-  if(!RequestSlot::isNull(slot)) {
+  if(slot) {
     getPeer()->snubbing(false);
     std::shared_ptr<Piece> piece = getPieceStorage()->getPiece(index_);
     int64_t offset =
@@ -118,8 +118,8 @@ void BtPieceMessage::doReceivedAction()
                      begin_,
                      blockLength_,
                      static_cast<int64_t>(offset),
-                     static_cast<unsigned long>(slot.getBlockIndex())));
-    if(piece->hasBlock(slot.getBlockIndex())) {
+                     static_cast<unsigned long>(slot->getBlockIndex())));
+    if(piece->hasBlock(slot->getBlockIndex())) {
       A2_LOG_DEBUG("Already have this block.");
       return;
     }
@@ -134,7 +134,7 @@ void BtPieceMessage::doReceivedAction()
       getPieceStorage()->getDiskAdaptor()->writeData(data_+9, blockLength_,
                                                      offset);
     }
-    piece->completeBlock(slot.getBlockIndex());
+    piece->completeBlock(slot->getBlockIndex());
     A2_LOG_DEBUG(fmt(MSG_PIECE_BITFIELD, getCuid(),
                      util::toHex(piece->getBitfield(),
                                  piece->getBitfieldLength()).c_str()));

+ 4 - 5
src/BtRejectMessage.cc

@@ -65,13 +65,12 @@ void BtRejectMessage::doReceivedAction()
   }
   // TODO Current implementation does not close a connection even if
   // a request for this reject message has never sent.
-  RequestSlot slot =
-    getBtMessageDispatcher()->getOutstandingRequest
+  auto slot = getBtMessageDispatcher()->getOutstandingRequest
     (getIndex(), getBegin(), getLength());
-  if(RequestSlot::isNull(slot)) {
-    //throw DL_ABORT_EX("reject received, but it is not in the request slots.");
-  } else {
+  if(slot) {
     getBtMessageDispatcher()->removeOutstandingRequest(slot);
+  } else {
+    //throw DL_ABORT_EX("reject received, but it is not in the request slots.");
   }
 
 }

+ 3 - 3
src/BtRequestMessage.cc

@@ -79,9 +79,9 @@ void BtRequestMessage::doReceivedAction()
 
 void BtRequestMessage::onQueued()
 {
-  RequestSlot requestSlot(getIndex(), getBegin(), getLength(), blockIndex_,
-                          getPieceStorage()->getPiece(getIndex()));
-  getBtMessageDispatcher()->addOutstandingRequest(requestSlot);
+  getBtMessageDispatcher()->addOutstandingRequest
+    (make_unique<RequestSlot>(getIndex(), getBegin(), getLength(), blockIndex_,
+                              getPieceStorage()->getPiece(getIndex())));
 }
 
 void BtRequestMessage::onAbortOutstandingRequestEvent

+ 74 - 186
src/DefaultBtMessageDispatcher.cc

@@ -143,40 +143,33 @@ void DefaultBtMessageDispatcher::doCancelSendingPieceAction
 
 namespace {
 void abortOutstandingRequest
-(const RequestSlot& slot, const std::shared_ptr<Piece>& piece, cuid_t cuid)
+(const RequestSlot* slot, const std::shared_ptr<Piece>& piece, cuid_t cuid)
 {
   A2_LOG_DEBUG(fmt(MSG_DELETING_REQUEST_SLOT,
                    cuid,
-                   static_cast<unsigned long>(slot.getIndex()),
-                   slot.getBegin(),
-                   static_cast<unsigned long>(slot.getBlockIndex())));
-  piece->cancelBlock(slot.getBlockIndex());
+                   static_cast<unsigned long>(slot->getIndex()),
+                   slot->getBegin(),
+                   static_cast<unsigned long>(slot->getBlockIndex())));
+  piece->cancelBlock(slot->getBlockIndex());
 }
 } // namespace
 
-namespace {
-struct FindRequestSlotByIndex {
-  size_t index;
-  FindRequestSlotByIndex(size_t index) : index(index) {}
-  bool operator()(const RequestSlot& slot) const
-  {
-    return slot.getIndex() == index;
-  }
-};
-} // namespace
-
 // localhost cancels outstanding download requests to the peer.
 void DefaultBtMessageDispatcher::doAbortOutstandingRequestAction
 (const std::shared_ptr<Piece>& piece) {
-  for(std::deque<RequestSlot>::iterator itr = requestSlots_.begin(),
-        eoi = requestSlots_.end(); itr != eoi; ++itr) {
-    if((*itr).getIndex() == piece->getIndex()) {
-      abortOutstandingRequest(*itr, piece, cuid_);
+  for(auto& slot : requestSlots_) {
+    if(slot->getIndex() == piece->getIndex()) {
+      abortOutstandingRequest(slot.get(), piece, cuid_);
     }
   }
-  requestSlots_.erase(std::remove_if(requestSlots_.begin(), requestSlots_.end(),
-                                     FindRequestSlotByIndex(piece->getIndex())),
-                      requestSlots_.end());
+  requestSlots_.erase
+    (std::remove_if(std::begin(requestSlots_),
+                    std::end(requestSlots_),
+                    [&](const std::unique_ptr<RequestSlot>& slot)
+                    {
+                      return slot->getIndex() == piece->getIndex();
+                    }),
+     std::end(requestSlots_));
 
   BtAbortOutstandingRequestEvent event(piece);
 
@@ -187,60 +180,26 @@ void DefaultBtMessageDispatcher::doAbortOutstandingRequestAction
   }
 }
 
-namespace {
-class ProcessChokedRequestSlot {
-private:
-  cuid_t cuid_;
-  std::shared_ptr<Peer> peer_;
-  PieceStorage* pieceStorage_;
-public:
-  ProcessChokedRequestSlot
-  (cuid_t cuid, const std::shared_ptr<Peer>& peer, PieceStorage* pieceStorage)
-    : cuid_(cuid),
-      peer_(peer),
-      pieceStorage_(pieceStorage)
-  {}
-
-  void operator()(const RequestSlot& slot) const
-  {
-    if(!peer_->isInPeerAllowedIndexSet(slot.getIndex())) {
+// localhost received choke message from the peer.
+void DefaultBtMessageDispatcher::doChokedAction()
+{
+  for(auto& slot : requestSlots_) {
+    if(!peer_->isInPeerAllowedIndexSet(slot->getIndex())) {
       A2_LOG_DEBUG(fmt(MSG_DELETING_REQUEST_SLOT_CHOKED,
                        cuid_,
-                       static_cast<unsigned long>(slot.getIndex()),
-                       slot.getBegin(),
-                       static_cast<unsigned long>(slot.getBlockIndex())));
-      std::shared_ptr<Piece> piece = pieceStorage_->getPiece(slot.getIndex());
-      piece->cancelBlock(slot.getBlockIndex());
+                       static_cast<unsigned long>(slot->getIndex()),
+                       slot->getBegin(),
+                       static_cast<unsigned long>(slot->getBlockIndex())));
+      slot->getPiece()->cancelBlock(slot->getBlockIndex());
     }
   }
-
-};
-} // namespace
-
-namespace {
-class FindChokedRequestSlot {
-private:
-  std::shared_ptr<Peer> peer_;
-public:
-  FindChokedRequestSlot(const std::shared_ptr<Peer>& peer):
-    peer_(peer) {}
-
-  bool operator()(const RequestSlot& slot) const
-  {
-    return !peer_->isInPeerAllowedIndexSet(slot.getIndex());
-  }
-};
-} // namespace
-
-// localhost received choke message from the peer.
-void DefaultBtMessageDispatcher::doChokedAction()
-{
-  std::for_each(requestSlots_.begin(), requestSlots_.end(),
-                ProcessChokedRequestSlot(cuid_, peer_, pieceStorage_));
-
-  requestSlots_.erase(std::remove_if(requestSlots_.begin(), requestSlots_.end(),
-                                     FindChokedRequestSlot(peer_)),
-                      requestSlots_.end());
+  requestSlots_.erase
+    (std::remove_if(std::begin(requestSlots_), std::end(requestSlots_),
+                    [&](const std::unique_ptr<RequestSlot>& slot)
+                    {
+                      return !peer_->isInPeerAllowedIndexSet(slot->getIndex());
+                    }),
+     std::end(requestSlots_));
 }
 
 // localhost dispatched choke message to the peer.
@@ -255,93 +214,38 @@ void DefaultBtMessageDispatcher::doChokingAction()
   }
 }
 
-namespace {
-class ProcessStaleRequestSlot {
-private:
-  cuid_t cuid_;
-  std::shared_ptr<Peer> peer_;
-  PieceStorage* pieceStorage_;
-  BtMessageDispatcher* messageDispatcher_;
-  BtMessageFactory* messageFactory_;
-  time_t requestTimeout_;
-public:
-  ProcessStaleRequestSlot
-  (cuid_t cuid, const std::shared_ptr<Peer>& peer,
-   PieceStorage* pieceStorage,
-   BtMessageDispatcher* dispatcher,
-   BtMessageFactory* factory,
-   time_t requestTimeout)
-    : cuid_(cuid),
-      peer_(peer),
-      pieceStorage_(pieceStorage),
-      messageDispatcher_(dispatcher),
-      messageFactory_(factory),
-      requestTimeout_(requestTimeout)
-  {}
-
-  void operator()(const RequestSlot& slot)
-  {
-    if(slot.isTimeout(requestTimeout_)) {
+
+void DefaultBtMessageDispatcher::checkRequestSlotAndDoNecessaryThing()
+{
+  for(auto& slot : requestSlots_) {
+    if(slot->isTimeout(requestTimeout_)) {
       A2_LOG_DEBUG(fmt(MSG_DELETING_REQUEST_SLOT_TIMEOUT,
                        cuid_,
-                       static_cast<unsigned long>(slot.getIndex()),
-                       slot.getBegin(),
-                       static_cast<unsigned long>(slot.getBlockIndex())));
-      slot.getPiece()->cancelBlock(slot.getBlockIndex());
+                       static_cast<unsigned long>(slot->getIndex()),
+                       slot->getBegin(),
+                       static_cast<unsigned long>(slot->getBlockIndex())));
+      slot->getPiece()->cancelBlock(slot->getBlockIndex());
       peer_->snubbing(true);
-    } else if(slot.getPiece()->hasBlock(slot.getBlockIndex())) {
+    } else if(slot->getPiece()->hasBlock(slot->getBlockIndex())) {
       A2_LOG_DEBUG(fmt(MSG_DELETING_REQUEST_SLOT_ACQUIRED,
                        cuid_,
-                       static_cast<unsigned long>(slot.getIndex()),
-                       slot.getBegin(),
-                       static_cast<unsigned long>(slot.getBlockIndex())));
-      messageDispatcher_->addMessageToQueue
-        (messageFactory_->createCancelMessage(slot.getIndex(),
-                                              slot.getBegin(),
-                                              slot.getLength()));
-    }
-  }
-};
-} // namespace
-
-namespace {
-class FindStaleRequestSlot {
-private:
-  PieceStorage* pieceStorage_;
-  time_t requestTimeout_;
-public:
-  FindStaleRequestSlot(PieceStorage* pieceStorage, time_t requestTimeout)
-    : pieceStorage_(pieceStorage),
-      requestTimeout_(requestTimeout) {}
-
-  bool operator()(const RequestSlot& slot)
-  {
-    if(slot.isTimeout(requestTimeout_)) {
-      return true;
-    } else {
-      if(slot.getPiece()->hasBlock(slot.getBlockIndex())) {
-        return true;
-      } else {
-        return false;
-      }
+                       static_cast<unsigned long>(slot->getIndex()),
+                       slot->getBegin(),
+                       static_cast<unsigned long>(slot->getBlockIndex())));
+      addMessageToQueue
+        (messageFactory_->createCancelMessage(slot->getIndex(),
+                                              slot->getBegin(),
+                                              slot->getLength()));
     }
   }
-};
-} // namespace
-
-void DefaultBtMessageDispatcher::checkRequestSlotAndDoNecessaryThing()
-{
-  std::for_each(requestSlots_.begin(), requestSlots_.end(),
-                ProcessStaleRequestSlot(cuid_,
-                                        peer_,
-                                        pieceStorage_,
-                                        this,
-                                        messageFactory_,
-                                        requestTimeout_));
-  requestSlots_.erase(std::remove_if(requestSlots_.begin(), requestSlots_.end(),
-                                     FindStaleRequestSlot(pieceStorage_,
-                                                          requestTimeout_)),
-                      requestSlots_.end());
+  requestSlots_.erase
+    (std::remove_if(std::begin(requestSlots_), std::end(requestSlots_),
+                    [&](const std::unique_ptr<RequestSlot>& slot)
+                    {
+                      return slot->isTimeout(requestTimeout_) ||
+                        slot->getPiece()->hasBlock(slot->getBlockIndex());
+                    }),
+     std::end(requestSlots_));
 }
 
 bool DefaultBtMessageDispatcher::isSendingInProgress()
@@ -349,63 +253,47 @@ bool DefaultBtMessageDispatcher::isSendingInProgress()
   return peerConnection_->getBufferEntrySize();
 }
 
-namespace {
-class BlockIndexLess {
-public:
-  bool operator()(const RequestSlot& lhs, const RequestSlot& rhs) const
-  {
-    if(lhs.getIndex() == rhs.getIndex()) {
-      return lhs.getBlockIndex() < rhs.getBlockIndex();
-    } else {
-      return lhs.getIndex() < rhs.getIndex();
-    }
-  }
-};
-} // namespace
-
 bool DefaultBtMessageDispatcher::isOutstandingRequest
 (size_t index, size_t blockIndex) {
-  for(std::deque<RequestSlot>::const_iterator itr = requestSlots_.begin(),
-        eoi = requestSlots_.end(); itr != eoi; ++itr) {
-    if((*itr).getIndex() == index && (*itr).getBlockIndex() == blockIndex) {
+  for(auto& slot : requestSlots_) {
+    if(slot->getIndex() == index && slot->getBlockIndex() == blockIndex) {
       return true;
     }
   }
   return false;
 }
 
-RequestSlot
+const RequestSlot*
 DefaultBtMessageDispatcher::getOutstandingRequest
 (size_t index, int32_t begin, int32_t length)
 {
-  for(std::deque<RequestSlot>::const_iterator itr = requestSlots_.begin(),
-        eoi = requestSlots_.end(); itr != eoi; ++itr) {
-    if((*itr).getIndex() == index &&
-       (*itr).getBegin() == begin &&
-       (*itr).getLength() == length) {
-      return *itr;
+  for(auto& slot : requestSlots_) {
+    if(slot->getIndex() == index &&
+       slot->getBegin() == begin &&
+       slot->getLength() == length) {
+      return slot.get();
     }
   }
-  return RequestSlot::nullSlot;
+  return nullptr;
 }
 
 void DefaultBtMessageDispatcher::removeOutstandingRequest
-(const RequestSlot& slot)
+(const RequestSlot* slot)
 {
-  for(std::deque<RequestSlot>::iterator itr = requestSlots_.begin(),
-        eoi = requestSlots_.end(); itr != eoi; ++itr) {
-    if(*itr == slot) {
-      abortOutstandingRequest(*itr, slot.getPiece(), cuid_);
-      requestSlots_.erase(itr);
+  for(auto i = std::begin(requestSlots_), eoi = std::end(requestSlots_);
+      i != eoi; ++i) {
+    if(*(*i) == *slot) {
+      abortOutstandingRequest((*i).get(), (*i)->getPiece(), cuid_);
+      requestSlots_.erase(i);
       break;
     }
   }
 }
 
 void DefaultBtMessageDispatcher::addOutstandingRequest
-(const RequestSlot& slot)
+(std::unique_ptr<RequestSlot> slot)
 {
-  requestSlots_.push_back(slot);
+  requestSlots_.push_back(std::move(slot));
 }
 
 size_t DefaultBtMessageDispatcher::countOutstandingUpload()

+ 5 - 5
src/DefaultBtMessageDispatcher.h

@@ -58,7 +58,7 @@ class DefaultBtMessageDispatcher : public BtMessageDispatcher {
 private:
   cuid_t cuid_;
   std::deque<std::shared_ptr<BtMessage> > messageQueue_;
-  std::deque<RequestSlot> requestSlots_;
+  std::deque<std::unique_ptr<RequestSlot>> requestSlots_;
   DownloadContext* downloadContext_;
   PeerStorage* peerStorage_;
   PieceStorage* pieceStorage_;
@@ -108,12 +108,12 @@ public:
 
   virtual bool isOutstandingRequest(size_t index, size_t blockIndex);
 
-  virtual RequestSlot getOutstandingRequest
+  virtual const RequestSlot* getOutstandingRequest
   (size_t index, int32_t begin, int32_t length);
 
-  virtual void removeOutstandingRequest(const RequestSlot& slot);
+  virtual void removeOutstandingRequest(const RequestSlot* slot);
 
-  virtual void addOutstandingRequest(const RequestSlot& requestSlot);
+  virtual void addOutstandingRequest(std::unique_ptr<RequestSlot> requestSlot);
 
   virtual size_t countOutstandingUpload();
 
@@ -122,7 +122,7 @@ public:
     return messageQueue_;
   }
 
-  const std::deque<RequestSlot>& getRequestSlots() const
+  const std::deque<std::unique_ptr<RequestSlot>>& getRequestSlots() const
   {
     return requestSlots_;
   }

+ 0 - 7
src/RequestSlot.cc

@@ -36,8 +36,6 @@
 
 namespace aria2 {
 
-RequestSlot RequestSlot::nullSlot = RequestSlot();
-
 void RequestSlot::setDispatchedTime(time_t sec) {
   dispatchedTime_.reset(sec);
 }
@@ -46,9 +44,4 @@ bool RequestSlot::isTimeout(time_t timeoutSec) const {
   return dispatchedTime_.difference(global::wallclock()) >= timeoutSec;
 }
 
-bool RequestSlot::isNull(const RequestSlot& requestSlot) {
-  return requestSlot.index_ == 0 && requestSlot.begin_ == 0&&
-    requestSlot.length_ == 0;
-}
-
 } // namespace aria2

+ 16 - 53
src/RequestSlot.h

@@ -43,60 +43,14 @@
 namespace aria2 {
 
 class RequestSlot {
-private:
-  Timer dispatchedTime_;
-  size_t index_;
-  int32_t begin_;
-  int32_t length_;
-  size_t blockIndex_;
-
-  // This is the piece whose index is index of this RequestSlot has.
-  // To detect duplicate RequestSlot, we have to find the piece using
-  // PieceStorage::getPiece() repeatedly. It turns out that this process
-  // takes time(about 1.7% of processing time). To reduce it, we put piece here
-  // at the construction of RequestSlot as a cache.
-  std::shared_ptr<Piece> piece_;
-
-  // inlined for performance reason
-  void copy(const RequestSlot& requestSlot)
-  {
-    dispatchedTime_ = requestSlot.dispatchedTime_;
-    index_ = requestSlot.index_;
-    begin_ = requestSlot.begin_;
-    length_ = requestSlot.length_;
-    blockIndex_ = requestSlot.blockIndex_;
-    piece_ = requestSlot.piece_;
-  }
 public:
-
   RequestSlot(size_t index, int32_t begin, int32_t length, size_t blockIndex,
-              const std::shared_ptr<Piece>& piece = std::shared_ptr<Piece>()):
-    dispatchedTime_(global::wallclock()),
-    index_(index), begin_(begin), length_(length), blockIndex_(blockIndex),
-    piece_(piece) {}
-
-  RequestSlot(const RequestSlot& requestSlot):
-    dispatchedTime_(requestSlot.dispatchedTime_),
-    index_(requestSlot.index_),
-    begin_(requestSlot.begin_),
-    length_(requestSlot.length_),
-    blockIndex_(requestSlot.blockIndex_),
-    piece_(requestSlot.piece_) {}
-
-  RequestSlot():dispatchedTime_(0), index_(0), begin_(0), length_(0),
-                blockIndex_(0)
+              const std::shared_ptr<Piece>& piece = std::shared_ptr<Piece>())
+    : dispatchedTime_(global::wallclock()),
+      index_(index), begin_(begin), length_(length), blockIndex_(blockIndex),
+      piece_(piece)
   {}
 
-  ~RequestSlot() {}
-
-  RequestSlot& operator=(const RequestSlot& requestSlot)
-  {
-    if(this != &requestSlot) {
-      copy(requestSlot);
-    }
-    return *this;
-  }
-
   bool operator==(const RequestSlot& requestSlot) const
   {
     return index_ == requestSlot.index_ && begin_ == requestSlot.begin_
@@ -137,10 +91,19 @@ public:
   {
     return piece_;
   }
+private:
+  Timer dispatchedTime_;
+  size_t index_;
+  int32_t begin_;
+  int32_t length_;
+  size_t blockIndex_;
 
-  static RequestSlot nullSlot;
-
-  static bool isNull(const RequestSlot& requestSlot);
+  // This is the piece whose index is index of this RequestSlot has.
+  // To detect duplicate RequestSlot, we have to find the piece using
+  // PieceStorage::getPiece() repeatedly. It turns out that this process
+  // takes time(about 1.7% of processing time). To reduce it, we put piece here
+  // at the construction of RequestSlot as a cache.
+  std::shared_ptr<Piece> piece_;
 };
 
 } // namespace aria2

+ 25 - 31
test/BtRejectMessageTest.cc

@@ -33,29 +33,30 @@ public:
 
   class MockBtMessageDispatcher2 : public MockBtMessageDispatcher {
   public:
-    RequestSlot slot;
-  public:
-    MockBtMessageDispatcher2():slot(RequestSlot::nullSlot) {}
+    std::unique_ptr<RequestSlot> slot;
 
-    void setRequestSlot(const RequestSlot& slot) {
-      this->slot = slot;
+    void setRequestSlot(std::unique_ptr<RequestSlot> s)
+    {
+      slot = std::move(s);
     }
 
-    virtual RequestSlot getOutstandingRequest
-    (size_t index, int32_t begin, int32_t length) {
-      if(slot.getIndex() == index && slot.getBegin() == begin &&
-         slot.getLength() == length) {
-        return slot;
+    virtual const RequestSlot* getOutstandingRequest
+    (size_t index, int32_t begin, int32_t length) override {
+      if(slot &&
+         slot->getIndex() == index && slot->getBegin() == begin &&
+         slot->getLength() == length) {
+        return slot.get();
       } else {
-        return RequestSlot::nullSlot;
+        return nullptr;
       }
     }
 
-    virtual void removeOutstandingRequest(const RequestSlot& slot) {
-      if(this->slot.getIndex() == slot.getIndex() &&
-         this->slot.getBegin() == slot.getBegin() &&
-         this->slot.getLength() == slot.getLength()) {
-        this->slot = RequestSlot::nullSlot;
+    virtual void removeOutstandingRequest(const RequestSlot* s) override
+    {
+      if(slot->getIndex() == s->getIndex() &&
+         slot->getBegin() == s->getBegin() &&
+         slot->getLength() == s->getLength()) {
+        slot.reset();
       }
     }
   };
@@ -131,39 +132,32 @@ void BtRejectMessageTest::testCreateMessage() {
 
 void BtRejectMessageTest::testDoReceivedAction() {
   peer->setFastExtensionEnabled(true);
-  RequestSlot slot(1, 16, 32, 2);
-  dispatcher->setRequestSlot(slot);
+  dispatcher->setRequestSlot(make_unique<RequestSlot>(1, 16, 32, 2));
 
-  CPPUNIT_ASSERT
-    (!RequestSlot::isNull(dispatcher->getOutstandingRequest(1, 16, 32)));
+  CPPUNIT_ASSERT(dispatcher->getOutstandingRequest(1, 16, 32));
 
   msg->doReceivedAction();
 
-  CPPUNIT_ASSERT
-    (RequestSlot::isNull(dispatcher->getOutstandingRequest(1, 16, 32)));
+  CPPUNIT_ASSERT(!dispatcher->getOutstandingRequest(1, 16, 32));
 }
 
 void BtRejectMessageTest::testDoReceivedActionNoMatch() {
   peer->setFastExtensionEnabled(true);
-  RequestSlot slot(2, 16, 32, 2);
-  dispatcher->setRequestSlot(slot);
+  dispatcher->setRequestSlot(make_unique<RequestSlot>(2, 16, 32, 2));
 
-  CPPUNIT_ASSERT
-    (!RequestSlot::isNull(dispatcher->getOutstandingRequest(2, 16, 32)));
+  CPPUNIT_ASSERT(dispatcher->getOutstandingRequest(2, 16, 32));
 
   msg->doReceivedAction();
 
-  CPPUNIT_ASSERT
-    (!RequestSlot::isNull(dispatcher->getOutstandingRequest(2, 16, 32)));
+  CPPUNIT_ASSERT(dispatcher->getOutstandingRequest(2, 16, 32));
 
 }
 
 void BtRejectMessageTest::testDoReceivedActionFastExtensionDisabled() {
   RequestSlot slot(1, 16, 32, 2);
-  dispatcher->setRequestSlot(slot);
+  dispatcher->setRequestSlot(make_unique<RequestSlot>(1, 16, 32, 2));
 
-  CPPUNIT_ASSERT
-    (!RequestSlot::isNull(dispatcher->getOutstandingRequest(1, 16, 32)));
+  CPPUNIT_ASSERT(dispatcher->getOutstandingRequest(1, 16, 32));
   try {
     msg->doReceivedAction();
     CPPUNIT_FAIL("exception must be thrown.");

+ 40 - 43
test/DefaultBtMessageDispatcherTest.cc

@@ -248,9 +248,7 @@ void DefaultBtMessageDispatcherTest::testDoCancelSendingPieceAction() {
 int MY_PIECE_LENGTH = 16*1024;
 
 void DefaultBtMessageDispatcherTest::testCheckRequestSlotAndDoNecessaryThing() {
-  std::shared_ptr<Piece> piece(new Piece(0, MY_PIECE_LENGTH));
-  RequestSlot slot(0, 0, MY_PIECE_LENGTH, 0, piece);
-
+  auto piece = std::make_shared<Piece>(0, MY_PIECE_LENGTH);
   size_t index;
   CPPUNIT_ASSERT(piece->getMissingUnusedBlockIndex(index));
   CPPUNIT_ASSERT_EQUAL((size_t)0, index);
@@ -260,7 +258,8 @@ void DefaultBtMessageDispatcherTest::testCheckRequestSlotAndDoNecessaryThing() {
 
   btMessageDispatcher->setRequestTimeout(60);
   btMessageDispatcher->setPieceStorage(pieceStorage.get());
-  btMessageDispatcher->addOutstandingRequest(slot);
+  btMessageDispatcher->addOutstandingRequest
+    (make_unique<RequestSlot>(0, 0, MY_PIECE_LENGTH, 0, piece));
 
   btMessageDispatcher->checkRequestSlotAndDoNecessaryThing();
 
@@ -270,12 +269,9 @@ void DefaultBtMessageDispatcherTest::testCheckRequestSlotAndDoNecessaryThing() {
                        btMessageDispatcher->getRequestSlots().size());
 }
 
-void DefaultBtMessageDispatcherTest::testCheckRequestSlotAndDoNecessaryThing_timeout() {
-  std::shared_ptr<Piece> piece(new Piece(0, MY_PIECE_LENGTH));
-  RequestSlot slot(0, 0, MY_PIECE_LENGTH, 0, piece);
-  // make this slot timeout
-  slot.setDispatchedTime(0);
-
+void DefaultBtMessageDispatcherTest::
+testCheckRequestSlotAndDoNecessaryThing_timeout() {
+  auto piece = std::make_shared<Piece>(0, MY_PIECE_LENGTH);
   size_t index;
   CPPUNIT_ASSERT(piece->getMissingUnusedBlockIndex(index));
   CPPUNIT_ASSERT_EQUAL((size_t)0, index);
@@ -285,8 +281,10 @@ void DefaultBtMessageDispatcherTest::testCheckRequestSlotAndDoNecessaryThing_tim
 
   btMessageDispatcher->setRequestTimeout(60);
   btMessageDispatcher->setPieceStorage(pieceStorage.get());
-  btMessageDispatcher->addOutstandingRequest(slot);
-
+  auto slot = make_unique<RequestSlot>(0, 0, MY_PIECE_LENGTH, 0, piece);
+  // make this slot timeout
+  slot->setDispatchedTime(0);
+  btMessageDispatcher->addOutstandingRequest(std::move(slot));
   btMessageDispatcher->checkRequestSlotAndDoNecessaryThing();
 
   CPPUNIT_ASSERT_EQUAL((size_t)0,
@@ -297,18 +295,17 @@ void DefaultBtMessageDispatcherTest::testCheckRequestSlotAndDoNecessaryThing_tim
   CPPUNIT_ASSERT_EQUAL(true, peer->snubbing());
 }
 
-void DefaultBtMessageDispatcherTest::testCheckRequestSlotAndDoNecessaryThing_completeBlock() {
-  std::shared_ptr<Piece> piece(new Piece(0, MY_PIECE_LENGTH));
+void DefaultBtMessageDispatcherTest::
+testCheckRequestSlotAndDoNecessaryThing_completeBlock() {
+  auto piece = std::make_shared<Piece>(0, MY_PIECE_LENGTH);
   piece->completeBlock(0);
-
-  RequestSlot slot(0, 0, MY_PIECE_LENGTH, 0, piece);
-
   auto pieceStorage = make_unique<MockPieceStorage2>();
   pieceStorage->setPiece(piece);
 
   btMessageDispatcher->setRequestTimeout(60);
   btMessageDispatcher->setPieceStorage(pieceStorage.get());
-  btMessageDispatcher->addOutstandingRequest(slot);
+  btMessageDispatcher->addOutstandingRequest
+    (make_unique<RequestSlot>(0, 0, MY_PIECE_LENGTH, 0, piece));
 
   btMessageDispatcher->checkRequestSlotAndDoNecessaryThing();
 
@@ -319,15 +316,15 @@ void DefaultBtMessageDispatcherTest::testCheckRequestSlotAndDoNecessaryThing_com
 }
 
 void DefaultBtMessageDispatcherTest::testCountOutstandingRequest() {
-  RequestSlot slot(0, 0, MY_PIECE_LENGTH, 0);
-  btMessageDispatcher->addOutstandingRequest(slot);
+  btMessageDispatcher->addOutstandingRequest
+    (make_unique<RequestSlot>(0, 0, MY_PIECE_LENGTH, 0));
   CPPUNIT_ASSERT_EQUAL((size_t)1,
                        btMessageDispatcher->countOutstandingRequest());
 }
 
 void DefaultBtMessageDispatcherTest::testIsOutstandingRequest() {
-  RequestSlot slot(0, 0, MY_PIECE_LENGTH, 0);
-  btMessageDispatcher->addOutstandingRequest(slot);
+  btMessageDispatcher->addOutstandingRequest
+    (make_unique<RequestSlot>(0, 0, MY_PIECE_LENGTH, 0));
 
   CPPUNIT_ASSERT(btMessageDispatcher->isOutstandingRequest(0, 0));
   CPPUNIT_ASSERT(!btMessageDispatcher->isOutstandingRequest(0, 1));
@@ -336,42 +333,42 @@ void DefaultBtMessageDispatcherTest::testIsOutstandingRequest() {
 }
 
 void DefaultBtMessageDispatcherTest::testGetOutstandingRequest() {
-  RequestSlot slot(1, 1024, 16*1024, 10);
-  btMessageDispatcher->addOutstandingRequest(slot);
+  btMessageDispatcher->addOutstandingRequest
+    (make_unique<RequestSlot>(1, 1024, 16*1024, 10));
 
-  RequestSlot s2 = btMessageDispatcher->getOutstandingRequest(1, 1024, 16*1024);
-  CPPUNIT_ASSERT(!RequestSlot::isNull(s2));
+  CPPUNIT_ASSERT(btMessageDispatcher->getOutstandingRequest(1, 1024, 16*1024));
 
-  RequestSlot s3 = btMessageDispatcher->getOutstandingRequest(1, 1024, 17*1024);
-  CPPUNIT_ASSERT(RequestSlot::isNull(s3));
+  CPPUNIT_ASSERT(!btMessageDispatcher->
+                 getOutstandingRequest(1, 1024, 17*1024));
 
-  RequestSlot s4 =
-    btMessageDispatcher->getOutstandingRequest(1, 2*1024, 16*1024);
-  CPPUNIT_ASSERT(RequestSlot::isNull(s4));
+  CPPUNIT_ASSERT(!btMessageDispatcher->
+                 getOutstandingRequest(1, 2*1024, 16*1024));
 
-  RequestSlot s5 = btMessageDispatcher->getOutstandingRequest(2, 1024, 16*1024);
-  CPPUNIT_ASSERT(RequestSlot::isNull(s5));
+  CPPUNIT_ASSERT(!btMessageDispatcher->
+                 getOutstandingRequest(2, 1024, 16*1024));
 }
 
 void DefaultBtMessageDispatcherTest::testRemoveOutstandingRequest() {
-  std::shared_ptr<Piece> piece(new Piece(1, 1024*1024));
+  auto piece = std::make_shared<Piece>(1, 1024*1024);
   size_t blockIndex = 0;
   CPPUNIT_ASSERT(piece->getMissingUnusedBlockIndex(blockIndex));
   uint32_t begin = blockIndex*piece->getBlockLength();
   size_t length = piece->getBlockLength(blockIndex);
-  RequestSlot slot(piece->getIndex(), begin, length, blockIndex, piece);
-  btMessageDispatcher->addOutstandingRequest(slot);
-
-  RequestSlot s2 = btMessageDispatcher->getOutstandingRequest
-    (piece->getIndex(), begin, length);
-  CPPUNIT_ASSERT(!RequestSlot::isNull(s2));
+  RequestSlot slot;
+  btMessageDispatcher->addOutstandingRequest
+    (make_unique<RequestSlot>(piece->getIndex(), begin, length, blockIndex,
+                              piece));
+
+  auto s2 = btMessageDispatcher->getOutstandingRequest(piece->getIndex(),
+                                                       begin, length);
+  CPPUNIT_ASSERT(s2);
   CPPUNIT_ASSERT(piece->isBlockUsed(blockIndex));
 
   btMessageDispatcher->removeOutstandingRequest(s2);
 
-  RequestSlot s3 = btMessageDispatcher->getOutstandingRequest
-    (piece->getIndex(), begin, length);
-  CPPUNIT_ASSERT(RequestSlot::isNull(s3));
+  auto s3 = btMessageDispatcher->getOutstandingRequest(piece->getIndex(),
+                                                       begin, length);
+  CPPUNIT_ASSERT(!s3);
   CPPUNIT_ASSERT(!piece->isBlockUsed(blockIndex));
 }
 

+ 4 - 4
test/MockBtMessageDispatcher.h

@@ -57,14 +57,14 @@ public:
     return false;
   }
 
-  virtual RequestSlot getOutstandingRequest
+  virtual const RequestSlot* getOutstandingRequest
   (size_t index, int32_t begin, int32_t length) {
-    return RequestSlot::nullSlot;
+    return nullptr;
   }
 
-  virtual void removeOutstandingRequest(const RequestSlot& slot) {}
+  virtual void removeOutstandingRequest(const RequestSlot* slot) {}
 
-  virtual void addOutstandingRequest(const RequestSlot& slot) {}
+  virtual void addOutstandingRequest(std::unique_ptr<RequestSlot> slot) {}
 
   virtual size_t countOutstandingUpload()
   {