MEDIUM: mux-h2: avoid doing expensive buffer realigns when not absolutely needed

Transferring large objects over H2 sometimes shows unexplained performance
variations. A long analysis resulted in the following discovery. Often the
mux buffer looks like this :

    [ empty_head |    data     | empty_tail ]

Typical numbers are (very common) :
  - empty_head = 31
  - empty_tail = 16  (total free=47)
  - data = 16337
  - size = 16384
  - data to copy: 43

The reason for these holes are the blocking factors that are not always
the same in and out (due to keeping 9 bytes for the frame size, or the
56 bytes corresponding to the HTX header). This can easily happen 10000
times a second if the network bandwidth permits it!

In this case, while copying a DATA frame we find that the buffer has its
free space wrapped so we decide to realign it to optimize the copy. It's
possible that this practice stems from the code used to emit headers,
which do not support fragmentation and which had no other option left.
But it comes with two problems :
  - we don't check if the data fits, which results in a memcpy for nothing
  - we can move huge amounts of data to just copy a small block.

This patch addresses this two ways :
  - first, by not forcing a data realignment if what we have to copy does
    not fit, as this is totally pointless ;

  - second, by refusing to move too large data blocks. The threshold was
    set to 1 kB, because it may make sense to move 1 kB of data to copy
    a 15 kB one at once, which will leave as a single 16 kB block, but
    it doesn't make sense to mvoe 15 kB to copy just 1 kB. In all cases
    the data would fit and would just be split into two blocks, which is
    not very expensive, hence the low limit to 1 kB

With such changes, realignments are very rare, they show up around once
every 15 seconds at 60 Gbps, and look like this, resulting in a much more
stable bit rate :

  buf=0x7fe6ec0c3510,h=16333,d=35,s=16384 room=16349 in=16337

This patch should be safe for backporting to 1.9 if some performance
issues are reported there.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 108d8eeba4950c1c1c5d628278dcdc72545ce206..8c696bf68800dcb6f958d1fffa9c0c594ec746db 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -239,6 +239,9 @@ DECLARE_STATIC_POOL(pool_head_h2s, "h2s", sizeof(struct h2s));
  */
 #define H2_INITIAL_WINDOW_INCREMENT ((1U<<31)-1 - 65535)
 
+/* maximum amount of data we're OK with re-aligning for buffer optimizations */
+#define MAX_DATA_REALIGN 1024
+
 /* a few settings from the global section */
 static int h2_settings_header_table_size      =  4096; /* initial value */
 static int h2_settings_initial_window_size    = 65535; /* initial value */
@@ -3749,9 +3752,12 @@ try_again:
                }
                goto try_again;
        }
-       else if (unlikely(b_space_wraps(csbuf))) {
-               /* it doesn't fit and the buffer is fragmented,
-                * so let's defragment it and try again.
+       else if (unlikely(b_space_wraps(csbuf) &&
+                         flen + chklen <= b_room(csbuf) &&
+                         b_data(csbuf) <= MAX_DATA_REALIGN)) {
+               /* it doesn't fit in a single block and the buffer is fragmented, if there are
+                * not too many data in the buffer, let's defragment it and try
+                * again.
                 */
                b_slow_realign(csbuf, trash.area, 0);
        }
@@ -4181,10 +4187,13 @@ static size_t h2s_frt_make_resp_data(struct h2s *h2s, const struct buffer *buf,
                size = h2c->mfs;
 
        if (size + 9 > outbuf.size) {
-               /* we have an opportunity for enlarging the too small
-                * available space, let's try.
+               /* It doesn't fit at once. If it at least fits once split and
+                * the amount of data to move is low, let's defragment the
+                * buffer now.
                 */
-               if (b_space_wraps(&h2c->mbuf))
+               if (b_space_wraps(&h2c->mbuf) &&
+                   (size + 9 <= b_room(&h2c->mbuf)) &&
+                   b_data(&h2c->mbuf) <= MAX_DATA_REALIGN)
                        goto realign_again;
                size = outbuf.size - 9;
        }
@@ -4936,13 +4945,13 @@ static size_t h2s_htx_frt_make_resp_data(struct h2s *h2s, struct buffer *buf, si
                fsize = h2c->mfs; // >0
 
        if (fsize + 9 > outbuf.size) {
-               /* we have an opportunity for enlarging the too small
-                * available space, let's try.
-                * FIXME: is this really interesting to do? Maybe we'll
-                * spend lots of time realigning instead of using two
-                * frames.
+               /* It doesn't fit at once. If it at least fits once split and
+                * the amount of data to move is low, let's defragment the
+                * buffer now.
                 */
-               if (b_space_wraps(&h2c->mbuf))
+               if (b_space_wraps(&h2c->mbuf) &&
+                   (fsize + 9 <= b_room(&h2c->mbuf)) &&
+                   b_data(&h2c->mbuf) <= MAX_DATA_REALIGN)
                        goto realign_again;
                fsize = outbuf.size - 9;