StreamIdSet for peer stream groups seen.

Summary: As in title. Missed this one. We also have to have two sets for unidirectional/bidirectional groups.

Reviewed By: sharmafb

Differential Revision: D52436642

fbshipit-source-id: 188682861e7aedebc52a28267be41544f1b4a47f

quic/codec/Types.h

@@ -31,7 +31,7 @@
 namespace quic {
 
 using StreamId = uint64_t;
-using StreamGroupId = uint64_t;
+using StreamGroupId = StreamId;
 
 enum class PacketNumberSpace : uint8_t {
   Initial,

quic/state/QuicStreamManager.cpp

@@ -334,10 +334,14 @@ QuicStreamManager::createNextUnidirectionalStream(
 QuicStreamState* FOLLY_NULLABLE QuicStreamManager::instantiatePeerStream(
     StreamId streamId,
     folly::Optional<StreamGroupId> groupId) {
-  if (groupId &&
-      (peerStreamGroupsSeen_.find(*groupId) == peerStreamGroupsSeen_.cend())) {
-    newPeerStreamGroups_.insert(*groupId);
-    peerStreamGroupsSeen_.insert(*groupId);
+  if (groupId) {
+    auto& seenSet = isUnidirectionalStream(streamId)
+        ? peerUnidirectionalStreamGroupsSeen_
+        : peerBidirectionalStreamGroupsSeen_;
+    if (!seenSet.contains(*groupId)) {
+      newPeerStreamGroups_.insert(*groupId);
+      seenSet.add(*groupId);
+    }
   }
 
   if (transportSettings_->notifyOnNewStreamsExplicitly) {

quic/state/QuicStreamManager.h

@@ -100,6 +100,8 @@ class QuicStreamManager {
       initialLocalUnidirectionalStreamId_ = 0x03;
       initialRemoteBidirectionalStreamId_ = 0x00;
       initialRemoteUnidirectionalStreamId_ = 0x02;
+      peerUnidirectionalStreamGroupsSeen_ = StreamIdSet(0x02);
+      peerBidirectionalStreamGroupsSeen_ = StreamIdSet(0x00);
     } else {
       nextAcceptablePeerBidirectionalStreamId_ = 0x01;
       nextAcceptablePeerUnidirectionalStreamId_ = 0x03;
@@ -111,6 +113,8 @@ class QuicStreamManager {
       initialLocalUnidirectionalStreamId_ = 0x02;
       initialRemoteBidirectionalStreamId_ = 0x01;
       initialRemoteUnidirectionalStreamId_ = 0x03;
+      peerUnidirectionalStreamGroupsSeen_ = StreamIdSet(0x03);
+      peerBidirectionalStreamGroupsSeen_ = StreamIdSet(0x01);
     }
     nextBidirectionalStreamGroupId_ = nextBidirectionalStreamId_;
     nextUnidirectionalStreamGroupId_ = nextUnidirectionalStreamId_;
@@ -188,7 +192,8 @@ class QuicStreamManager {
         std::move(other.openUnidirectionalLocalStreamGroups_);
     newPeerStreams_ = std::move(other.newPeerStreams_);
     newPeerStreamGroups_ = std::move(other.newPeerStreamGroups_);
-    peerStreamGroupsSeen_ = std::move(other.peerStreamGroupsSeen_);
+    peerUnidirectionalStreamGroupsSeen_ =
+        std::move(other.peerUnidirectionalStreamGroupsSeen_);
     newGroupedPeerStreams_ = std::move(other.newGroupedPeerStreams_);
     blockedStreams_ = std::move(other.blockedStreams_);
     stopSendingStreams_ = std::move(other.stopSendingStreams_);
@@ -408,7 +413,8 @@ class QuicStreamManager {
     openUnidirectionalPeerStreams_.clear();
     openBidirectionalLocalStreamGroups_.clear();
     openUnidirectionalLocalStreamGroups_.clear();
-    peerStreamGroupsSeen_.clear();
+    peerUnidirectionalStreamGroupsSeen_.clear();
+    peerBidirectionalStreamGroupsSeen_.clear();
     streams_.clear();
   }
 
@@ -1060,7 +1066,8 @@ class QuicStreamManager {
   }
 
   [[nodiscard]] size_t getNumPeerStreamGroupsSeen() const {
-    return peerStreamGroupsSeen_.size();
+    return peerUnidirectionalStreamGroupsSeen_.size() +
+        peerBidirectionalStreamGroupsSeen_.size();
   }
 
  private:
@@ -1204,7 +1211,8 @@ class QuicStreamManager {
   folly::F14FastSet<StreamGroupId> newPeerStreamGroups_;
 
   // Peer group ids seen.
-  folly::F14FastSet<StreamGroupId> peerStreamGroupsSeen_;
+  StreamIdSet peerUnidirectionalStreamGroupsSeen_;
+  StreamIdSet peerBidirectionalStreamGroupsSeen_;
 
   // Map of streams that were blocked
   folly::F14FastMap<StreamId, StreamDataBlockedFrame> blockedStreams_;

quic/state/test/QuicStreamManagerTest.cpp

@@ -853,7 +853,7 @@ TEST_P(QuicStreamManagerGroupsTest, TestPeerStreamsWithGroup) {
   manager.refreshTransportSettings(conn.transportSettings);
 
   const StreamId peerStreamId = 2;
-  const StreamGroupId peeGroupId = 0;
+  const StreamGroupId peeGroupId = 2;
   auto stream = manager.getStream(peerStreamId, peeGroupId);
   EXPECT_NE(stream, nullptr);
   EXPECT_EQ(stream->groupId, peeGroupId);
@@ -877,7 +877,7 @@ TEST_P(QuicStreamManagerGroupsTest, TestPeerStreamsWithGroupAccounting) {
   manager.refreshTransportSettings(conn.transportSettings);
 
   StreamId peerStreamId = 2;
-  StreamGroupId peeGroupId = 0;
+  StreamGroupId peeGroupId = 2;
   auto stream = manager.getStream(peerStreamId, peeGroupId);
   EXPECT_NE(stream, nullptr);
   EXPECT_EQ(stream->groupId, peeGroupId);
@@ -886,7 +886,7 @@ TEST_P(QuicStreamManagerGroupsTest, TestPeerStreamsWithGroupAccounting) {
 
   // Another stream, same group.
   peerStreamId = 6;
-  peeGroupId = 0;
+  peeGroupId = 2;
   stream = manager.getStream(peerStreamId, peeGroupId);
   EXPECT_NE(stream, nullptr);
   EXPECT_EQ(stream->groupId, peeGroupId);
@@ -895,7 +895,7 @@ TEST_P(QuicStreamManagerGroupsTest, TestPeerStreamsWithGroupAccounting) {
 
   // New stream, new group.
   peerStreamId = 10;
-  peeGroupId = 4;
+  peeGroupId = 6;
   stream = manager.getStream(peerStreamId, peeGroupId);
   EXPECT_NE(stream, nullptr);
   EXPECT_EQ(stream->groupId, peeGroupId);
@@ -904,7 +904,7 @@ TEST_P(QuicStreamManagerGroupsTest, TestPeerStreamsWithGroupAccounting) {
 
   // New stream, previous group.
   peerStreamId = 14;
-  peeGroupId = 0;
+  peeGroupId = 2;
   stream = manager.getStream(peerStreamId, peeGroupId);
   EXPECT_NE(stream, nullptr);
   EXPECT_EQ(stream->groupId, peeGroupId);
@@ -913,7 +913,7 @@ TEST_P(QuicStreamManagerGroupsTest, TestPeerStreamsWithGroupAccounting) {
 
   // New stream, current group.
   peerStreamId = 18;
-  peeGroupId = 4;
+  peeGroupId = 6;
   stream = manager.getStream(peerStreamId, peeGroupId);
   EXPECT_NE(stream, nullptr);
   EXPECT_EQ(stream->groupId, peeGroupId);