Simplify process_connection_writes condition

And comment explaining that we there's something more precise but more
complex available.
This commit is contained in:
2025-09-10 16:45:04 -04:00
parent f56ed2bfbe
commit 962a010724
3 changed files with 16 additions and 15 deletions

View File

@@ -338,8 +338,8 @@ private:
// Direct access methods for Server // Direct access methods for Server
int getFd() const { return fd_; } int getFd() const { return fd_; }
bool hasMessages() const { return !messages_.empty(); } bool has_messages() const { return !messages_.empty(); }
bool shouldClose() const { return closeConnection_; } bool should_close() const { return closeConnection_; }
size_t getEpollIndex() const { return epoll_index_; } size_t getEpollIndex() const { return epoll_index_; }
const int fd_; const int fd_;
const int64_t id_; const int64_t id_;

View File

@@ -162,7 +162,7 @@ void Server::receiveConnectionBack(std::unique_ptr<Connection> connection) {
// Re-add the connection to epoll for continued processing // Re-add the connection to epoll for continued processing
struct epoll_event event{}; struct epoll_event event{};
if (!connection->hasMessages()) { if (!connection->has_messages()) {
event.events = EPOLLIN | EPOLLONESHOT; event.events = EPOLLIN | EPOLLONESHOT;
} else { } else {
event.events = EPOLLOUT | EPOLLONESHOT; event.events = EPOLLOUT | EPOLLONESHOT;
@@ -482,12 +482,13 @@ void Server::process_connection_reads(std::unique_ptr<Connection> &conn,
} }
void Server::process_connection_writes(std::unique_ptr<Connection> &conn, void Server::process_connection_writes(std::unique_ptr<Connection> &conn,
int events) { int /*events*/) {
assert(conn); assert(conn);
// Send immediately if we have outgoing messages (either from EPOLLOUT or // For simplicity, we always attempt to write when an event fires. We could be
// after reading) // more precise and skip the write if we detect that we've already seen EAGAIN
if ((events & EPOLLOUT) || ((events & EPOLLIN) && conn->hasMessages())) { // on this connection and we don't have EPOLLOUT.
bool had_messages = conn->hasMessages(); if (conn->has_messages()) {
bool had_messages = conn->has_messages();
bool error = conn->writeBytes(); bool error = conn->writeBytes();
if (error) { if (error) {
@@ -504,7 +505,7 @@ void Server::process_connection_writes(std::unique_ptr<Connection> &conn,
} }
// Check if buffer became empty (transition from non-empty -> empty) // Check if buffer became empty (transition from non-empty -> empty)
if (had_messages && !conn->hasMessages()) { if (had_messages && !conn->has_messages()) {
handler_.on_write_buffer_drained(conn); handler_.on_write_buffer_drained(conn);
// If handler took ownership (conn is now null), return // If handler took ownership (conn is now null), return
if (!conn) { if (!conn) {
@@ -513,7 +514,7 @@ void Server::process_connection_writes(std::unique_ptr<Connection> &conn,
} }
// Check if we should close the connection according to application // Check if we should close the connection according to application
if (!conn->hasMessages() && conn->shouldClose()) { if (!conn->has_messages() && conn->should_close()) {
conn.reset(); // Connection should be closed conn.reset(); // Connection should be closed
return; return;
} }
@@ -547,7 +548,7 @@ void Server::process_connection_batch(
int fd = conn_ptr->getFd(); int fd = conn_ptr->getFd();
struct epoll_event event{}; struct epoll_event event{};
if (!conn_ptr->hasMessages()) { if (!conn_ptr->has_messages()) {
event.events = EPOLLIN | EPOLLONESHOT; event.events = EPOLLIN | EPOLLONESHOT;
} else { } else {
event.events = EPOLLOUT | EPOLLONESHOT; event.events = EPOLLOUT | EPOLLONESHOT;

View File

@@ -248,7 +248,7 @@ struct Connection {
} }
// Match server's connection state management // Match server's connection state management
bool hasMessages() const { return !request.empty(); } bool has_messages() const { return !request.empty(); }
bool error = false; bool error = false;
~Connection() { ~Connection() {
@@ -698,7 +698,7 @@ int main(int argc, char *argv[]) {
// Transfer back to epoll instance. This thread or another thread // Transfer back to epoll instance. This thread or another thread
// will wake when fd is ready // will wake when fd is ready
if (conn->hasMessages()) { if (conn->has_messages()) {
events[i].events = EPOLLOUT | EPOLLONESHOT; events[i].events = EPOLLOUT | EPOLLONESHOT;
} else { } else {
events[i].events = EPOLLIN | EPOLLONESHOT; events[i].events = EPOLLIN | EPOLLONESHOT;
@@ -748,7 +748,7 @@ int main(int argc, char *argv[]) {
// Try to write once in the connect thread before handing off to network // Try to write once in the connect thread before handing off to network
// threads // threads
assert(conn->hasMessages()); assert(conn->has_messages());
bool writeFinished = conn->writeBytes(); bool writeFinished = conn->writeBytes();
if (conn->error) { if (conn->error) {
continue; // Connection failed, destructor will clean up continue; // Connection failed, destructor will clean up
@@ -766,7 +766,7 @@ int main(int argc, char *argv[]) {
event.events = EPOLLIN | EPOLLONESHOT; event.events = EPOLLIN | EPOLLONESHOT;
} else { } else {
event.events = event.events =
(conn->hasMessages() ? EPOLLOUT : EPOLLIN) | EPOLLONESHOT; (conn->has_messages() ? EPOLLOUT : EPOLLIN) | EPOLLONESHOT;
} }
// Add to a round-robin selected epoll instance to distribute load // Add to a round-robin selected epoll instance to distribute load