Create IOBuf on the stack using folly::IOBuf::wrapBufferAsValue for body

Summary: This is similar to the previous commit. We use a stack-based IOBuf instead of allocating one on the heap. This saves CPU because allocations/deallocations on the stack are a lot cheaper.

Reviewed By: hanidamlaj

Differential Revision: D53101722

fbshipit-source-id: dd59a7eca6498db19472a62f954db3e2f2f27a42

quic/api/QuicTransportFunctions.cpp

@@ -261,7 +261,7 @@ DataPathResult continuousMemoryBuildScheduleEncrypt(
     }
     return DataPathResult::makeBuildFailure();
   }
-  if (!packet->body) {
+  if (packet->body.empty()) {
     // No more space remaining.
     rollbackBuf();
     ioBufBatch.flush();
@@ -274,8 +274,7 @@ DataPathResult continuousMemoryBuildScheduleEncrypt(
   auto headerLen = packet->header.length();
   buf = connection.bufAccessor->obtain();
   CHECK(
-      packet->body->data() > buf->data() &&
-      packet->body->tail() <= buf->tail());
+      packet->body.data() > buf->data() && packet->body.tail() <= buf->tail());
   CHECK(
       packet->header.data() >= buf->data() &&
       packet->header.tail() < buf->tail());
@@ -345,7 +344,7 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt(
     }
     return DataPathResult::makeBuildFailure();
   }
-  if (!packet->body) {
+  if (packet->body.empty()) {
     // No more space remaining.
     ioBufBatch.flush();
     if (connection.loopDetectorCallback) {
@@ -355,10 +354,10 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt(
   }
   packet->header.coalesce();
   auto headerLen = packet->header.length();
-  auto bodyLen = packet->body->computeChainDataLength();
+  auto bodyLen = packet->body.computeChainDataLength();
   auto unencrypted = folly::IOBuf::createCombined(
       headerLen + bodyLen + aead.getCipherOverhead());
-  auto bodyCursor = folly::io::Cursor(packet->body.get());
+  auto bodyCursor = folly::io::Cursor(&packet->body);
   bodyCursor.pull(unencrypted->writableData() + headerLen, bodyLen);
   unencrypted->advance(headerLen);
   unencrypted->append(bodyLen);
@@ -1263,20 +1262,21 @@ void writeCloseCommon(
   }
   auto packet = std::move(packetBuilder).buildPacket();
   packet.header.coalesce();
-  packet.body->reserve(0, aead.getCipherOverhead());
-  CHECK_GE(packet.body->tailroom(), aead.getCipherOverhead());
-  auto body =
-      aead.inplaceEncrypt(std::move(packet.body), &packet.header, packetNum);
-  body->coalesce();
+  packet.body.reserve(0, aead.getCipherOverhead());
+  CHECK_GE(packet.body.tailroom(), aead.getCipherOverhead());
+  auto bufUniquePtr = packet.body.clone();
+  bufUniquePtr =
+      aead.inplaceEncrypt(std::move(bufUniquePtr), &packet.header, packetNum);
+  bufUniquePtr->coalesce();
   encryptPacketHeader(
       headerForm,
       packet.header.writableData(),
       packet.header.length(),
-      body->data(),
-      body->length(),
+      bufUniquePtr->data(),
+      bufUniquePtr->length(),
       headerCipher);
   folly::IOBuf packetBuf(std::move(packet.header));
-  packetBuf.prependChain(std::move(body));
+  packetBuf.prependChain(std::move(bufUniquePtr));
   auto packetSize = packetBuf.computeChainDataLength();
   if (connection.qLogger) {
     connection.qLogger->addPacket(packet.packet, packetSize);

quic/api/test/QuicPacketSchedulerTest.cpp

@@ -151,7 +151,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoPaddingInitialPacket) {
   auto result = cryptoOnlyScheduler.scheduleFramesForPacket(
       std::move(builder), conn.udpSendPacketLen);
   auto packetLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   EXPECT_EQ(conn.udpSendPacketLen, packetLength);
 }
 
@@ -184,7 +184,7 @@ TEST_F(QuicPacketSchedulerTest, PaddingInitialPureAcks) {
   auto result = acksOnlyScheduler.scheduleFramesForPacket(
       std::move(builder), conn.udpSendPacketLen);
   auto packetLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   EXPECT_EQ(conn.udpSendPacketLen, packetLength);
 }
 
@@ -218,7 +218,7 @@ TEST_F(QuicPacketSchedulerTest, InitialPaddingDoesNotUseWrapper) {
   auto result = acksOnlyScheduler.scheduleFramesForPacket(
       std::move(builder), conn.udpSendPacketLen - cipherOverhead);
   auto packetLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   EXPECT_EQ(conn.udpSendPacketLen, packetLength);
 }
 
@@ -251,7 +251,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoServerInitialPadded) {
   auto result = scheduler.scheduleFramesForPacket(
       std::move(builder1), conn.udpSendPacketLen);
   auto packetLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   EXPECT_EQ(conn.udpSendPacketLen, packetLength);
 }
 
@@ -284,7 +284,7 @@ TEST_F(QuicPacketSchedulerTest, PadTwoInitialPackets) {
   auto result = scheduler.scheduleFramesForPacket(
       std::move(builder1), conn.udpSendPacketLen);
   auto packetLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   EXPECT_EQ(conn.udpSendPacketLen, packetLength);
 
   increaseNextPacketNum(conn, PacketNumberSpace::Initial);
@@ -303,7 +303,7 @@ TEST_F(QuicPacketSchedulerTest, PadTwoInitialPackets) {
   auto result2 = scheduler.scheduleFramesForPacket(
       std::move(builder2), conn.udpSendPacketLen);
   packetLength = result2.packet->header.computeChainDataLength() +
-      result2.packet->body->computeChainDataLength();
+      result2.packet->body.computeChainDataLength();
   EXPECT_EQ(conn.udpSendPacketLen, packetLength);
 }
 
@@ -335,7 +335,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoPaddingRetransmissionClientInitial) {
   auto result = scheduler.scheduleFramesForPacket(
       std::move(builder), conn.udpSendPacketLen);
   auto packetLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   EXPECT_EQ(conn.udpSendPacketLen, packetLength);
 }
 
@@ -398,7 +398,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoWritePartialLossBuffer) {
   auto result = cryptoOnlyScheduler.scheduleFramesForPacket(
       std::move(builder), conn.udpSendPacketLen);
   auto packetLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   EXPECT_LE(packetLength, 25);
   EXPECT_TRUE(result.packet->packet.frames[0].asWriteCryptoFrame() != nullptr);
   EXPECT_FALSE(conn.cryptoState->initialStream.lossBuffer.empty());
@@ -806,7 +806,9 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) {
                 folly::IOBuf(
                     folly::IOBuf::CopyBufferOp::COPY_BUFFER,
                     "if you are the dealer"),
-                folly::IOBuf::copyBuffer("I'm out of the game"));
+                folly::IOBuf(
+                    folly::IOBuf::CopyBufferOp::COPY_BUFFER,
+                    "I'm out of the game"));
             return SchedulingResult(folly::none, std::move(builtPacket));
           }));
   RegularQuicPacketBuilder builder(
@@ -831,7 +833,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) {
       *folly::IOBuf::copyBuffer("if you are the dealer"),
       result.packet->header));
   EXPECT_TRUE(folly::IOBufEqualTo{}(
-      *folly::IOBuf::copyBuffer("I'm out of the game"), *result.packet->body));
+      *folly::IOBuf::copyBuffer("I'm out of the game"), result.packet->body));
 }
 
 TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) {
@@ -992,7 +994,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilderFullPacket) {
   auto result = scheduler.scheduleFramesForPacket(
       std::move(builder), conn.udpSendPacketLen);
   auto bufferLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   EXPECT_EQ(conn.udpSendPacketLen, bufferLength);
   updateConnection(
       conn,
@@ -1063,7 +1065,7 @@ TEST_F(QuicPacketSchedulerTest, CloneLargerThanOriginalPacket) {
       conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0));
   auto packetResult = scheduler.scheduleFramesForPacket(
       std::move(builder), conn.udpSendPacketLen - cipherOverhead);
-  auto encodedSize = packetResult.packet->body->computeChainDataLength() +
+  auto encodedSize = packetResult.packet->body.computeChainDataLength() +
       packetResult.packet->header.computeChainDataLength() + cipherOverhead;
   EXPECT_EQ(encodedSize, conn.udpSendPacketLen);
   updateConnection(
@@ -2402,14 +2404,14 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingWithSpaceForPadding) {
       std::move(builder2), conn.udpSendPacketLen);
 
   auto headerLength1 = result1.packet->header.computeChainDataLength();
-  auto bodyLength1 = result1.packet->body->computeChainDataLength();
+  auto bodyLength1 = result1.packet->body.computeChainDataLength();
   auto packetLength1 = headerLength1 + bodyLength1;
   auto expectedPadding1 =
       (conn.udpSendPacketLen - (inputDataLength1 + headerLength1)) %
       paddingModulo;
 
   auto headerLength2 = result2.packet->header.computeChainDataLength();
-  auto bodyLength2 = result2.packet->body->computeChainDataLength();
+  auto bodyLength2 = result2.packet->body.computeChainDataLength();
   auto packetLength2 = headerLength2 + bodyLength2;
   auto expectedPadding2 =
       (conn.udpSendPacketLen - (inputDataLength2 + headerLength2)) %
@@ -2460,7 +2462,7 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingNearMaxPacketLength) {
       std::move(builder), conn.udpSendPacketLen);
 
   auto headerLength = result.packet->header.computeChainDataLength();
-  auto bodyLength = result.packet->body->computeChainDataLength();
+  auto bodyLength = result.packet->body.computeChainDataLength();
 
   auto packetLength = headerLength + bodyLength;
 
@@ -2515,7 +2517,7 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingMaxPacketLength) {
       std::move(builder), conn.udpSendPacketLen);
 
   auto headerLength = result.packet->header.computeChainDataLength();
-  auto bodyLength = result.packet->body->computeChainDataLength();
+  auto bodyLength = result.packet->body.computeChainDataLength();
 
   auto packetLength = headerLength + bodyLength;
 
@@ -2554,7 +2556,7 @@ TEST_F(QuicPacketSchedulerTest, ImmediateAckFrameSchedulerOnRequest) {
   auto result = immediateAckOnlyScheduler.scheduleFramesForPacket(
       std::move(builder), conn.udpSendPacketLen);
   auto packetLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   EXPECT_EQ(conn.udpSendPacketLen, packetLength);
 }
 
@@ -2590,7 +2592,7 @@ TEST_F(QuicPacketSchedulerTest, ImmediateAckFrameSchedulerNotRequested) {
   auto result = immediateAckOnlyScheduler.scheduleFramesForPacket(
       std::move(builder), conn.udpSendPacketLen);
   auto packetLength = result.packet->header.computeChainDataLength() +
-      result.packet->body->computeChainDataLength();
+      result.packet->body.computeChainDataLength();
   // The immediate ACK scheduler was not triggered. This packet has no frames
   // and it shouldn't get padded.
   EXPECT_LT(packetLength, conn.udpSendPacketLen);

quic/api/test/QuicTransportFunctionsTest.cpp

@@ -149,17 +149,17 @@ uint64_t getEncodedSize(const RegularQuicPacketBuilder::Packet& packet) {
   if (!packet.header.empty()) {
     encodedSize += packet.header.computeChainDataLength();
   }
-  if (packet.body) {
-    encodedSize += packet.body->computeChainDataLength();
+  if (!packet.body.empty()) {
+    encodedSize += packet.body.computeChainDataLength();
   }
   return encodedSize;
 }
 
 uint64_t getEncodedBodySize(const RegularQuicPacketBuilder::Packet& packet) {
   // calculate size as the plaintext size
   uint32_t encodedBodySize = 0;
-  if (packet.body) {
-    encodedBodySize += packet.body->computeChainDataLength();
+  if (!packet.body.empty()) {
+    encodedBodySize += packet.body.computeChainDataLength();
   }
   return encodedBodySize;
 }
@@ -1177,7 +1177,8 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) {
   auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake);
   auto packetEncodedSize =
       !packet.header.empty() ? packet.header.computeChainDataLength() : 0;
-  packetEncodedSize += packet.body ? packet.body->computeChainDataLength() : 0;
+  packetEncodedSize +=
+      !packet.body.empty() ? packet.body.computeChainDataLength() : 0;
 
   packet.packet.frames.push_back(WriteCryptoFrame(0, 0));
   updateConnection(
@@ -1194,8 +1195,9 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) {
   packetEncodedSize = !nonHandshake.header.empty()
       ? nonHandshake.header.computeChainDataLength()
       : 0;
-  packetEncodedSize +=
-      nonHandshake.body ? nonHandshake.body->computeChainDataLength() : 0;
+  packetEncodedSize += !nonHandshake.body.empty()
+      ? nonHandshake.body.computeChainDataLength()
+      : 0;
   auto stream1 = conn->streamManager->createNextBidirectionalStream().value();
   writeDataToQuicStream(*stream1, nullptr, true);
 

quic/codec/QuicPacketBuilder.cpp

@@ -131,9 +131,9 @@ RegularQuicPacketBuilder::RegularQuicPacketBuilder(
       largestAckedPacketNum_(largestAckedPacketNum),
       packet_(std::move(header)),
       header_(folly::IOBuf::CreateOp::CREATE, kLongHeaderHeaderSize),
-      body_(folly::IOBuf::create(kAppenderGrowthSize)),
+      body_(folly::IOBuf::CreateOp::CREATE, kAppenderGrowthSize),
       headerAppender_(&header_, kLongHeaderHeaderSize),
-      bodyAppender_(body_.get(), kAppenderGrowthSize) {
+      bodyAppender_(&body_, kAppenderGrowthSize) {
   if (frameHint) {
     packet_.frames.reserve(frameHint);
   }
@@ -236,7 +236,7 @@ RegularQuicPacketBuilder::Packet RegularQuicPacketBuilder::buildPacket() && {
   size_t minBodySize = kMaxPacketNumEncodingSize -
       packetNumberEncoding_->length + sizeof(Sample);
   size_t extraDataWritten = 0;
-  size_t bodyLength = body_->computeChainDataLength();
+  size_t bodyLength = body_.computeChainDataLength();
   while (bodyLength + extraDataWritten + cipherOverhead_ < minBodySize &&
          !packet_.empty && remainingBytes_ > kMaxPacketLenSize) {
     // We can add padding frames, but we don't need to store them.
@@ -246,7 +246,7 @@ RegularQuicPacketBuilder::Packet RegularQuicPacketBuilder::buildPacket() && {
   }
   if (longHeader && longHeader->getHeaderType() != LongHeader::Types::Retry) {
     QuicInteger pktLen(
-        packetNumberEncoding_->length + body_->computeChainDataLength() +
+        packetNumberEncoding_->length + body_.computeChainDataLength() +
         cipherOverhead_);
     pktLen.encode([&](auto val) { headerAppender_.writeBE(val); });
     appendBytes(
@@ -389,7 +389,7 @@ RegularSizeEnforcedPacketBuilder::RegularSizeEnforcedPacketBuilder(
     : packet_(std::move(packet.packet)),
       header_(std::move(packet.header)),
       body_(std::move(packet.body)),
-      bodyAppender_(body_.get(), kAppenderGrowthSize),
+      bodyAppender_(&body_, kAppenderGrowthSize),
       enforcedSize_(enforcedSize),
       cipherOverhead_(cipherOverhead) {}
 
@@ -399,7 +399,7 @@ bool RegularSizeEnforcedPacketBuilder::canBuildPacket() const noexcept {
   const ShortHeader* shortHeader = packet_.header.asShort();
   // We also don't want to send packets longer than kDefaultMaxUDPPayload
   return shortHeader && enforcedSize_ <= kDefaultMaxUDPPayload &&
-      (body_->computeChainDataLength() + header_.computeChainDataLength() +
+      (body_.computeChainDataLength() + header_.computeChainDataLength() +
            cipherOverhead_ <
        enforcedSize_);
 }
@@ -408,7 +408,7 @@ PacketBuilderInterface::Packet
 RegularSizeEnforcedPacketBuilder::buildPacket() && {
   // Store counters on the stack to overhead from function calls
   size_t extraDataWritten = 0;
-  size_t bodyLength = body_->computeChainDataLength();
+  size_t bodyLength = body_.computeChainDataLength();
   size_t headerLength = header_.computeChainDataLength();
   while (extraDataWritten + bodyLength + headerLength + cipherOverhead_ <
          enforcedSize_) {
@@ -435,7 +435,7 @@ InplaceSizeEnforcedPacketBuilder::InplaceSizeEnforcedPacketBuilder(
 bool InplaceSizeEnforcedPacketBuilder::canBuildPacket() const noexcept {
   const ShortHeader* shortHeader = packet_.header.asShort();
   size_t encryptedPacketSize =
-      header_.length() + body_->length() + cipherOverhead_;
+      header_.length() + body_.length() + cipherOverhead_;
   size_t delta = enforcedSize_ - encryptedPacketSize;
   return shortHeader && enforcedSize_ <= kDefaultMaxUDPPayload &&
       encryptedPacketSize < enforcedSize_ && iobuf_->tailroom() >= delta;
@@ -445,13 +445,13 @@ PacketBuilderInterface::Packet
 InplaceSizeEnforcedPacketBuilder::buildPacket() && {
   // Create bodyWriter
   size_t encryptedPacketSize =
-      header_.length() + body_->length() + cipherOverhead_;
+      header_.length() + body_.length() + cipherOverhead_;
   size_t paddingSize = enforcedSize_ - encryptedPacketSize;
   BufWriter bodyWriter(*iobuf_, paddingSize);
 
   // Store counters on the stack to overhead from function calls
   size_t extraDataWritten = 0;
-  size_t bodyLength = body_->computeChainDataLength();
+  size_t bodyLength = body_.computeChainDataLength();
   size_t headerLength = header_.computeChainDataLength();
   while (extraDataWritten + bodyLength + headerLength + cipherOverhead_ <
          enforcedSize_) {
@@ -463,7 +463,8 @@ InplaceSizeEnforcedPacketBuilder::buildPacket() && {
   PacketBuilderInterface::Packet builtPacket(
       std::move(packet_),
       std::move(header_),
-      folly::IOBuf::wrapBuffer(body_->data(), iobuf_->tail() - body_->data()));
+      folly::IOBuf::wrapBufferAsValue(
+          body_.data(), iobuf_->tail() - body_.data()));
 
   // Release internal iobuf
   bufAccessor_.release(std::move(iobuf_));
@@ -731,9 +732,9 @@ PacketBuilderInterface::Packet InplaceQuicPacketBuilder::buildPacket() && {
       (bodyStart_ ? folly::IOBuf::wrapBufferAsValue(
                         headerStart_, (bodyStart_ - headerStart_))
                   : folly::IOBuf()),
-      (bodyStart_
-           ? folly::IOBuf::wrapBuffer(bodyStart_, iobuf_->tail() - bodyStart_)
-           : nullptr));
+      (bodyStart_ ? folly::IOBuf::wrapBufferAsValue(
+                        bodyStart_, iobuf_->tail() - bodyStart_)
+                  : folly::IOBuf()));
   releaseOutputBufferInternal();
   return builtPacket;
 }