On Sun, Jan 24, 2016 at 07:51:16PM +0100, Cyril Bonté wrote:
> Le 24/01/2016 19:42, Willy Tarreau a écrit :
> >- when I tried to lower the buffer size, I entered exactly 16000. For a
> > reason I ignore, the transfer hangs after the end of the first
> > transfer.
> > I have not investigated this and it could be my server (a hacked
> > haproxy
> > 1.6 modified to implement the good old httpterm). So I don't consider
> > this a real issue until investigated for real.
>
> It hangs here too (I'm using apache as a backend server).
OK so I'm having a preliminary patch for the performance regression
causing incomplete buffers to be processed. It's not optimal yet,
but it's safe and works for me. Now I can reproduce the tests and
always observe the exact same performance between all runs +/- 0.2%.
It clearly outlines the effect of tune.bufsize which causes SSL to
fragment the output. With tune.bufsize 16300 I'm seeing 400.0 MB/s
and with 16384 (16392 effectively) I'm seeing 363.4 MB/s. I feel
uncomfortable with the hang issue I'm noticing at certain sizes,
and I tracked it down to something related to keepalive because
I'm seeing that we're waiting for some data after the expected
amount of data came from the server. Maybe it's not properly
accounted for somewhere.
I'm attaching the patch so that we can make progress on this issue.
We'll have to backport it to 1.6 and 1.5 later once written in a
more optimal way.
Cheers,
Willy
diff --git a/include/proto/channel.h b/include/proto/channel.h
index 733f9d2..e59a555 100644
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -158,32 +158,6 @@ static inline int channel_may_send(const struct channel
*chn)
return chn_cons(chn)->state == SI_ST_EST;
}
-/* Returns the amount of bytes from the channel that are already scheduled for
- * leaving (buf->o) or that are still part of the input and expected to be sent
- * soon as covered by to_forward. This is useful to know by how much we can
- * shrink the rewrite reserve during forwards. Buffer data are not considered
- * in transit until the channel is connected, so that the reserve remains
- * protected.
- */
-static inline int channel_in_transit(const struct channel *chn)
-{
- int ret;
-
- if (!channel_may_send(chn))
- return 0;
-
- /* below, this is min(i, to_forward) optimized for the fast case */
- if (chn->to_forward >= chn->buf->i ||
- (CHN_INFINITE_FORWARD < MAX_RANGE(typeof(chn->buf->i)) &&
- chn->to_forward == CHN_INFINITE_FORWARD))
- ret = chn->buf->i;
- else
- ret = chn->to_forward;
-
- ret += chn->buf->o;
- return ret;
-}
-
/* Returns non-zero if the channel can still receive data. This is used to
* decide when to stop reading into a buffer when we want to ensure that we
* leave the reserve untouched after all pending outgoing data are forwarded.
@@ -324,30 +298,48 @@ static inline void channel_dont_read(struct channel *chn)
/*************************************************/
-/* Return the number of reserved bytes in the channel's visible
- * buffer, which ensures that once all pending data are forwarded, the
- * buffer still has global.tune.maxrewrite bytes free. The result is
- * between 0 and global.tune.maxrewrite, which is itself smaller than
- * any chn->size. Special care is taken to avoid any possible integer
- * overflow in the operations.
- */
-static inline int channel_reserved(const struct channel *chn)
-{
- int reserved;
-
- reserved = global.tune.maxrewrite - channel_in_transit(chn);
- if (reserved < 0)
- reserved = 0;
- return reserved;
-}
-
/* Return the max number of bytes the buffer can contain so that once all the
* data in transit are forwarded, the buffer still has global.tune.maxrewrite
* bytes free. The result sits between chn->size - maxrewrite and chn->size.
+ * The principle is the following :
+ * - all output bytes are ignored, they're leaving
+ * - all input bytes covered by to_forward are considered in transit and
+ * virtually don't take room
+ * - the reserve may be covered up to the min of (fwd-transit) since these
+ * bytes will be in transit later thus will only take temporary space.
+ *
+ * So the formula is to return this limit is :
+ * size - maxrewrite + min(fwd - min(i, fwd), maxrewrite)
+ * = size - maxrewrite + min( min(fwd - i, 0), maxrewrite)
*/
static inline int channel_recv_limit(const struct channel *chn)
{
- return chn->buf->size - channel_reserved(chn);
+ int transit;
+ int reserve;
+
+ if (chn->buf == &buf_empty)
+ return 0;
+
+ if (!channel_may_send(chn))
+ return chn->buf->size - global.tune.maxrewrite;
+
+ /* in practice we never see this case since nobody uses 2GB buffers.
+ * The test is here so that the compiler takes care of it.
+ */
+ if (CHN_INFINITE_FORWARD < MAX_RANGE(typeof(chn->buf->i)) &&
+ chn->to_forward == CHN_INFINITE_FORWARD)
+ return chn->buf->size;
+
+
+ transit = chn->to_forward - chn->buf->i;
+ if (transit < 0)
+ transit = 0;
+
+ reserve = global.tune.maxrewrite - transit;
+ if (reserve < 0)
+ reserve = 0;
+
+ return chn->buf->size - reserve;
}
/* Returns the amount of space available at the input of the buffer, taking the
