Make huffman encoding faster

lib/nghttp2_hd_huffman.c

@@ -43,57 +43,73 @@ extern const nghttp2_huff_decode huff_decode_table[][16];
 static ssize_t huff_encode_sym(nghttp2_bufs *bufs, size_t *avail_ptr,
                                size_t rembits, const nghttp2_huff_sym *sym) {
   int rv;
-  size_t nbits = sym->nbits;
+  uint32_t nbits = sym->nbits;
+  uint32_t code = sym->code;
 
-  for (;;) {
-    if (rembits > nbits) {
-      if (*avail_ptr) {
-        nghttp2_bufs_fast_orb_hold(bufs, sym->code << (rembits - nbits));
-      } else {
-        rv = nghttp2_bufs_orb_hold(bufs, sym->code << (rembits - nbits));
-        if (rv != 0) {
-          return rv;
-        }
-
-        *avail_ptr = nghttp2_bufs_cur_avail(bufs);
-      }
+  /* We assume that sym->nbits <= 32 */
+  if (rembits > nbits) {
+    nghttp2_bufs_fast_orb_hold(bufs, code << (rembits - nbits));
+    return (ssize_t)(rembits - nbits);
+  }
 
-      rembits -= nbits;
+  if (rembits == nbits) {
+    nghttp2_bufs_fast_orb(bufs, code);
+    --*avail_ptr;
+    return 8;
+  }
 
-      break;
-    }
+  nghttp2_bufs_fast_orb(bufs, code >> (nbits - rembits));
+  --*avail_ptr;
 
-    if (*avail_ptr) {
-      nghttp2_bufs_fast_orb(bufs, sym->code >> (nbits - rembits));
-      --*avail_ptr;
-    } else {
-      rv = nghttp2_bufs_orb(bufs, sym->code >> (nbits - rembits));
-      if (rv != 0) {
-        return rv;
-      }
+  nbits -= rembits;
+  if (nbits & 0x7) {
+    /* align code to MSB byte boundary */
+    code <<= 8 - (nbits & 0x7);
+  }
 
-      *avail_ptr = nghttp2_bufs_cur_avail(bufs);
+  /* we lose at most 3 bytes, but it is not critical in practice */
+  if (*avail_ptr < (nbits + 7) / 8) {
+    rv = nghttp2_bufs_advance(bufs);
+    if (rv != 0) {
+      return rv;
     }
+    *avail_ptr = nghttp2_bufs_cur_avail(bufs);
+    /* we assume that we at least 3 buffer space available */
+    assert(*avail_ptr >= 3);
+  }
 
-    nbits -= rembits;
-    rembits = 8;
+  /* fast path, since most code is less than 8 */
+  if (nbits < 8) {
+    nghttp2_bufs_fast_addb_hold(bufs, code);
+    *avail_ptr = nghttp2_bufs_cur_avail(bufs);
+    return (ssize_t)(8 - nbits);
+  }
 
-    if (nbits == 0) {
-      break;
-    }
+  /* handle longer code path */
+  if (nbits > 24) {
+    nghttp2_bufs_fast_addb(bufs, code >> 24);
+    nbits -= 8;
+  }
 
-    if (*avail_ptr) {
-      nghttp2_bufs_fast_addb_hold(bufs, 0);
-    } else {
-      rv = nghttp2_bufs_addb_hold(bufs, 0);
-      if (rv != 0) {
-        return rv;
-      }
+  if (nbits > 16) {
+    nghttp2_bufs_fast_addb(bufs, code >> 16);
+    nbits -= 8;
+  }
 
-      *avail_ptr = nghttp2_bufs_cur_avail(bufs);
-    }
+  if (nbits > 8) {
+    nghttp2_bufs_fast_addb(bufs, code >> 8);
+    nbits -= 8;
   }
-  return (ssize_t)rembits;
+
+  if (nbits == 8) {
+    nghttp2_bufs_fast_addb(bufs, code);
+    *avail_ptr = nghttp2_bufs_cur_avail(bufs);
+    return 8;
+  }
+
+  nghttp2_bufs_fast_addb_hold(bufs, code);
+  *avail_ptr = nghttp2_bufs_cur_avail(bufs);
+  return (ssize_t)(8 - nbits);
 }
 
 size_t nghttp2_hd_huff_encode_count(const uint8_t *src, size_t len) {
@@ -136,17 +152,12 @@ int nghttp2_hd_huff_encode(nghttp2_bufs *bufs, const uint8_t *src,
   }
   /* 256 is special terminal symbol, pad with its prefix */
   if (rembits < 8) {
+    /* if rembits < 8, we should have at least 1 buffer space
+       available */
     const nghttp2_huff_sym *sym = &huff_sym_table[256];
-
+    assert(avail);
     /* Caution we no longer adjust avail here */
-    if (avail) {
-      nghttp2_bufs_fast_orb(bufs, sym->code >> (sym->nbits - rembits));
-    } else {
-      rv = nghttp2_bufs_orb(bufs, sym->code >> (sym->nbits - rembits));
-      if (rv != 0) {
-        return rv;
-      }
-    }
+    nghttp2_bufs_fast_orb(bufs, sym->code >> (sym->nbits - rembits));
   }
 
   return 0;

tests/main.c

@@ -303,6 +303,7 @@ int main(int argc _U_, char *argv[] _U_) {
       !CU_add_test(pSuite, "hd_deflate_bound", test_nghttp2_hd_deflate_bound) ||
       !CU_add_test(pSuite, "hd_public_api", test_nghttp2_hd_public_api) ||
       !CU_add_test(pSuite, "hd_decode_length", test_nghttp2_hd_decode_length) ||
+      !CU_add_test(pSuite, "hd_huff_encode", test_nghttp2_hd_huff_encode) ||
       !CU_add_test(pSuite, "adjust_local_window_size",
                    test_nghttp2_adjust_local_window_size) ||
       !CU_add_test(pSuite, "check_header_name",

tests/nghttp2_hd_test.c

@@ -1211,3 +1211,32 @@ void test_nghttp2_hd_decode_length(void) {
 
   CU_ASSERT(-1 == rv);
 }
+
+void test_nghttp2_hd_huff_encode(void) {
+  int rv;
+  ssize_t len;
+  nghttp2_bufs bufs, outbufs;
+  nghttp2_hd_huff_decode_context ctx;
+  const uint8_t t1[] = {22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11,
+                        10, 9,  8,  7,  6,  5,  4,  3,  2,  1,  0};
+
+  frame_pack_bufs_init(&bufs);
+  frame_pack_bufs_init(&outbufs);
+
+  rv = nghttp2_hd_huff_encode(&bufs, t1, sizeof(t1));
+
+  CU_ASSERT(rv == 0);
+
+  nghttp2_hd_huff_decode_context_init(&ctx);
+
+  len = nghttp2_hd_huff_decode(&ctx, &outbufs, bufs.cur->buf.pos,
+                               nghttp2_bufs_len(&bufs), 1);
+
+  CU_ASSERT(nghttp2_bufs_len(&bufs) == len);
+  CU_ASSERT((ssize_t)sizeof(t1) == nghttp2_bufs_len(&outbufs));
+
+  CU_ASSERT(0 == memcmp(t1, outbufs.cur->buf.pos, sizeof(t1)));
+
+  nghttp2_bufs_free(&bufs);
+  nghttp2_bufs_free(&outbufs);
+}

tests/nghttp2_hd_test.h

@@ -42,5 +42,6 @@ void test_nghttp2_hd_no_index(void);
 void test_nghttp2_hd_deflate_bound(void);
 void test_nghttp2_hd_public_api(void);
 void test_nghttp2_hd_decode_length(void);
+void test_nghttp2_hd_huff_encode(void);
 
 #endif /* NGHTTP2_HD_TEST_H */