BUG/MEDIUM: slz: bound the bits wasted by the 9-bit literals

Commit 002e838 ("bug: always make sure to limit fixed output to less than
worst case literals") made sure that switching from EOB to FIXED to emit
a reference is only done when the reference pays for the way back, based
on the fact that once in FIXED state a reference is always smaller than
the bytes it replaces.

This was unfortunately not enough: the literals interleaved with those
references can still fail. Indeed, octets 144 to 255 cost 9 bits instead
of the 8 they would cost in a stored block. The <bit9> counter measures
exactly that, but it was reset after every reference, so a stream
alternating just under 52 such literals with a cheap reference never
reached the threshold that sends them as a stored block, and kept
inflating for as long as the pattern lasted. 51 random literals >= 144
followed by 4 bytes copied from 8 bytes earlier (i.e. almost the exact
same pattern as previously tested) produce 1073161 bytes out of 1048576
(+2.34%) where the API promises at most 1048663, i.e. 24 kB more than a
caller sizing its output buffer from that promise would expect for a
single call.

Bit9 isn't sufficient to track the debt cross references, so let's add a
second <debt> counter to the stream's unused space. It accumulates the
bits actually wasted by the literals emitted in huffman mode, and each
reference records what it saved over the same bytes sent as literals,
bounded to zero. Above SLZ_MAX_DEBT (200 bits, i.e. 25 bytes) the encoder
stops trusting bit9: pending literals are stored and references have to
compensate for the full round trip, which lets the literal runs merge into
a full 65535-byte block and stop the growth.

The crafted stream now produces 1048677 bytes (+14 bytes over the
documented maximum instead of +24509). Other inputs such as text or
silesia corpus do not show any change since it's quite hard to fall
into this case.

Note that the threshold is deliberately much larger than the 52 bits of
a switch to amortize oscillations without needlessly sending literals.

This needs to be backported to 1.2.

This is libslz upstream commit 039fdf8aac3acfdcaa27ae30b387c942bc58ef84.

Note: the impact in haproxy will start with tune.bufsize above 43kB
      for the default 1kB reserve. A simple workaround consists in
      always keeping the reserve (tune.maxrewrite) at least 1/32 of
      bufsize.

diff --git a/include/import/slz.h b/include/import/slz.h
index 10fd34c4216a9aaab73614c2db43864f841a28d7..d8c2eb95776a667e8bd75e8b1652cd2789f7961d 100644 (file)
--- a/include/import/slz.h
+++ b/include/import/slz.h
@@ -80,7 +80,10 @@ struct slz_stream {
        uint16_t state; /* one of slz_state */
        uint8_t level:1; /* 0 = no compression, 1 = compression */
        uint8_t format:2; /* SLZ_FMT_* */
-       uint8_t unused1; /* unused for now */
+       uint8_t debt;    /* number of bits by which the fixed huffman encoding is
+                         * currently behind the equivalent stored blocks, see
+                         * SLZ_MAX_DEBT in slz.c
+                         */
        uint32_t crc32;
        uint32_t ilen;
 };

diff --git a/src/slz.c b/src/slz.c
index 20ba2303664325e53a2b1e61966c6308cb15c19f..1c406c662025c9256c5eeff79a10c21a291a5c94 100644 (file)
--- a/src/slz.c
+++ b/src/slz.c
@@ -523,6 +523,20 @@ static void reset_refs(union ref *refs, long count)
 #define SLZ_LAST_COST        42
 #define SLZ_LAST_COST_EOB    32
 
+/* Maximum number of bits the fixed huffman encoding is allowed to be behind
+ * the equivalent stored blocks before the encoder stops trusting the local
+ * decisions above and falls back to stored blocks only (see <debt> in
+ * slz_rfc1951_encode_blk()). This bounds by 25 bytes the amount by which a
+ * stream may exceed the size of the same data sent as stored blocks. It must
+ * be large enough not to trigger on regular data, where the debt oscillates
+ * around zero: measured on the 212 MB silesia.tar, no value from 196 up
+ * changes the output by a single byte, while 52 costs 2331 bytes and 192
+ * costs 615. It must also leave room for one last group of literals in the
+ * 8-bit counter that holds the debt, which is why it may not exceed
+ * 255 - (SLZ_SWITCH_COST - 1) = 204.
+ */
+#define SLZ_MAX_DEBT        200
+
 /* Compresses <ilen> bytes from <in> into <out> according to RFC1951. The
  * output result may be up to 5 bytes larger than the input for each 65535
  * nput bytes, to which 2 extra bytes may be added to send the last chunk due
@@ -707,7 +721,7 @@ long slz_rfc1951_encode(struct slz_stream *strm, unsigned char *out, const unsig
                 * is what guarantees that once switched to FIXED we can stay
                 * in it for as long as needed.
                 */
-               if (strm->state == SLZ_ST_EOB &&
+               if ((strm->state == SLZ_ST_EOB || strm->debt >= SLZ_MAX_DEBT) &&
                    (dist & 0x1f) + (code >> 16) + SLZ_SWITCH_COST > 8 * mlen)
                        goto send_as_lit;
 
@@ -718,12 +732,17 @@ long slz_rfc1951_encode(struct slz_stream *strm, unsigned char *out, const unsig
                         * and no-comp then huffman consumes 52 bits (7 for EOB + 3 for
                         * block type + 7 for alignment + 32 for LEN+NLEN + 3 for next
                         * block. Only use plain literals if there are more than 52 bits
-                        * to save then.
+                        * to save then, or if we're already too much in debt, in which
+                        * case only stored blocks may be emitted, see <debt> below.
+                        * Literals sent in a stored block do not cost anything extra,
+                        * only those sent in huffman mode add to the debt.
                         */
-                       if (bit9 >= SLZ_SWITCH_COST)
+                       if (bit9 >= SLZ_SWITCH_COST || strm->debt >= SLZ_MAX_DEBT)
                                copy_lit(strm, in + pos - plit, plit, 1);
-                       else
+                       else {
                                copy_lit_huff(strm, in + pos - plit, plit, 1);
+                               strm->debt += bit9;
+                       }
 
                        plit = 0;
                }
@@ -739,6 +758,26 @@ long slz_rfc1951_encode(struct slz_stream *strm, unsigned char *out, const unsig
 
                /* in fixed huffman mode, dist is fixed 5 bits */
                enqueue24(strm, dist >> 5, dist & 0x1f);
+
+               /* <bit9> only measures the current group of literals, which is
+                * what the decisions above compare against, and it restarts
+                * from zero after each reference. <debt> accumulates how much
+                * the literals already emitted in fixed huffman mode are still
+                * costing us compared to the same bytes sent as stored blocks,
+                * and is what bounds the growth of the output: the reference
+                * just used (code>>16)+(dist&0x1f) bits in place of the 8*mlen
+                * bits these bytes would have taken as literals, so it repays
+                * that much of the debt. Without this, a stream made of series
+                * of 9-bit literals interleaved with cheap references would
+                * never reach the threshold above and would keep inflating. The
+                * debt is saturated at zero since we're not interested in
+                * accumulating credits. The update is unconditional because a
+                * test on a debt that is often null but not always turns into
+                * a mispredicted branch on binary data.
+                */
+               code = strm->debt + (code >> 16) + (dist & 0x1f);
+               strm->debt = (code > 8 * (uint32_t)mlen) ? code - 8 * (uint32_t)mlen : 0;
+
                bit9 = 0;
                rem -= mlen;
                pos += mlen;
@@ -778,7 +817,7 @@ long slz_rfc1951_encode(struct slz_stream *strm, unsigned char *out, const unsig
                else
                        cost = (strm->state == SLZ_ST_EOB) ? SLZ_LAST_COST_EOB : SLZ_LAST_COST;
 
-               if (bit9 >= cost)
+               if (bit9 >= cost || strm->debt >= SLZ_MAX_DEBT)
                        copy_lit(strm, in + pos - plit, plit, more);
                else
                        copy_lit_huff(strm, in + pos - plit, plit, more);
@@ -804,6 +843,7 @@ int slz_rfc1951_init(struct slz_stream *strm, int level)
        strm->ilen  = 0;
        strm->qbits = 0;
        strm->queue = 0;
+       strm->debt  = 0;
        return 0;
 }
 
@@ -1177,6 +1217,7 @@ int slz_rfc1952_init(struct slz_stream *strm, int level)
        strm->ilen   = 0;
        strm->qbits  = 0;
        strm->queue  = 0;
+       strm->debt   = 0;
        return 0;
 }
 
@@ -1454,6 +1495,7 @@ int slz_rfc1950_init(struct slz_stream *strm, int level)
        strm->ilen   = 0;
        strm->qbits  = 0;
        strm->queue  = 0;
+       strm->debt   = 0;
        return 0;
 }