Skip to content

Commit

Permalink
Improve naming of data member and member function
Browse files Browse the repository at this point in the history
Signed-off-by: Antonio Vicente <avd@google.com>
  • Loading branch information
antoniovicente committed Oct 27, 2020
1 parent 6647411 commit b9d8ac8
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 22 deletions.
33 changes: 18 additions & 15 deletions source/common/network/connection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 6 additions & 7 deletions source/common/network/connection_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
};

/**
Expand Down

0 comments on commit b9d8ac8

Please sign in to comment.