Remove unnecessary copy in QuicChaosProtector
When QuicChaosProtector was first written, QuicStreamSendBuffer did not allow out-of-order writes. To work around this, QuicChaosProtector copied out the entire crypto data in one go. Now that cl/712548578 has landed, QuicStreamSendBuffer allows out-of-order writes so this copy is no longer required. This CL removes that unnecessary copy.
Protected by FLAGS_quic_reloadable_flag_quic_chaos_protector_avoid_copy.
PiperOrigin-RevId: 712664648
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index 52a8c84..065699e 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -19,6 +19,7 @@
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_block_until_settings_received_copt, true, true, "If enabled and a BSUS connection is received, blocks server connections until SETTINGS frame is received.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_buffered_store_set_client_cid, false, true, "If true, QuicBufferedPacketStore will set client CID in packets it generates.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_can_send_ack_frequency, true, true, "If true, ack frequency frame can be sent from server to client.")
+QUICHE_FLAG(bool, quiche_reloadable_flag_quic_chaos_protector_avoid_copy, false, true, "When true, avoid unnecessary copy in QuicChaosProtector.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_conservative_bursts, false, false, "If true, set burst token to 2 in cwnd bootstrapping experiment.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_conservative_cwnd_and_pacing_gains, false, false, "If true, uses conservative cwnd gain and pacing gain when cwnd gets bootstrapped.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_default_enable_5rto_blackhole_detection2, true, true, "If true, default-enable 5RTO blachole detection.")
diff --git a/quiche/quic/core/quic_chaos_protector.cc b/quiche/quic/core/quic_chaos_protector.cc
index 8295730..f7a8bc2 100644
--- a/quiche/quic/core/quic_chaos_protector.cc
+++ b/quiche/quic/core/quic_chaos_protector.cc
@@ -24,6 +24,8 @@
#include "quiche/quic/core/quic_stream_frame_data_producer.h"
#include "quiche/quic/core/quic_types.h"
#include "quiche/quic/platform/api/quic_bug_tracker.h"
+#include "quiche/quic/platform/api/quic_flag_utils.h"
+#include "quiche/quic/platform/api/quic_flags.h"
#include "quiche/common/platform/api/quiche_logging.h"
#include "quiche/common/simple_buffer_allocator.h"
@@ -228,10 +230,14 @@
QuicChaosProtector::QuicChaosProtector(size_t packet_size,
EncryptionLevel level,
QuicFramer* framer, QuicRandom* random)
- : packet_size_(packet_size),
+ : avoid_copy_(GetQuicReloadableFlag(quic_chaos_protector_avoid_copy)),
+ packet_size_(packet_size),
level_(level),
framer_(framer),
random_(random) {
+ if (avoid_copy_) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_chaos_protector_avoid_copy);
+ }
QUICHE_DCHECK_NE(framer_, nullptr);
QUICHE_DCHECK_NE(framer_->data_producer(), nullptr);
QUICHE_DCHECK_NE(random_, nullptr);
@@ -295,11 +301,13 @@
<< header.packet_number;
return std::nullopt;
}
- if (!CopyCryptoDataToLocalBuffer()) {
- QUIC_DVLOG(1) << "Failed to copy crypto data to local buffer for initial "
- "packet number "
- << header.packet_number;
- return std::nullopt;
+ if (!avoid_copy_) {
+ if (!CopyCryptoDataToLocalBuffer()) {
+ QUIC_DVLOG(1) << "Failed to copy crypto data to local buffer for initial "
+ "packet number "
+ << header.packet_number;
+ return std::nullopt;
+ }
}
SplitCryptoFrame();
AddPingFrames();
@@ -321,6 +329,10 @@
QuicStreamOffset offset,
QuicByteCount data_length,
QuicDataWriter* writer) {
+ if (avoid_copy_) {
+ QUIC_BUG(chaos avoid copy WriteCryptoData) << "This should never be called";
+ return false;
+ }
if (level != level_) {
QUIC_BUG(chaos bad level) << "Unexpected " << level << " != " << level_;
return false;
@@ -341,6 +353,11 @@
}
bool QuicChaosProtector::CopyCryptoDataToLocalBuffer() {
+ if (avoid_copy_) {
+ QUIC_BUG(chaos avoid copy CopyCryptoDataToLocalBuffer)
+ << "This should never be called";
+ return false;
+ }
size_t frame_size = QuicDataWriter::GetVarInt62Len(crypto_buffer_offset_) +
QuicDataWriter::GetVarInt62Len(crypto_data_length_) +
crypto_data_length_;
@@ -480,14 +497,18 @@
std::optional<size_t> QuicChaosProtector::BuildPacket(
const QuicPacketHeader& header, char* buffer) {
- QuicStreamFrameDataProducer* original_data_producer =
- framer_->data_producer();
- framer_->set_data_producer(this);
+ QuicStreamFrameDataProducer* original_data_producer;
+ if (!avoid_copy_) {
+ original_data_producer = framer_->data_producer();
+ framer_->set_data_producer(this);
+ }
size_t length =
framer_->BuildDataPacket(header, frames_, buffer, packet_size_, level_);
- framer_->set_data_producer(original_data_producer);
+ if (!avoid_copy_) {
+ framer_->set_data_producer(original_data_producer);
+ }
if (length == 0) {
QUIC_DVLOG(1) << "Failed to build data packet for initial packet number "
<< header.packet_number;
diff --git a/quiche/quic/core/quic_chaos_protector.h b/quiche/quic/core/quic_chaos_protector.h
index 5dbb3ad..bed2d6f 100644
--- a/quiche/quic/core/quic_chaos_protector.h
+++ b/quiche/quic/core/quic_chaos_protector.h
@@ -154,6 +154,7 @@
std::optional<size_t> BuildPacket(const QuicPacketHeader& header,
char* buffer);
+ const bool avoid_copy_; // Latched from quic_chaos_protector_avoid_copy flag.
size_t packet_size_;
std::unique_ptr<char[]> crypto_frame_buffer_;
const char* crypto_data_buffer_ = nullptr;
diff --git a/quiche/quic/core/quic_chaos_protector_test.cc b/quiche/quic/core/quic_chaos_protector_test.cc
index b090916..6531c10 100644
--- a/quiche/quic/core/quic_chaos_protector_test.cc
+++ b/quiche/quic/core/quic_chaos_protector_test.cc
@@ -299,8 +299,10 @@
QuicByteCount data_length,
QuicDataWriter* writer) override {
EXPECT_EQ(level, level);
- EXPECT_EQ(offset, crypto_offset_);
- EXPECT_EQ(data_length, crypto_data_length_);
+ if (!chaos_protector_->avoid_copy_) {
+ EXPECT_EQ(offset, crypto_offset_);
+ EXPECT_EQ(data_length, crypto_data_length_);
+ }
for (QuicByteCount i = 0; i < data_length; i++) {
EXPECT_TRUE(writer->WriteUInt8(static_cast<uint8_t>((offset + i) & 0xFF)))
<< i;
