From 67767e2f61d41dd208b4f3851d50868961db9e37 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 19 Sep 2008 14:11:41 +0000 Subject: [PATCH] 2008-09-19 Tatsuhiro Tsujikawa Fixed the bug that a block in a piece is requested when same block is already requested to the same peer in end game mode. * src/BtRequestFactory.h * src/DefaultBtInteractive.cc * src/DefaultBtRequestFactory.cc * src/DefaultBtRequestFactory.h * src/DefaultPieceStorage.cc * src/DefaultPieceStorage.h * src/PieceStorage.h * src/UnknownLengthPieceStorage.cc * src/UnknownLengthPieceStorage.h * test/DefaultBtRequestFactoryTest.cc * test/DefaultPieceStorageTest.cc * test/MockBtRequestFactory.h * test/MockPieceStorage.h --- ChangeLog | 18 ++++++ src/BtRequestFactory.h | 7 +++ src/DefaultBtInteractive.cc | 13 +++- src/DefaultBtRequestFactory.cc | 7 +++ src/DefaultBtRequestFactory.h | 2 + src/DefaultPieceStorage.cc | 96 +++++++++++++++++++---------- src/DefaultPieceStorage.h | 19 +++++- src/PieceStorage.h | 16 +++++ src/UnknownLengthPieceStorage.cc | 12 ++++ src/UnknownLengthPieceStorage.h | 6 ++ test/DefaultBtRequestFactoryTest.cc | 20 ++++++ test/DefaultPieceStorageTest.cc | 48 +++++++++++++++ test/MockBtRequestFactory.h | 2 + test/MockPieceStorage.h | 12 ++++ 14 files changed, 242 insertions(+), 36 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3608062b..1c04ab75 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2008-09-19 Tatsuhiro Tsujikawa + + Fixed the bug that a block in a piece is requested when same block is + already requested to the same peer in end game mode. + * src/BtRequestFactory.h + * src/DefaultBtInteractive.cc + * src/DefaultBtRequestFactory.cc + * src/DefaultBtRequestFactory.h + * src/DefaultPieceStorage.cc + * src/DefaultPieceStorage.h + * src/PieceStorage.h + * src/UnknownLengthPieceStorage.cc + * src/UnknownLengthPieceStorage.h + * test/DefaultBtRequestFactoryTest.cc + * test/DefaultPieceStorageTest.cc + * test/MockBtRequestFactory.h + * test/MockPieceStorage.h + 2008-09-19 Tatsuhiro Tsujikawa Removed _uploadLength and _downloadLength from PeerSessionResource diff --git a/src/BtRequestFactory.h b/src/BtRequestFactory.h index ae31966d..3e4b2282 100644 --- a/src/BtRequestFactory.h +++ b/src/BtRequestFactory.h @@ -76,6 +76,13 @@ public: */ virtual void createRequestMessagesOnEndGame (std::deque >& requests, size_t max) = 0; + + /** + * Stores the list of index of pieces added using addTargetPiece() into + * indexes. + */ + virtual void getTargetPieceIndexes(std::deque& indexes) const = 0; + }; typedef SharedHandle BtRequestFactoryHandle; diff --git a/src/DefaultBtInteractive.cc b/src/DefaultBtInteractive.cc index ea29c08c..148c0db4 100644 --- a/src/DefaultBtInteractive.cc +++ b/src/DefaultBtInteractive.cc @@ -295,25 +295,32 @@ void DefaultBtInteractive::fillPiece(size_t maxMissingBlock) { if(peer->peerChoking()) { if(peer->isFastExtensionEnabled()) { - + std::deque excludedIndexes; + btRequestFactory->getTargetPieceIndexes(excludedIndexes); while(numMissingBlock < maxMissingBlock) { - PieceHandle piece = pieceStorage->getMissingFastPiece(peer); + SharedHandle piece = + pieceStorage->getMissingFastPiece(peer, excludedIndexes); if(piece.isNull()) { break; } else { btRequestFactory->addTargetPiece(piece); numMissingBlock += piece->countMissingBlock(); + excludedIndexes.push_back(piece->getIndex()); } } } } else { + std::deque excludedIndexes; + btRequestFactory->getTargetPieceIndexes(excludedIndexes); while(numMissingBlock < maxMissingBlock) { - PieceHandle piece = pieceStorage->getMissingPiece(peer); + SharedHandle piece = + pieceStorage->getMissingPiece(peer, excludedIndexes); if(piece.isNull()) { break; } else { btRequestFactory->addTargetPiece(piece); numMissingBlock += piece->countMissingBlock(); + excludedIndexes.push_back(piece->getIndex()); } } } diff --git a/src/DefaultBtRequestFactory.cc b/src/DefaultBtRequestFactory.cc index 616367b4..6227b799 100644 --- a/src/DefaultBtRequestFactory.cc +++ b/src/DefaultBtRequestFactory.cc @@ -210,6 +210,13 @@ size_t DefaultBtRequestFactory::countMissingBlock() CountMissingBlock()).getNumMissingBlock(); } +void DefaultBtRequestFactory::getTargetPieceIndexes +(std::deque& indexes) const +{ + std::transform(pieces.begin(), pieces.end(), std::back_inserter(indexes), + mem_fun_sh(&Piece::getIndex)); +} + std::deque >& DefaultBtRequestFactory::getTargetPieces() { return pieces; diff --git a/src/DefaultBtRequestFactory.h b/src/DefaultBtRequestFactory.h index 84cf701f..5067138d 100644 --- a/src/DefaultBtRequestFactory.h +++ b/src/DefaultBtRequestFactory.h @@ -84,6 +84,8 @@ public: virtual void createRequestMessagesOnEndGame (std::deque >& requests, size_t max); + virtual void getTargetPieceIndexes(std::deque& indexes) const; + std::deque >& getTargetPieces(); void setCuid(int32_t cuid) diff --git a/src/DefaultPieceStorage.cc b/src/DefaultPieceStorage.cc index 36634659..abb48ebe 100644 --- a/src/DefaultPieceStorage.cc +++ b/src/DefaultPieceStorage.cc @@ -87,22 +87,20 @@ bool DefaultPieceStorage::isEndGame() return bitfieldMan->countMissingBlock() <= endGamePieceNum; } -bool DefaultPieceStorage::getMissingPieceIndex(size_t& index, const PeerHandle& peer) +bool DefaultPieceStorage::getMissingPieceIndex(size_t& index, + const unsigned char* bitfield, + size_t& length) { std::deque indexes; bool r; if(isEndGame()) { - r = bitfieldMan->getAllMissingIndexes(indexes, peer->getBitfield(), - peer->getBitfieldLength()); + r = bitfieldMan->getAllMissingIndexes(indexes, bitfield, length); } else { - r = bitfieldMan->getAllMissingUnusedIndexes(indexes, - peer->getBitfield(), - peer->getBitfieldLength()); + r = bitfieldMan->getAllMissingUnusedIndexes(indexes, bitfield, length); } if(r) { // We assume indexes is sorted using comparator less. - _pieceSelector->select(index, indexes); - return true; + return _pieceSelector->select(index, indexes); } else { return false; } @@ -171,45 +169,81 @@ PieceHandle DefaultPieceStorage::findUsedPiece(size_t index) const } } -PieceHandle DefaultPieceStorage::getMissingPiece(const PeerHandle& peer) +SharedHandle DefaultPieceStorage::getMissingPiece +(const unsigned char* bitfield, size_t length) { size_t index; - if(getMissingPieceIndex(index, peer)) { + if(getMissingPieceIndex(index, bitfield, length)) { return checkOutPiece(index); } else { return SharedHandle(); } } -bool DefaultPieceStorage::getMissingFastPieceIndex(size_t& index, - const PeerHandle& peer) +SharedHandle DefaultPieceStorage::getMissingPiece +(const BitfieldMan& bitfield) +{ + return getMissingPiece(bitfield.getBitfield(), bitfield.getBitfieldLength()); +} + +PieceHandle DefaultPieceStorage::getMissingPiece(const SharedHandle& peer) +{ + return getMissingPiece(peer->getBitfield(), peer->getBitfieldLength()); +} + +void DefaultPieceStorage::createFastIndexBitfield +(BitfieldMan& bitfield, const SharedHandle& peer) +{ + for(std::deque::const_iterator itr = + peer->getPeerAllowedIndexSet().begin(); + itr != peer->getPeerAllowedIndexSet().end(); ++itr) { + if(!bitfieldMan->isBitSet(*itr) && peer->hasPiece(*itr)) { + bitfield.setBit(*itr); + } + } +} + +PieceHandle DefaultPieceStorage::getMissingFastPiece +(const SharedHandle& peer) { if(peer->isFastExtensionEnabled() && peer->countPeerAllowedIndexSet() > 0) { BitfieldMan tempBitfield(bitfieldMan->getBlockLength(), bitfieldMan->getTotalLength()); - for(std::deque::const_iterator itr = peer->getPeerAllowedIndexSet().begin(); - itr != peer->getPeerAllowedIndexSet().end(); itr++) { - if(!bitfieldMan->isBitSet(*itr) && peer->hasPiece(*itr)) { - tempBitfield.setBit(*itr); - } - } - if(isEndGame()) { - return bitfieldMan->getMissingIndex(index, tempBitfield.getBitfield(), - tempBitfield.getBitfieldLength()); - } else { - return bitfieldMan->getMissingUnusedIndex(index, - tempBitfield.getBitfield(), - tempBitfield.getBitfieldLength()); - } + createFastIndexBitfield(tempBitfield, peer); + return getMissingPiece(tempBitfield); + } else { + return SharedHandle(); } - return false; } -PieceHandle DefaultPieceStorage::getMissingFastPiece(const PeerHandle& peer) +static void unsetExcludedIndexes(BitfieldMan& bitfield, + const std::deque& excludedIndexes) { - size_t index; - if(getMissingFastPieceIndex(index, peer)) { - return checkOutPiece(index); + for(std::deque::const_iterator i = excludedIndexes.begin(); + i != excludedIndexes.end(); ++i) { + bitfield.unsetBit(*i); + } +} + +SharedHandle DefaultPieceStorage::getMissingPiece +(const SharedHandle& peer, const std::deque& excludedIndexes) +{ + BitfieldMan tempBitfield(bitfieldMan->getBlockLength(), + bitfieldMan->getTotalLength()); + tempBitfield.setBitfield(peer->getBitfield(), peer->getBitfieldLength()); + unsetExcludedIndexes(tempBitfield, excludedIndexes); + return getMissingPiece(tempBitfield); +} + +SharedHandle DefaultPieceStorage::getMissingFastPiece +(const SharedHandle& peer, const std::deque& excludedIndexes) +{ + if(peer->isFastExtensionEnabled() && peer->countPeerAllowedIndexSet() > 0) { + BitfieldMan tempBitfield(bitfieldMan->getBlockLength(), + bitfieldMan->getTotalLength()); + createFastIndexBitfield(tempBitfield, peer); + unsetExcludedIndexes(tempBitfield, excludedIndexes); + return getMissingPiece(tempBitfield); } else { return SharedHandle(); } diff --git a/src/DefaultPieceStorage.h b/src/DefaultPieceStorage.h index 95e5ce8b..20d64306 100644 --- a/src/DefaultPieceStorage.h +++ b/src/DefaultPieceStorage.h @@ -83,8 +83,17 @@ private: SharedHandle _pieceSelector; - bool getMissingPieceIndex(size_t& index, const SharedHandle& peer); - bool getMissingFastPieceIndex(size_t& index, const SharedHandle& peer); + bool getMissingPieceIndex(size_t& index, + const unsigned char* bitfield, size_t& length); + + SharedHandle getMissingPiece(const unsigned char* bitfield, + size_t length); + + SharedHandle getMissingPiece(const BitfieldMan& bitfield); + + void createFastIndexBitfield(BitfieldMan& bitfield, + const SharedHandle& peer); + SharedHandle checkOutPiece(size_t index); // size_t deleteUsedPiecesByFillRate(int fillRate, size_t toDelete); // void reduceUsedPieces(size_t upperBound); @@ -109,6 +118,12 @@ public: virtual SharedHandle getMissingFastPiece(const SharedHandle& peer); + virtual SharedHandle getMissingPiece + (const SharedHandle& peer, const std::deque& excludedIndexes); + + virtual SharedHandle getMissingFastPiece + (const SharedHandle& peer, const std::deque& excludedIndexes); + virtual SharedHandle getMissingPiece(); virtual SharedHandle getMissingPiece(size_t index); diff --git a/src/PieceStorage.h b/src/PieceStorage.h index 019daaa2..b23bcb8c 100644 --- a/src/PieceStorage.h +++ b/src/PieceStorage.h @@ -67,6 +67,14 @@ public: virtual SharedHandle getMissingPiece(const SharedHandle& peer) = 0; + /** + * Same as getMissingPiece(const SharedHandle& peer), but the indexes in + * excludedIndexes are excluded. + */ + virtual SharedHandle getMissingPiece + (const SharedHandle& peer, + const std::deque& excludedIndexes) = 0; + /** * Returns a piece that the peer has but localhost doesn't. * Only pieces that declared as "fast" are returned. @@ -76,6 +84,14 @@ public: */ virtual SharedHandle getMissingFastPiece(const SharedHandle& peer) = 0; + + /** + * Same as getMissingFastPiece(const SharedHandle& peer), but the + * indexes in excludedIndexes are excluded. + */ + virtual SharedHandle getMissingFastPiece + (const SharedHandle& peer, + const std::deque& excludedIndexes) = 0; /** * Returns a missing piece if available. Otherwise returns 0; diff --git a/src/UnknownLengthPieceStorage.cc b/src/UnknownLengthPieceStorage.cc index 5dac7d03..8169abc9 100644 --- a/src/UnknownLengthPieceStorage.cc +++ b/src/UnknownLengthPieceStorage.cc @@ -79,11 +79,23 @@ SharedHandle UnknownLengthPieceStorage::getMissingPiece(const SharedHandl abort(); } +SharedHandle UnknownLengthPieceStorage::getMissingPiece +(const SharedHandle& peer, const std::deque& excludedIndexes) +{ + abort(); +} + SharedHandle UnknownLengthPieceStorage::getMissingFastPiece(const SharedHandle& peer) { abort(); } +SharedHandle UnknownLengthPieceStorage::getMissingFastPiece +(const SharedHandle& peer, const std::deque& excludedIndexes) +{ + abort(); +} + PieceHandle UnknownLengthPieceStorage::getMissingPiece() { if(_downloadFinished) { diff --git a/src/UnknownLengthPieceStorage.h b/src/UnknownLengthPieceStorage.h index 139b11c6..d0b4276c 100644 --- a/src/UnknownLengthPieceStorage.h +++ b/src/UnknownLengthPieceStorage.h @@ -79,6 +79,9 @@ public: */ virtual SharedHandle getMissingPiece(const SharedHandle& peer); + virtual SharedHandle getMissingPiece + (const SharedHandle& peer, const std::deque& excludedIndexes); + /** * Returns a piece that the peer has but localhost doesn't. * Only pieces that declared as "fast" are returned. @@ -88,6 +91,9 @@ public: */ virtual SharedHandle getMissingFastPiece(const SharedHandle& peer); + virtual SharedHandle getMissingFastPiece + (const SharedHandle& peer, const std::deque& excludedIndexes); + /** * Returns a missing piece if available. Otherwise returns 0; */ diff --git a/test/DefaultBtRequestFactoryTest.cc b/test/DefaultBtRequestFactoryTest.cc index 299b6f7d..f0938a3d 100644 --- a/test/DefaultBtRequestFactoryTest.cc +++ b/test/DefaultBtRequestFactoryTest.cc @@ -25,6 +25,7 @@ class DefaultBtRequestFactoryTest:public CppUnit::TestFixture { CPPUNIT_TEST(testCreateRequestMessages); CPPUNIT_TEST(testCreateRequestMessages_onEndGame); CPPUNIT_TEST(testRemoveTargetPiece); + CPPUNIT_TEST(testGetTargetPieceIndexes); CPPUNIT_TEST_SUITE_END(); private: SharedHandle btRequestFactory; @@ -35,6 +36,7 @@ public: void testCreateRequestMessages(); void testCreateRequestMessages_onEndGame(); void testRemoveTargetPiece(); + void testGetTargetPieceIndexes(); class MockBtRequestMessage : public MockBtMessage { public: @@ -234,4 +236,22 @@ void DefaultBtRequestFactoryTest::testRemoveTargetPiece() { piece1) == btRequestFactory->getTargetPieces().end()); } +void DefaultBtRequestFactoryTest::testGetTargetPieceIndexes() +{ + SharedHandle piece1(new Piece(1, btContext->getPieceLength())); + SharedHandle piece3(new Piece(3, btContext->getPieceLength())); + SharedHandle piece5(new Piece(5, btContext->getPieceLength())); + + btRequestFactory->addTargetPiece(piece3); + btRequestFactory->addTargetPiece(piece1); + btRequestFactory->addTargetPiece(piece5); + + std::deque indexes; + btRequestFactory->getTargetPieceIndexes(indexes); + CPPUNIT_ASSERT_EQUAL((size_t)3, indexes.size()); + CPPUNIT_ASSERT_EQUAL((size_t)3, indexes[0]); + CPPUNIT_ASSERT_EQUAL((size_t)1, indexes[1]); + CPPUNIT_ASSERT_EQUAL((size_t)5, indexes[2]); +} + } // namespace aria2 diff --git a/test/DefaultPieceStorageTest.cc b/test/DefaultPieceStorageTest.cc index 5b370e64..55559b99 100644 --- a/test/DefaultPieceStorageTest.cc +++ b/test/DefaultPieceStorageTest.cc @@ -18,7 +18,9 @@ class DefaultPieceStorageTest:public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(DefaultPieceStorageTest); CPPUNIT_TEST(testGetTotalLength); CPPUNIT_TEST(testGetMissingPiece); + CPPUNIT_TEST(testGetMissingPiece_excludedIndexes); CPPUNIT_TEST(testGetMissingFastPiece); + CPPUNIT_TEST(testGetMissingFastPiece_excludedIndexes); CPPUNIT_TEST(testHasMissingPiece); CPPUNIT_TEST(testCompletePiece); CPPUNIT_TEST(testGetPiece); @@ -56,7 +58,9 @@ public: void testGetTotalLength(); void testGetMissingPiece(); + void testGetMissingPiece_excludedIndexes(); void testGetMissingFastPiece(); + void testGetMissingFastPiece_excludedIndexes(); void testHasMissingPiece(); void testCompletePiece(); void testGetPiece(); @@ -94,6 +98,28 @@ void DefaultPieceStorageTest::testGetMissingPiece() { CPPUNIT_ASSERT(piece.isNull()); } +void DefaultPieceStorageTest::testGetMissingPiece_excludedIndexes() +{ + DefaultPieceStorage pss(btContext, option, false); + pss.setEndGamePieceNum(0); + + peer->setAllBitfield(); + + std::deque excludedIndexes; + excludedIndexes.push_back(1); + + SharedHandle piece = pss.getMissingPiece(peer, excludedIndexes); + CPPUNIT_ASSERT_EQUAL(std::string("piece: index=0, length=128"), + piece->toString()); + + piece = pss.getMissingPiece(peer, excludedIndexes); + CPPUNIT_ASSERT_EQUAL(std::string("piece: index=2, length=128"), + piece->toString()); + + piece = pss.getMissingPiece(peer, excludedIndexes); + CPPUNIT_ASSERT(piece.isNull()); +} + void DefaultPieceStorageTest::testGetMissingFastPiece() { DefaultPieceStorage pss(btContext, option, false); pss.setEndGamePieceNum(0); @@ -105,6 +131,28 @@ void DefaultPieceStorageTest::testGetMissingFastPiece() { SharedHandle piece = pss.getMissingFastPiece(peer); CPPUNIT_ASSERT_EQUAL(std::string("piece: index=2, length=128"), piece->toString()); + + CPPUNIT_ASSERT(pss.getMissingFastPiece(peer).isNull()); +} + +void DefaultPieceStorageTest::testGetMissingFastPiece_excludedIndexes() +{ + DefaultPieceStorage pss(btContext, option, false); + pss.setEndGamePieceNum(0); + + peer->setAllBitfield(); + peer->setFastExtensionEnabled(true); + peer->addPeerAllowedIndex(1); + peer->addPeerAllowedIndex(2); + + std::deque excludedIndexes; + excludedIndexes.push_back(2); + + SharedHandle piece = pss.getMissingFastPiece(peer, excludedIndexes); + CPPUNIT_ASSERT_EQUAL(std::string("piece: index=1, length=128"), + piece->toString()); + + CPPUNIT_ASSERT(pss.getMissingFastPiece(peer, excludedIndexes).isNull()); } void DefaultPieceStorageTest::testHasMissingPiece() { diff --git a/test/MockBtRequestFactory.h b/test/MockBtRequestFactory.h index fa0d2721..6942ffb2 100644 --- a/test/MockBtRequestFactory.h +++ b/test/MockBtRequestFactory.h @@ -28,6 +28,8 @@ public: virtual void createRequestMessagesOnEndGame (std::deque >& requests, size_t max) {} + + virtual void getTargetPieceIndexes(std::deque& indexes) const {} }; } // namespace aria2 diff --git a/test/MockPieceStorage.h b/test/MockPieceStorage.h index e51f2994..476735f4 100644 --- a/test/MockPieceStorage.h +++ b/test/MockPieceStorage.h @@ -42,10 +42,22 @@ public: return SharedHandle(new Piece()); } + virtual SharedHandle getMissingPiece + (const SharedHandle& peer, const std::deque& excludedIndexes) + { + return SharedHandle(new Piece()); + } + virtual SharedHandle getMissingFastPiece(const SharedHandle& peer) { return SharedHandle(new Piece()); } + virtual SharedHandle getMissingFastPiece + (const SharedHandle& peer, const std::deque& excludedIndexes) + { + return SharedHandle(new Piece()); + } + virtual SharedHandle getMissingPiece() { return SharedHandle(new Piece());