From d0c124fe158a3215c187d5db07ad4e186fc72b46 Mon Sep 17 00:00:00 2001 From: Paul Farcasanu Date: Wed, 22 Jan 2025 11:31:02 -0800 Subject: [PATCH] cleanup addWritable Summary: All the functionality is a duplicated from `updateWritableStreams`. Btw the only callsites were inside UT. Reviewed By: kvtsoy Differential Revision: D68351972 fbshipit-source-id: 7e17f38ffcffecea23a64f5d2d5c1dec7a8c43f5 --- quic/api/test/QuicTransportBaseTest.cpp | 6 ----- quic/api/test/QuicTransportFunctionsTest.cpp | 2 +- quic/state/QuicStreamManager.h | 26 -------------------- quic/state/test/QuicStreamFunctionsTest.cpp | 2 +- 4 files changed, 2 insertions(+), 34 deletions(-) diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index db00d4bda..21b5994dd 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -4829,7 +4829,6 @@ TEST_P( stream->writeBuffer.append(std::move(dataBuf)); // Insert streamId into the list. - conn->streamManager->addWritable(*stream); conn->streamManager->updateWritableStreams(*stream); // Write looper is running. @@ -4893,7 +4892,6 @@ TEST_P( stream->writeBuffer.append(std::move(dataBuf)); // Insert streamId into the list. - conn->streamManager->addWritable(*stream); conn->streamManager->updateWritableStreams(*stream); // Write looper is stopped. @@ -4929,7 +4927,6 @@ TEST_P( stream->writeBuffer.append(std::move(dataBuf)); // Insert streamId into the list. - conn->streamManager->addWritable(*stream); conn->streamManager->updateWritableStreams(*stream); // Write looper is running. @@ -4969,7 +4966,6 @@ TEST_P( stream->writeBuffer.append(std::move(dataBuf)); // Insert streamId into the list. - conn->streamManager->addWritable(*stream); conn->streamManager->updateWritableStreams(*stream); // Write looper is running. @@ -5038,7 +5034,6 @@ TEST_P( conn->flowControlState.sumCurStreamBufferLen = testString.length(); // Insert streamId into the list. - conn->streamManager->addWritable(*stream); conn->streamManager->updateWritableStreams(*stream); // Mock arming the write callback @@ -5108,7 +5103,6 @@ TEST_P( stream->writeBuffer.append(std::move(dataBuf)); // Insert streamId into the list. - conn->streamManager->addWritable(*stream); conn->streamManager->updateWritableStreams(*stream); // Write looper is running. diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 00679556a..c903ddcad 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -3766,7 +3766,7 @@ TEST_F(QuicTransportFunctionsTest, HasAppDataToWrite) { conn->flowControlState.sumCurWriteOffset = 800; QuicStreamState stream(0, *conn); writeDataToQuicStream(stream, folly::IOBuf::copyBuffer("I'm a devil"), true); - conn->streamManager->addWritable(stream); + conn->streamManager->updateWritableStreams(stream); EXPECT_EQ(WriteDataReason::NO_WRITE, hasNonAckDataToWrite(*conn)); conn->oneRttWriteCipher = test::createNoOpAead(); diff --git a/quic/state/QuicStreamManager.h b/quic/state/QuicStreamManager.h index 794febd94..9ae4c3cd2 100644 --- a/quic/state/QuicStreamManager.h +++ b/quic/state/QuicStreamManager.h @@ -492,32 +492,6 @@ class QuicStreamManager { return !writableStreams_.empty() || !controlWriteQueue_.empty(); } - /* - * Add a writable stream id. - */ - void addWritable(const QuicStreamState& stream) { - if (stream.isControl) { - // Control streams get their own queue. - CHECK(stream.hasSchedulableData()); - controlWriteQueue_.insert(stream.id); - } else { - CHECK(stream.hasSchedulableData() || stream.hasSchedulableDsr()); - writeQueue_.insertOrUpdate(stream.id, stream.priority); - } - if (stream.hasWritableData()) { - writableStreams_.insert(stream.id); - } - if (stream.hasWritableBufMeta()) { - writableDSRStreams_.insert(stream.id); - } - if (!stream.lossBuffer.empty()) { - lossStreams_.insert(stream.id); - } - if (!stream.lossBufMetas.empty()) { - lossDSRStreams_.insert(stream.id); - } - } - /* * Remove a writable stream id. */ diff --git a/quic/state/test/QuicStreamFunctionsTest.cpp b/quic/state/test/QuicStreamFunctionsTest.cpp index 221fea9de..9d3ad8f59 100644 --- a/quic/state/test/QuicStreamFunctionsTest.cpp +++ b/quic/state/test/QuicStreamFunctionsTest.cpp @@ -1772,7 +1772,7 @@ TEST_P(QuicStreamFunctionsTestBase, RemovedClosedState) { conn.streamManager->readableStreams().emplace(streamId); conn.streamManager->peekableStreams().emplace(streamId); writeDataToQuicStream(*stream, folly::IOBuf::copyBuffer("write data"), true); - conn.streamManager->addWritable(*stream); + conn.streamManager->updateWritableStreams(*stream); conn.streamManager->queueBlocked(streamId, 0); conn.streamManager->addDeliverable(streamId); conn.streamManager->addLoss(streamId);