Skip to content

Commit

Permalink
Make SchedulingGroup::Leave processing more efficient
Browse files Browse the repository at this point in the history
Maintain a BitSet of advertised RibOut indices in the PeerState
so that PeerState::IsMember can use BitSet::test operation which
is O(1), instead of map::count operation which is O(Log(N)).

Time required to execute the new unit test went from 11.x seconds
to 2.x seconds.

Change-Id: Ia6daf9242fbd91793938897d7cc3bec8ac02df04
Partial-Bug: 1461322
  • Loading branch information
Nischal Sheth committed Jun 2, 2015
1 parent e614cfe commit 28ced7c
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 4 deletions.
10 changes: 7 additions & 3 deletions src/bgp/scheduling_group.cc
Expand Up @@ -153,8 +153,8 @@ class SchedulingGroup::PeerState {

void Remove(RibState *rs);

bool IsMember(int index) const {
return rib_set_.count(index) > 0;
bool IsMember(size_t index) const {
return rib_bitset_.test(index);
}

iterator begin(const RibStateMap &indexmap) {
Expand Down Expand Up @@ -211,6 +211,7 @@ class SchedulingGroup::PeerState {
pair<Map::iterator, bool> result =
rib_set_.insert(make_pair(index, loc->second));
assert(result.second);
rib_bitset_.set(index);
for (int i = 0; i < RibOutUpdates::QCOUNT; i++) {
if (BitIsSet(loc->second.qactive, i)) {
qactive_cnt_[i] += 1;
Expand Down Expand Up @@ -248,8 +249,9 @@ class SchedulingGroup::PeerState {

private:
IPeerUpdate *key_;
size_t index_; // assigned from PeerStateMap in the group
size_t index_; // assigned from PeerStateMap in the group
Map rib_set_; // list of RibOuts advertised by the peer.
BitSet rib_bitset_; // bitset of RibOuts advertised by the peer
vector<int> qactive_cnt_;
bool in_sync_; // whether the peer may dequeue tail markers.
tbb::atomic<bool> send_ready_; // whether the peer may send updates.
Expand Down Expand Up @@ -368,6 +370,7 @@ void SchedulingGroup::PeerState::Add(RibState *rs) {
CHECK_CONCURRENCY("bgp::PeerMembership");
PeerRibState init;
rib_set_.insert(make_pair(rs->index(), init));
rib_bitset_.set(rs->index());
}

void SchedulingGroup::PeerState::Remove(RibState *rs) {
Expand All @@ -376,6 +379,7 @@ void SchedulingGroup::PeerState::Remove(RibState *rs) {
SetQueueInactive(rs->index(), queue_id);
}
rib_set_.erase(rs->index());
rib_bitset_.reset(rs->index());
}

void SchedulingGroup::PeerState::SetSync() {
Expand Down
48 changes: 47 additions & 1 deletion src/bgp/test/scheduling_group_test.cc
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) 2013 Juniper Networks, Inc. All rights reserved.
*/

#include "bgp/scheduling_group.h"
#include <boost/foreach.hpp>

#include "base/logging.h"
#include "base/task_annotations.h"
Expand All @@ -12,6 +12,7 @@
#include "bgp/bgp_log.h"
#include "bgp/bgp_peer.h"
#include "bgp/bgp_ribout_updates.h"
#include "bgp/scheduling_group.h"
#include "bgp/l3vpn/inetvpn_table.h"
#include "control-node/control_node.h"
#include "db/db.h"
Expand Down Expand Up @@ -451,6 +452,51 @@ TEST_F(SchedulingGroupManagerTest, TwoTablesMultiplePeers) {
STLDeleteValues(&peers);
}

//
// Exercise the Leave code with a large number of RibOuts.
// Test is constructed such that there's only a single SchedulingGroup.
// Intention is to test the performance of SchedulingGroup::GetPeerRibList.
//
TEST_F(SchedulingGroupManagerTest, LeaveScaling1) {
// Create 2 peers and join them to a common ribout.
vector<BgpTestPeer *> peers;
BgpTestPeer *test_peer(new BgpTestPeer());
BgpTestPeer *other_peer(new BgpTestPeer());
peers.push_back(test_peer);
peers.push_back(other_peer);
RibOut ribout_common(inetvpn_table_, &sgman_, RibExportPolicy());
Join(&ribout_common, test_peer);
Join(&ribout_common, other_peer);
EXPECT_EQ(1, sgman_.size());

// Create large number of additional RibOuts.
vector<RibOut *> ribouts;
for (int idx = 0; idx < 8192; ++idx) {
RibOut *ribout(new RibOut(inetvpn_table_, &sgman_,
RibExportPolicy(BgpProto::IBGP, RibExportPolicy::BGP, idx, 0)));
ribouts.push_back(ribout);
}

// Join the test peer to all the additional RibOuts.
BOOST_FOREACH(RibOut *ribout, ribouts) {
Join(ribout, test_peer);
}
EXPECT_EQ(1, sgman_.size());

// Leave the test peer from all the additional RibOuts.
BOOST_FOREACH(RibOut *ribout, ribouts) {
Leave(ribout, test_peer);
}
EXPECT_EQ(1, sgman_.size());

// Cleanup.
Leave(&ribout_common, test_peer);
Leave(&ribout_common, other_peer);
EXPECT_EQ(0, sgman_.size());
STLDeleteValues(&ribouts);
STLDeleteValues(&peers);
}

// Parameterize number of entries in the work queue and the order in which
// the entries are added.

Expand Down

0 comments on commit 28ced7c

Please sign in to comment.