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;