From bca5e142350db02be91c294c92d738a4d767810e Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 27 Sep 2024 15:05:11 +0200 Subject: [PATCH] OPTIM: stconn: Don't pretend mux have more data to deliver on EOI/EOS/ERROR Doing some benchs on the 3.0, we encountered a small loss on requests/sec on small objects compared to the 2.8 . After bisecting the issue, it appeared that this was introduced when the mux-to-mux zero-copy data forwarding was implemented in 2.9-dev8. Extra subscribes on receives at the end of the message were responsible of the loss. A basic configuration, sending H2 requests to a H1 server returning responses without payload is enough to observe the issue. With the following command, we can observe a huge increase of epoll_ctl calls on 2.9/3.x: h2load -c 100 -m 10 -n 100000 http://... On 2.8 we have around 3200 calls to epoll_ctl against more than 20k on 3.1. The fix seems obvious. After a receive, there is no reason to state a mux have more data to deliver if EOI/EOS/ERROR flag was set on the stream-endpoint descriptor. With this change, extra calls to epoll_ctl disappear. However it is a sensitive part so it is important to keep an eye on it and to not backport it. Thanks to Willy and Emeric to have spot the issue. --- src/stconn.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/stconn.c b/src/stconn.c index eacbb5c6be..3fa9dc827c 100644 --- a/src/stconn.c +++ b/src/stconn.c @@ -1520,19 +1520,33 @@ int sc_conn_recv(struct stconn *sc) sc_conn_eos(sc); ret = 1; } - if (sc_ep_test(sc, SE_FL_ERROR)) { sc->flags |= SC_FL_ERROR; ret = 1; } + + if (sc->flags & (SC_FL_EOS|SC_FL_ERROR)) { + /* No more data are expected at this stage */ + se_have_no_more_data(sc->sedesc); + } else if (!cur_read && !(sc->flags & (SC_FL_WONT_READ|SC_FL_NEED_BUFF|SC_FL_NEED_ROOM)) && !(sc->flags & (SC_FL_EOS|SC_FL_ABRT_DONE))) { - /* Subscribe to receive events if we're blocking on I/O */ + /* Subscribe to receive events if we're blocking on I/O. Nothing + * was received and it was not because of a blocking + * condition. + */ conn->mux->subscribe(sc, SUB_RETRY_RECV, &sc->wait_event); se_have_no_more_data(sc->sedesc); } + else if (sc->flags & SC_FL_EOI) { + /* No more data are expected at this stage */ + se_have_no_more_data(sc->sedesc); + } else { + /* The mux may have more data to deliver. Be sure to be able to + * ask it ASAP + */ se_have_more_data(sc->sedesc); ret = 1; } -- 2.39.5