1
0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-08-06 22:22:38 +03:00

Always call writeGSO

Summary: There's no need to call write when there's only one packet, since write just wraps writeGSO. This simplifies the use of options and will allow for setting a TXTIME when there's only one packet to send.

Reviewed By: kvtsoy

Differential Revision: D48526018

fbshipit-source-id: d934ddce8d3a35febb58ff253fc7a9bed3ef975c
This commit is contained in:
Matt Joras
2023-08-21 16:13:38 -07:00
committed by Facebook GitHub Bot
parent 646bd89e65
commit df49cb664f
3 changed files with 26 additions and 22 deletions

View File

@@ -72,13 +72,13 @@ bool GSOPacketBatchWriter::append(
ssize_t GSOPacketBatchWriter::write( ssize_t GSOPacketBatchWriter::write(
QuicAsyncUDPSocketType& sock, QuicAsyncUDPSocketType& sock,
const folly::SocketAddress& address) { const folly::SocketAddress& address) {
return (currBufs_ > 1) // Even though it's called writeGSO, it can handle individual writes by
? sock.writeGSO( // setting gsoVal = 0.
address, int gsoVal = currBufs_ > 1 ? static_cast<int>(prevSize_) : 0;
buf_, return sock.writeGSO(
folly::AsyncUDPSocket::WriteOptions( address,
static_cast<int>(prevSize_) /*gsoVal*/, false /* zerocopyVal*/)) buf_,
: sock.write(address, buf_); folly::AsyncUDPSocket::WriteOptions(gsoVal, false /* zerocopyVal */));
} }
GSOInplacePacketBatchWriter::GSOInplacePacketBatchWriter( GSOInplacePacketBatchWriter::GSOInplacePacketBatchWriter(
@@ -155,13 +155,13 @@ ssize_t GSOInplacePacketBatchWriter::write(
} }
uint64_t diffToStart = lastPacketEnd_ - buf->data(); uint64_t diffToStart = lastPacketEnd_ - buf->data();
buf->trimEnd(diffToEnd); buf->trimEnd(diffToEnd);
auto bytesWritten = (numPackets_ > 1) // Even though it's called writeGSO, it can handle individual writes by
? sock.writeGSO( // setting gsoVal = 0.
address, int gsoVal = numPackets_ > 1 ? static_cast<int>(prevSize_) : 0;
buf, auto bytesWritten = sock.writeGSO(
folly::AsyncUDPSocket::WriteOptions( address,
static_cast<int>(prevSize_) /*gsoVal*/, false /* zerocopyVal*/)) buf,
: sock.write(address, buf); folly::AsyncUDPSocket::WriteOptions(gsoVal, false /* zerocopyVal */));
/** /**
* If there is one more bytes after lastPacketEnd_, that means there is a * If there is one more bytes after lastPacketEnd_, that means there is a
* packet we choose not to write in this batch (e.g., it has a size larger * packet we choose not to write in this batch (e.g., it has a size larger

View File

@@ -515,10 +515,11 @@ TEST_P(QuicBatchWriterTest, InplaceWriterWriteOne) {
ASSERT_FALSE( ASSERT_FALSE(
batchWriter->append(nullptr, 1000, folly::SocketAddress(), nullptr)); batchWriter->append(nullptr, 1000, folly::SocketAddress(), nullptr));
EXPECT_CALL(sock, write(_, _)) EXPECT_CALL(sock, writeGSO(_, _, _))
.Times(1) .Times(1)
.WillOnce(Invoke([&](const auto& /* addr */, .WillOnce(Invoke([&](const auto& /* addr */,
const std::unique_ptr<folly::IOBuf>& buf) { const std::unique_ptr<folly::IOBuf>& buf,
auto) {
EXPECT_EQ(1000, buf->length()); EXPECT_EQ(1000, buf->length());
return 1000; return 1000;
})); }));
@@ -606,10 +607,11 @@ TEST_P(QuicBatchWriterTest, InplaceWriterBufResidueCheck) {
rawBuf->append(packetSizeBig); rawBuf->append(packetSizeBig);
EXPECT_TRUE(batchWriter->needsFlush(packetSizeBig)); EXPECT_TRUE(batchWriter->needsFlush(packetSizeBig));
EXPECT_CALL(sock, write(_, _)) EXPECT_CALL(sock, writeGSO(_, _, _))
.Times(1) .Times(1)
.WillOnce(Invoke([&](const auto& /* addr */, .WillOnce(Invoke([&](const auto& /* addr */,
const std::unique_ptr<folly::IOBuf>& buf) { const std::unique_ptr<folly::IOBuf>& buf,
auto) {
EXPECT_EQ(700, buf->length()); EXPECT_EQ(700, buf->length());
return 700; return 700;
})); }));

View File

@@ -4351,10 +4351,11 @@ TEST_F(QuicTransportFunctionsTest, WriteWithInplaceBuilder) {
auto stream = conn->streamManager->createNextBidirectionalStream().value(); auto stream = conn->streamManager->createNextBidirectionalStream().value();
auto buf = folly::IOBuf::copyBuffer("Andante in C minor"); auto buf = folly::IOBuf::copyBuffer("Andante in C minor");
writeDataToQuicStream(*stream, buf->clone(), true); writeDataToQuicStream(*stream, buf->clone(), true);
EXPECT_CALL(mockSock, write(_, _)) EXPECT_CALL(mockSock, writeGSO(_, _, _))
.Times(1) .Times(1)
.WillOnce(Invoke([&](const SocketAddress&, .WillOnce(Invoke([&](const SocketAddress&,
const std::unique_ptr<folly::IOBuf>& sockBuf) { const std::unique_ptr<folly::IOBuf>& sockBuf,
auto) {
EXPECT_GT(bufPtr->length(), 0); EXPECT_GT(bufPtr->length(), 0);
EXPECT_GE(sockBuf->length(), buf->length()); EXPECT_GE(sockBuf->length(), buf->length());
EXPECT_EQ(sockBuf.get(), bufPtr); EXPECT_EQ(sockBuf.get(), bufPtr);
@@ -4499,10 +4500,11 @@ TEST_F(QuicTransportFunctionsTest, WriteProbingWithInplaceBuilder) {
conn->outstandings.packets.front().metadata.encodedSize; conn->outstandings.packets.front().metadata.encodedSize;
auto outstandingPacketsCount = conn->outstandings.packets.size(); auto outstandingPacketsCount = conn->outstandings.packets.size();
ASSERT_EQ(firstPacketSize, conn->udpSendPacketLen); ASSERT_EQ(firstPacketSize, conn->udpSendPacketLen);
EXPECT_CALL(mockSock, write(_, _)) EXPECT_CALL(mockSock, writeGSO(_, _, _))
.Times(1) .Times(1)
.WillOnce(Invoke([&](const folly::SocketAddress&, .WillOnce(Invoke([&](const folly::SocketAddress&,
const std::unique_ptr<folly::IOBuf>& buf) { const std::unique_ptr<folly::IOBuf>& buf,
auto) {
EXPECT_FALSE(buf->isChained()); EXPECT_FALSE(buf->isChained());
EXPECT_EQ(buf->length(), firstPacketSize); EXPECT_EQ(buf->length(), firstPacketSize);
return buf->length(); return buf->length();