Remove max_streams_window_ in QuicStreamIdManager to make the code more readable.
There is no need to have a dedicated member variable to store the window value because the computation of window is really simple and only used in one place.
gfe-relnote: protected by gfe2_reloadable_flag_quic_enable_version_draft_25_v3
PiperOrigin-RevId: 305126280
Change-Id: Iee5f76af87dfe40c702ba74a6d21cdcc15ca2aa3
diff --git a/quic/core/quic_stream_id_manager.cc b/quic/core/quic_stream_id_manager.cc
index 469a692..533c89e 100644
--- a/quic/core/quic_stream_id_manager.cc
+++ b/quic/core/quic_stream_id_manager.cc
@@ -41,10 +41,7 @@
incoming_initial_max_open_streams_(max_allowed_incoming_streams),
incoming_stream_count_(0),
largest_peer_created_stream_id_(
- QuicUtils::GetInvalidStreamId(transport_version)),
- max_streams_window_(0) {
- CalculateIncomingMaxStreamsWindow();
-}
+ QuicUtils::GetInvalidStreamId(transport_version)) {}
QuicStreamIdManager::~QuicStreamIdManager() {}
@@ -100,12 +97,11 @@
incoming_actual_max_streams_ = max_open_streams;
incoming_advertised_max_streams_ = max_open_streams;
incoming_initial_max_open_streams_ = max_open_streams;
- CalculateIncomingMaxStreamsWindow();
}
void QuicStreamIdManager::MaybeSendMaxStreamsFrame() {
if ((incoming_advertised_max_streams_ - incoming_stream_count_) >
- max_streams_window_) {
+ (incoming_initial_max_open_streams_ / kMaxStreamsWindowDivisor)) {
// window too large, no advertisement
return;
}
@@ -252,11 +248,4 @@
return incoming_advertised_max_streams_ - incoming_stream_count_;
}
-void QuicStreamIdManager::CalculateIncomingMaxStreamsWindow() {
- max_streams_window_ = incoming_actual_max_streams_ / kMaxStreamsWindowDivisor;
- if (max_streams_window_ == 0) {
- max_streams_window_ = 1;
- }
-}
-
} // namespace quic
diff --git a/quic/core/quic_stream_id_manager.h b/quic/core/quic_stream_id_manager.h
index e396e64..e66bc22 100644
--- a/quic/core/quic_stream_id_manager.h
+++ b/quic/core/quic_stream_id_manager.h
@@ -58,7 +58,7 @@
", incoming_stream_count_: ", incoming_stream_count_,
", available_streams_.size(): ", available_streams_.size(),
", largest_peer_created_stream_id_: ", largest_peer_created_stream_id_,
- ", max_streams_window_: ", max_streams_window_, " }");
+ " }");
}
// Processes the STREAMS_BLOCKED frame. If error is encountered, populates
@@ -109,8 +109,6 @@
return incoming_initial_max_open_streams_;
}
- QuicStreamCount max_streams_window() const { return max_streams_window_; }
-
QuicStreamId next_outgoing_stream_id() const {
return next_outgoing_stream_id_;
}
@@ -156,8 +154,6 @@
QuicStreamId GetFirstOutgoingStreamId() const;
QuicStreamId GetFirstIncomingStreamId() const;
- void CalculateIncomingMaxStreamsWindow();
-
// Back reference to the session containing this Stream ID Manager.
// needed to access various session methods, such as perspective()
DelegateInterface* delegate_;
@@ -206,14 +202,6 @@
QuicUnorderedSet<QuicStreamId> available_streams_;
QuicStreamId largest_peer_created_stream_id_;
-
- // When incoming streams close the local node sends MAX_STREAMS frames. It
- // does so only when the peer can open fewer than |max_stream_id_window_|
- // streams. That is, when |incoming_actual_max_streams_| -
- // |incoming_advertised_max_streams_| is less than the window.
- // max_streams_window_ is set to 1/2 of the initial number of incoming streams
- // that are allowed (as set in the constructor).
- QuicStreamId max_streams_window_;
};
} // namespace quic
diff --git a/quic/core/quic_stream_id_manager_test.cc b/quic/core/quic_stream_id_manager_test.cc
index 81198f8..1062412 100644
--- a/quic/core/quic_stream_id_manager_test.cc
+++ b/quic/core/quic_stream_id_manager_test.cc
@@ -115,11 +115,6 @@
stream_id_manager_.incoming_advertised_max_streams());
EXPECT_EQ(kDefaultMaxStreamsPerConnection,
stream_id_manager_.incoming_initial_max_open_streams());
-
- // The window for advertising updates to the MAX STREAM ID is half the number
- // of streams allowed.
- EXPECT_EQ(kDefaultMaxStreamsPerConnection / 2,
- stream_id_manager_.max_streams_window());
}
// This test checks that the stream advertisement window is set to 1
@@ -128,7 +123,6 @@
stream_id_manager_.SetMaxOpenIncomingStreams(1);
EXPECT_EQ(1u, stream_id_manager_.incoming_initial_max_open_streams());
EXPECT_EQ(1u, stream_id_manager_.incoming_actual_max_streams());
- EXPECT_EQ(1u, stream_id_manager_.max_streams_window());
}
TEST_P(QuicStreamIdManagerTest, CheckMaxStreamsBadValuesOverMaxFailsOutgoing) {
@@ -326,15 +320,11 @@
}
TEST_P(QuicStreamIdManagerTest, MaxStreamsWindow) {
- // Test that a MAX_STREAMS frame is generated when the peer has less than
- // |max_streams_window_| streams left that it can initiate.
-
- // First, open, and then close, max_streams_window_ streams. This will
- // max_streams_window_ streams available for the peer -- no MAX_STREAMS
- // should be sent. The -1 is because the check in
- // QuicStreamIdManager::MaybeSendMaxStreamsFrame sends a MAX_STREAMS if the
- // number of available streams at the peer is <= |max_streams_window_|
- int stream_count = stream_id_manager_.max_streams_window() - 1;
+ // Open and then close a number of streams to get close to the threshold of
+ // sending a MAX_STREAM_FRAME.
+ int stream_count = stream_id_manager_.incoming_initial_max_open_streams() /
+ kMaxStreamsWindowDivisor -
+ 1;
// Should not get a control-frame transmission since the peer should have
// "plenty" of stream IDs to use.
@@ -344,7 +334,8 @@
QuicStreamId stream_id = GetNthIncomingStreamId(0);
size_t old_available_incoming_streams =
stream_id_manager_.available_incoming_streams();
- while (stream_count) {
+ auto i = stream_count;
+ while (i) {
EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id,
nullptr));
@@ -354,12 +345,11 @@
EXPECT_EQ(old_available_incoming_streams,
stream_id_manager_.available_incoming_streams());
- stream_count--;
+ i--;
stream_id += QuicUtils::StreamIdDelta(transport_version());
}
// Now close them, still should get no MAX_STREAMS
- stream_count = stream_id_manager_.max_streams_window();
stream_id = GetNthIncomingStreamId(0);
QuicStreamCount expected_actual_max =
stream_id_manager_.incoming_actual_max_streams();
@@ -420,18 +410,15 @@
stream_id_manager_.incoming_advertised_max_streams();
// Open/close enough streams to shrink the window without causing a MAX
- // STREAMS to be generated. The window will open (and a MAX STREAMS generated)
- // when max_streams_window() stream IDs have been made available. The loop
+ // STREAMS to be generated. The loop
// will make that many stream IDs available, so the last CloseStream should
-
// cause a MAX STREAMS frame to be generated.
- int i = static_cast<int>(stream_id_manager_.max_streams_window());
+ int i =
+ static_cast<int>(stream_id_manager_.incoming_initial_max_open_streams() /
+ kMaxStreamsWindowDivisor);
QuicStreamId id =
QuicStreamIdManagerPeer::GetFirstIncomingStreamId(&stream_id_manager_);
- EXPECT_CALL(
- delegate_,
- SendMaxStreams(first_advert + stream_id_manager_.max_streams_window(),
- IsUnidirectional()));
+ EXPECT_CALL(delegate_, SendMaxStreams(first_advert + i, IsUnidirectional()));
while (i) {
EXPECT_TRUE(
stream_id_manager_.MaybeIncreaseLargestPeerStreamId(id, nullptr));
