Deprecate gfe2_reloadable_flag_quic_stream_id_manager_handles_accounting.
PiperOrigin-RevId: 314205564
Change-Id: I10fc6c0c96255bb5bee3bdc8aa2e278bb705cf98
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index f527271..8b350ee 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -3307,7 +3307,9 @@
// though more resources need to be pushed.
if (!version_.HasIetfQuicFrames()) {
server_thread_->Pause();
- EXPECT_EQ(kNumMaxStreams, GetServerSession()->GetNumOpenOutgoingStreams());
+ EXPECT_EQ(kNumMaxStreams,
+ QuicSessionPeer::GetStreamIdManager(GetServerSession())
+ ->num_open_outgoing_streams());
server_thread_->Resume();
}
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index ba22c81..f35e2fc 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -240,7 +240,7 @@
TestStream* CreateIncomingStream(QuicStreamId id) override {
// Enforce the limit on the number of open streams.
if (!VersionHasIetfQuicFrames(connection()->transport_version()) &&
- GetNumOpenIncomingStreams() + 1 >
+ stream_id_manager().num_open_incoming_streams() + 1 >
max_open_incoming_bidirectional_streams()) {
connection()->CloseConnection(
QUIC_TOO_MANY_OPEN_STREAMS, "Too many streams!",
diff --git a/quic/core/legacy_quic_stream_id_manager.cc b/quic/core/legacy_quic_stream_id_manager.cc
index 6120f85..af4de8b 100644
--- a/quic/core/legacy_quic_stream_id_manager.cc
+++ b/quic/core/legacy_quic_stream_id_manager.cc
@@ -31,42 +31,20 @@
: QuicUtils::GetCryptoStreamId(transport_version_))
: QuicUtils::GetInvalidStreamId(transport_version_)),
num_open_incoming_streams_(0),
- num_open_outgoing_streams_(0),
- handles_accounting_(
- GetQuicReloadableFlag(quic_stream_id_manager_handles_accounting)) {
- if (handles_accounting_) {
- QUIC_RELOADABLE_FLAG_COUNT(quic_stream_id_manager_handles_accounting);
- }
-}
+ num_open_outgoing_streams_(0) {}
LegacyQuicStreamIdManager::~LegacyQuicStreamIdManager() {}
-bool LegacyQuicStreamIdManager::CanOpenNextOutgoingStream(
- size_t current_num_open_outgoing_streams) const {
- if (handles_accounting_) {
- DCHECK_LE(num_open_outgoing_streams_, max_open_outgoing_streams_);
- QUIC_DLOG_IF(INFO, num_open_outgoing_streams_ == max_open_outgoing_streams_)
- << "Failed to create a new outgoing stream. "
- << "Already " << num_open_outgoing_streams_ << " open.";
- return num_open_outgoing_streams_ < max_open_outgoing_streams_;
- }
- if (current_num_open_outgoing_streams >= max_open_outgoing_streams_) {
- QUIC_DLOG(INFO) << "Failed to create a new outgoing stream. "
- << "Already " << current_num_open_outgoing_streams
- << " open.";
- return false;
- }
- return true;
+bool LegacyQuicStreamIdManager::CanOpenNextOutgoingStream() const {
+ DCHECK_LE(num_open_outgoing_streams_, max_open_outgoing_streams_);
+ QUIC_DLOG_IF(INFO, num_open_outgoing_streams_ == max_open_outgoing_streams_)
+ << "Failed to create a new outgoing stream. "
+ << "Already " << num_open_outgoing_streams_ << " open.";
+ return num_open_outgoing_streams_ < max_open_outgoing_streams_;
}
-bool LegacyQuicStreamIdManager::CanOpenIncomingStream(
- size_t current_num_open_incoming_streams) const {
- if (handles_accounting_) {
- return num_open_incoming_streams_ < max_open_incoming_streams_;
- }
- // Check if the new number of open streams would cause the number of
- // open streams to exceed the limit.
- return current_num_open_incoming_streams < max_open_incoming_streams_;
+bool LegacyQuicStreamIdManager::CanOpenIncomingStream() const {
+ return num_open_incoming_streams_ < max_open_incoming_streams_;
}
bool LegacyQuicStreamIdManager::MaybeIncreaseLargestPeerStreamId(
@@ -121,7 +99,6 @@
}
void LegacyQuicStreamIdManager::ActivateStream(bool is_incoming) {
- DCHECK(handles_accounting_);
if (is_incoming) {
++num_open_incoming_streams_;
return;
@@ -130,7 +107,6 @@
}
void LegacyQuicStreamIdManager::OnStreamClosed(bool is_incoming) {
- DCHECK(handles_accounting_);
if (is_incoming) {
QUIC_BUG_IF(num_open_incoming_streams_ == 0);
--num_open_incoming_streams_;
diff --git a/quic/core/legacy_quic_stream_id_manager.h b/quic/core/legacy_quic_stream_id_manager.h
index 6c1309e..0131587 100644
--- a/quic/core/legacy_quic_stream_id_manager.h
+++ b/quic/core/legacy_quic_stream_id_manager.h
@@ -29,11 +29,10 @@
~LegacyQuicStreamIdManager();
// Returns true if the next outgoing stream ID can be allocated.
- bool CanOpenNextOutgoingStream(
- size_t current_num_open_outgoing_streams) const;
+ bool CanOpenNextOutgoingStream() const;
// Returns true if a new incoming stream can be opened.
- bool CanOpenIncomingStream(size_t current_num_open_incoming_streams) const;
+ bool CanOpenIncomingStream() const;
// Returns false when increasing the largest created stream id to |id| would
// violate the limit, so the connection should be closed.
@@ -95,8 +94,6 @@
return num_open_outgoing_streams_;
}
- bool handles_accounting() const { return handles_accounting_; }
-
private:
friend class test::QuicSessionPeer;
@@ -118,16 +115,11 @@
QuicStreamId largest_peer_created_stream_id_;
- // A counter for peer initiated open streams. Used when handles_accounting_ is
- // true.
+ // A counter for peer initiated open streams.
size_t num_open_incoming_streams_;
- // A counter for self initiated open streams. Used when handles_accounting_ is
- // true.
+ // A counter for self initiated open streams.
size_t num_open_outgoing_streams_;
-
- // Latched value of quic_stream_id_manager_handles_accounting.
- const bool handles_accounting_;
};
} // namespace quic
diff --git a/quic/core/legacy_quic_stream_id_manager_test.cc b/quic/core/legacy_quic_stream_id_manager_test.cc
index 00654b4..b7ba1d2 100644
--- a/quic/core/legacy_quic_stream_id_manager_test.cc
+++ b/quic/core/legacy_quic_stream_id_manager_test.cc
@@ -78,33 +78,21 @@
::testing::PrintToStringParamName());
TEST_P(LegacyQuicStreamIdManagerTest, CanOpenNextOutgoingStream) {
- if (GetQuicReloadableFlag(quic_stream_id_manager_handles_accounting)) {
- for (size_t i = 0; i < manager_.max_open_outgoing_streams() - 1; ++i) {
- manager_.ActivateStream(/*is_incoming=*/false);
- }
- }
- EXPECT_TRUE(manager_.CanOpenNextOutgoingStream(
- manager_.max_open_outgoing_streams() - 1));
- if (GetQuicReloadableFlag(quic_stream_id_manager_handles_accounting)) {
+ for (size_t i = 0; i < manager_.max_open_outgoing_streams() - 1; ++i) {
manager_.ActivateStream(/*is_incoming=*/false);
}
- EXPECT_FALSE(
- manager_.CanOpenNextOutgoingStream(manager_.max_open_outgoing_streams()));
+ EXPECT_TRUE(manager_.CanOpenNextOutgoingStream());
+ manager_.ActivateStream(/*is_incoming=*/false);
+ EXPECT_FALSE(manager_.CanOpenNextOutgoingStream());
}
TEST_P(LegacyQuicStreamIdManagerTest, CanOpenIncomingStream) {
- if (GetQuicReloadableFlag(quic_stream_id_manager_handles_accounting)) {
- for (size_t i = 0; i < manager_.max_open_incoming_streams() - 1; ++i) {
- manager_.ActivateStream(/*is_incoming=*/true);
- }
- }
- EXPECT_TRUE(
- manager_.CanOpenIncomingStream(manager_.max_open_incoming_streams() - 1));
- if (GetQuicReloadableFlag(quic_stream_id_manager_handles_accounting)) {
+ for (size_t i = 0; i < manager_.max_open_incoming_streams() - 1; ++i) {
manager_.ActivateStream(/*is_incoming=*/true);
}
- EXPECT_FALSE(
- manager_.CanOpenIncomingStream(manager_.max_open_incoming_streams()));
+ EXPECT_TRUE(manager_.CanOpenIncomingStream());
+ manager_.ActivateStream(/*is_incoming=*/true);
+ EXPECT_FALSE(manager_.CanOpenIncomingStream());
}
TEST_P(LegacyQuicStreamIdManagerTest, AvailableStreams) {
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 7fb52dd..218384f 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -73,12 +73,10 @@
config_.GetMaxBidirectionalStreamsToSend(),
config_.GetMaxUnidirectionalStreamsToSend() +
num_expected_unidirectional_static_streams),
- num_dynamic_incoming_streams_(0),
num_draining_incoming_streams_(0),
num_draining_outgoing_streams_(0),
num_outgoing_static_streams_(0),
num_incoming_static_streams_(0),
- num_locally_closed_incoming_streams_highest_offset_(0),
flow_controller_(
this,
QuicUtils::GetInvalidStreamId(connection->transport_version()),
@@ -137,16 +135,6 @@
QuicSession::~QuicSession() {
QUIC_LOG_IF(WARNING, !zombie_streams_.empty()) << "Still have zombie streams";
- QUIC_LOG_IF(WARNING, num_locally_closed_incoming_streams_highest_offset() >
- stream_id_manager_.max_open_incoming_streams())
- << "Surprisingly high number of locally closed peer initiated streams"
- "still waiting for final byte offset: "
- << num_locally_closed_incoming_streams_highest_offset();
- QUIC_LOG_IF(WARNING, GetNumLocallyClosedOutgoingStreamsHighestOffset() >
- stream_id_manager_.max_open_outgoing_streams())
- << "Surprisingly high number of locally closed self initiated streams"
- "still waiting for final byte offset: "
- << GetNumLocallyClosedOutgoingStreamsHighestOffset();
}
void QuicSession::PendingStreamOnStreamFrame(const QuicStreamFrame& frame) {
@@ -879,9 +867,6 @@
const QuicStreamId id,
QuicStreamOffset offset) {
locally_closed_streams_highest_offset_[id] = offset;
- if (IsIncomingStream(id)) {
- ++num_locally_closed_incoming_streams_highest_offset_;
- }
}
void QuicSession::OnStreamClosed(QuicStreamId stream_id) {
@@ -918,9 +903,6 @@
InsertLocallyClosedStreamsHighestOffset(
stream_id, stream->flow_controller()->highest_received_byte_offset());
stream_map_.erase(it);
- if (IsIncomingStream(stream_id)) {
- --num_dynamic_incoming_streams_;
- }
return;
}
@@ -928,9 +910,6 @@
QUIC_DVLOG_IF(1, stream_was_draining)
<< ENDPOINT << "Stream " << stream_id << " was draining";
stream_map_.erase(it);
- if (IsIncomingStream(stream_id)) {
- --num_dynamic_incoming_streams_;
- }
if (stream_was_draining) {
if (IsIncomingStream(stream_id)) {
QUIC_BUG_IF(num_draining_incoming_streams_ == 0);
@@ -948,8 +927,7 @@
// disconnected.
return;
}
- if (stream_id_manager_.handles_accounting() &&
- !VersionHasIetfQuicFrames(transport_version())) {
+ if (!VersionHasIetfQuicFrames(transport_version())) {
stream_id_manager_.OnStreamClosed(
/*is_incoming=*/IsIncomingStream(stream_id));
}
@@ -1004,13 +982,11 @@
flow_controller_.AddBytesConsumed(offset_diff);
locally_closed_streams_highest_offset_.erase(it);
- if (stream_id_manager_.handles_accounting() &&
- !VersionHasIetfQuicFrames(transport_version())) {
+ if (!VersionHasIetfQuicFrames(transport_version())) {
stream_id_manager_.OnStreamClosed(
/*is_incoming=*/IsIncomingStream(stream_id));
}
if (IsIncomingStream(stream_id)) {
- --num_locally_closed_incoming_streams_highest_offset_;
if (VersionHasIetfQuicFrames(transport_version())) {
v99_streamid_manager_.OnStreamClosed(stream_id);
}
@@ -1588,14 +1564,12 @@
<< ". activating stream " << stream_id;
DCHECK(!QuicContainsKey(stream_map_, stream_id));
stream_map_[stream_id] = std::move(stream);
- if (IsIncomingStream(stream_id)) {
- is_static ? ++num_incoming_static_streams_
- : ++num_dynamic_incoming_streams_;
- } else if (is_static) {
- ++num_outgoing_static_streams_;
+ if (is_static) {
+ IsIncomingStream(stream_id) ? ++num_incoming_static_streams_
+ : ++num_outgoing_static_streams_;
+ return;
}
- if (stream_id_manager_.handles_accounting() && !is_static &&
- !VersionHasIetfQuicFrames(transport_version())) {
+ if (!VersionHasIetfQuicFrames(transport_version())) {
// Do not inform stream ID manager of static streams.
stream_id_manager_.ActivateStream(
/*is_incoming=*/IsIncomingStream(stream_id));
@@ -1618,8 +1592,7 @@
bool QuicSession::CanOpenNextOutgoingBidirectionalStream() {
if (!VersionHasIetfQuicFrames(transport_version())) {
- return stream_id_manager_.CanOpenNextOutgoingStream(
- GetNumOpenOutgoingStreams());
+ return stream_id_manager_.CanOpenNextOutgoingStream();
}
if (v99_streamid_manager_.CanOpenNextOutgoingBidirectionalStream()) {
return true;
@@ -1635,8 +1608,7 @@
bool QuicSession::CanOpenNextOutgoingUnidirectionalStream() {
if (!VersionHasIetfQuicFrames(transport_version())) {
- return stream_id_manager_.CanOpenNextOutgoingStream(
- GetNumOpenOutgoingStreams());
+ return stream_id_manager_.CanOpenNextOutgoingStream();
}
if (v99_streamid_manager_.CanOpenNextOutgoingUnidirectionalStream()) {
return true;
@@ -1685,15 +1657,11 @@
return nullptr;
}
- if (!VersionHasIetfQuicFrames(transport_version())) {
- // TODO(fayang): Let LegacyQuicStreamIdManager count open streams and make
- // CanOpenIncomingStream interface consistent with that of v99.
- if (!stream_id_manager_.CanOpenIncomingStream(
- GetNumOpenIncomingStreams())) {
- // Refuse to open the stream.
- ResetStream(stream_id, QUIC_REFUSED_STREAM, 0);
- return nullptr;
- }
+ if (!VersionHasIetfQuicFrames(transport_version()) &&
+ !stream_id_manager_.CanOpenIncomingStream()) {
+ // Refuse to open the stream.
+ ResetStream(stream_id, QUIC_REFUSED_STREAM, 0);
+ return nullptr;
}
return CreateIncomingStream(stream_id);
@@ -1704,7 +1672,7 @@
QUIC_DVLOG(1) << ENDPOINT << "Stream " << stream_id << " is draining";
if (VersionHasIetfQuicFrames(transport_version())) {
v99_streamid_manager_.OnStreamClosed(stream_id);
- } else if (stream_id_manager_.handles_accounting()) {
+ } else {
stream_id_manager_.OnStreamClosed(
/*is_incoming=*/IsIncomingStream(stream_id));
}
@@ -1830,31 +1798,8 @@
return it->second->is_static();
}
-size_t QuicSession::GetNumOpenIncomingStreams() const {
- DCHECK(!VersionHasIetfQuicFrames(transport_version()));
- if (stream_id_manager_.handles_accounting()) {
- return stream_id_manager_.num_open_incoming_streams();
- }
- return num_dynamic_incoming_streams_ - num_draining_incoming_streams_ +
- num_locally_closed_incoming_streams_highest_offset_;
-}
-
-size_t QuicSession::GetNumOpenOutgoingStreams() const {
- DCHECK(!VersionHasIetfQuicFrames(transport_version()));
- if (stream_id_manager_.handles_accounting()) {
- return stream_id_manager_.num_open_outgoing_streams();
- }
- DCHECK_GE(GetNumDynamicOutgoingStreams() +
- GetNumLocallyClosedOutgoingStreamsHighestOffset(),
- GetNumDrainingOutgoingStreams());
- return GetNumDynamicOutgoingStreams() +
- GetNumLocallyClosedOutgoingStreamsHighestOffset() -
- GetNumDrainingOutgoingStreams();
-}
-
size_t QuicSession::GetNumActiveStreams() const {
- if (!VersionHasIetfQuicFrames(transport_version()) &&
- stream_id_manager_.handles_accounting()) {
+ if (!VersionHasIetfQuicFrames(transport_version())) {
// Exclude locally_closed_streams when determine whether to keep connection
// alive.
return stream_id_manager_.num_open_incoming_streams() +
@@ -1897,27 +1842,10 @@
control_frame_manager_.WritePing();
}
-size_t QuicSession::GetNumDynamicOutgoingStreams() const {
- DCHECK_GE(
- static_cast<size_t>(stream_map_.size() + pending_stream_map_.size()),
- num_dynamic_incoming_streams_ + num_outgoing_static_streams_ +
- num_incoming_static_streams_);
- return stream_map_.size() + pending_stream_map_.size() -
- num_dynamic_incoming_streams_ - num_outgoing_static_streams_ -
- num_incoming_static_streams_;
-}
-
size_t QuicSession::GetNumDrainingOutgoingStreams() const {
return num_draining_outgoing_streams_;
}
-size_t QuicSession::GetNumLocallyClosedOutgoingStreamsHighestOffset() const {
- DCHECK_GE(locally_closed_streams_highest_offset_.size(),
- num_locally_closed_incoming_streams_highest_offset_);
- return locally_closed_streams_highest_offset_.size() -
- num_locally_closed_incoming_streams_highest_offset_;
-}
-
bool QuicSession::IsConnectionFlowControlBlocked() const {
return flow_controller_.IsBlocked();
}
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index d7400d5..c156a6e 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -334,20 +334,6 @@
// Returns the number of currently draining streams.
size_t GetNumDrainingStreams() const;
- // Returns the number of currently open peer initiated streams, excluding
- // static streams.
- // TODO(fayang): remove this and instead use
- // LegacyStreamIdManager::num_open_incoming_streams() in tests when
- // deprecating quic_stream_id_manager_handles_accounting.
- size_t GetNumOpenIncomingStreams() const;
-
- // Returns the number of currently open self initiated streams, excluding
- // static streams.
- // TODO(fayang): remove this and instead use
- // LegacyStreamIdManager::num_open_outgoing_streams() in tests when
- // deprecating quic_stream_id_manager_handles_accounting.
- size_t GetNumOpenOutgoingStreams() const;
-
// Returns the number of open peer initiated static streams.
size_t num_incoming_static_streams() const {
return num_incoming_static_streams_;
@@ -448,12 +434,6 @@
// Return true if given stream is peer initiated.
bool IsIncomingStream(QuicStreamId id) const;
- size_t GetNumLocallyClosedOutgoingStreamsHighestOffset() const;
-
- size_t num_locally_closed_incoming_streams_highest_offset() const {
- return num_locally_closed_incoming_streams_highest_offset_;
- }
-
// Record errors when a connection is closed at the server side, should only
// be called from server's perspective.
// Noop if |error| is QUIC_NO_ERROR.
@@ -591,8 +571,6 @@
return &write_blocked_streams_;
}
- size_t GetNumDynamicOutgoingStreams() const;
-
size_t GetNumDrainingOutgoingStreams() const;
// Returns true if the stream is still active.
@@ -772,37 +750,26 @@
// Manages stream IDs for version99/IETF QUIC
UberQuicStreamIdManager v99_streamid_manager_;
- // A counter for peer initiated dynamic streams which are in the stream_map_.
- // TODO(fayang): Remove this when deprecating
- // quic_stream_id_manager_handles_accounting.
- size_t num_dynamic_incoming_streams_;
-
// A counter for peer initiated streams which have sent and received FIN but
// waiting for application to consume data.
- // TODO(fayang): Remove this when deprecating
- // quic_stream_id_manager_handles_accounting.
+ // TODO(fayang): Merge num_draining_incoming_streams_ and
+ // num_draining_outgoing_streams_.
size_t num_draining_incoming_streams_;
// A counter for self initiated streams which have sent and received FIN but
// waiting for application to consume data.
- // TODO(fayang): Remove this when deprecating
- // quic_stream_id_manager_handles_accounting.
size_t num_draining_outgoing_streams_;
// A counter for self initiated static streams which are in
// stream_map_.
+ // TODO(fayang): Merge num_outgoing_static_streams_ and
+ // num_incoming_static_streams_.
size_t num_outgoing_static_streams_;
// A counter for peer initiated static streams which are in
// stream_map_.
size_t num_incoming_static_streams_;
- // A counter for peer initiated streams which are in the
- // locally_closed_streams_highest_offset_.
- // TODO(fayang): Remove this when deprecating
- // quic_stream_id_manager_handles_accounting.
- size_t num_locally_closed_incoming_streams_highest_offset_;
-
// Received information for a connection close.
QuicConnectionCloseFrame on_closed_frame_;
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index ad10f43..d2ff9f8 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -230,7 +230,7 @@
TestStream* CreateIncomingStream(QuicStreamId id) override {
// Enforce the limit on the number of open streams.
if (!VersionHasIetfQuicFrames(connection()->transport_version()) &&
- GetNumOpenIncomingStreams() + 1 >
+ stream_id_manager().num_open_incoming_streams() + 1 >
max_open_incoming_bidirectional_streams()) {
// No need to do this test for version 99; it's done by
// QuicSession::GetOrCreateStream.
