From b9d8ac8780d7cc13adbcc9bacc38f7f47e5510c2 Mon Sep 17 00:00:00 2001 From: Antonio Vicente Date: Tue, 27 Oct 2020 19:28:03 -0400 Subject: [PATCH] Improve naming of data member and member function Signed-off-by: Antonio Vicente --- source/common/network/connection_impl.cc | 33 +++++++++++++----------- source/common/network/connection_impl.h | 13 +++++----- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 6f46e580d441..f77e6241f781 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -58,7 +58,7 @@ ConnectionImpl::ConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPt write_buffer_above_high_watermark_(false), detect_early_close_(true), enable_half_close_(false), read_end_stream_raised_(false), read_end_stream_(false), write_end_stream_(false), current_write_end_stream_(false), dispatch_buffered_data_(false), - want_read_(false) { + transport_wants_read_(false) { if (!connected) { connecting_ = true; @@ -184,7 +184,7 @@ Connection::State ConnectionImpl::state() const { void ConnectionImpl::closeConnectionImmediately() { closeSocket(ConnectionEvent::LocalClose); } -bool ConnectionImpl::consumerWantsToRead() { +bool ConnectionImpl::filterChainWantsData() { return read_disable_count_ == 0 || (read_disable_count_ == 1 && read_buffer_.highWatermarkTriggered()); } @@ -264,7 +264,7 @@ void ConnectionImpl::noDelay(bool enable) { } void ConnectionImpl::onRead(uint64_t read_buffer_size) { - if (inDelayedClose() || !consumerWantsToRead()) { + if (inDelayedClose() || !filterChainWantsData()) { return; } ASSERT(ioHandle().isOpen()); @@ -350,14 +350,17 @@ void ConnectionImpl::readDisable(bool disable) { file_event_->setEnabled(Event::FileReadyType::Read | Event::FileReadyType::Write); } - if ((consumerWantsToRead() && read_buffer_.length() > 0) || - (read_disable_count_ == 0 && want_read_)) { - // If the read_buffer_ is not empty or want_read_ is true, the connection may be able to - // process additional bytes even if there is no data in the kernel to kick off the filter - // chain. Alternately if the read buffer has data the fd could be read disabled. To handle - // these cases, fake an event to make sure the buffered data in the read buffer or in + if (filterChainWantsData() && (read_buffer_.length() > 0 || transport_wants_read_)) { + // If the read_buffer_ is not empty or transport_wants_read_ is true, the connection may be + // able to process additional bytes even if there is no data in the kernel to kick off the + // filter chain. Alternately if the read buffer has data the fd could be read disabled. To + // handle these cases, fake an event to make sure the buffered data in the read buffer or in // transport socket internal buffers gets processed regardless and ensure that we dispatch it // via onRead. + + // Sanity check: resumption with read_disable_count_ > 0 should only happen if the read + // buffer's high watermark has triggered. + ASSERT(read_buffer_.length() > 0 || read_disable_count_ == 0); dispatch_buffered_data_ = true; setReadBufferReady(); } @@ -552,19 +555,19 @@ void ConnectionImpl::onReadReady() { // 2) The consumer of connection data called readDisable(true), and instead of reading from the // socket we simply need to dispatch already read data. if (read_disable_count_ != 0) { - // Do not clear want_read_ when returning early; the early return skips the transport socket - // doRead call. - if (latched_dispatch_buffered_data && consumerWantsToRead()) { + // Do not clear transport_wants_read_ when returning early; the early return skips the transport + // socket doRead call. + if (latched_dispatch_buffered_data && filterChainWantsData()) { onRead(read_buffer_.length()); } return; } - // Clear want_read_ just before the call to doRead. This is the only way to ensure that the - // transport socket read resumption happens as requested; onReadReady() returns early without + // Clear transport_wants_read_ just before the call to doRead. This is the only way to ensure that + // the transport socket read resumption happens as requested; onReadReady() returns early without // reading from the transport if the read buffer is above high watermark at the start of the // method. - want_read_ = false; + transport_wants_read_ = false; IoResult result = transport_socket_->doRead(read_buffer_); uint64_t new_buffer_size = read_buffer_.length(); updateReadBufferStats(result.bytes_processed_, new_buffer_size); diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h index f3bb53331686..a3c36eac3fed 100644 --- a/source/common/network/connection_impl.h +++ b/source/common/network/connection_impl.h @@ -117,7 +117,7 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback // fair sharing of CPU resources, the underlying event loop does not make any fairness guarantees. // Reconsider how to make fairness happen. void setReadBufferReady() override { - want_read_ = true; + transport_wants_read_ = true; file_event_->activate(Event::FileReadyType::Read); } void flushWriteBuffer() override; @@ -129,11 +129,10 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback // A convenience function which returns true if // 1) The read disable count is zero or // 2) The read disable count is one due to the read buffer being overrun. - // In either case the consumer of the data would like to read from the buffer. - // If the read count is greater than one, or equal to one when the buffer is - // not overrun, then the consumer of the data has called readDisable, and does - // not want to read. - bool consumerWantsToRead(); + // In either case the filter chain would like to process data from the read buffer or transport + // socket. If the read count is greater than one, or equal to one when the buffer is not overrun, + // then the filter chain has called readDisable, and does not want additional data. + bool filterChainWantsData(); // Network::ConnectionImplBase void closeConnectionImmediately() final; @@ -203,7 +202,7 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback // to schedule read resumption after yielding due to shouldDrainReadBuffer(). When true, // readDisable must schedule read resumption when read_disable_count_ == 0 to ensure that read // resumption happens when remaining bytes are held in transport socket internal buffers. - bool want_read_ : 1; + bool transport_wants_read_ : 1; }; /**