Author: rhuijben
Date: Wed Nov  4 16:04:26 2015
New Revision: 1712574

URL: http://svn.apache.org/viewvc?rev=1712574&view=rev
Log:
Following up on r1712559, make the hpack encoder use standard buffer chunks
to encode data instead of many small buckets of just a few bytes.

This allows removing the handling of insane amounts of iovecs in the http2
frame bucket that used to be necessary when we wanted to read this data at
once. It will also make it easier to encode things more efficiently later.

* buckets/hpack_buckets.c
  (hpack_int): Update argument to document that the result won't be larger
    than 6 bytes. (1 prefix byte + 5 bytes containing 7 bits each)
  (serialize): Copy data to a few larger memory chunks instead of producing
    many small chunks.
  (serf_hpack_get_remaining): New function, triggering a serialize call like
    the other read functions.

* buckets/http2_frame_buckets.c
  (http2_prepare_frame): Use IOV_MAX (default: 50) instead of 512. That should
    be far more than enough to read most created frames now.

Modified:
    serf/trunk/buckets/hpack_buckets.c
    serf/trunk/buckets/http2_frame_buckets.c

Modified: serf/trunk/buckets/hpack_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/hpack_buckets.c?rev=1712574&r1=1712573&r2=1712574&view=diff
==============================================================================
--- serf/trunk/buckets/hpack_buckets.c (original)
+++ serf/trunk/buckets/hpack_buckets.c Wed Nov  4 16:04:26 2015
@@ -742,7 +742,7 @@ void serf__bucket_hpack_do(serf_bucket_t
     }
 }
 
-static void hpack_int(unsigned char flags, int bits, apr_uint32_t value, char 
to[10], apr_size_t *used)
+static void hpack_int(unsigned char flags, int bits, apr_uint32_t value, char 
to[6], apr_size_t *used)
 {
   unsigned char max_direct;
   apr_size_t u;
@@ -784,60 +784,87 @@ serialize(serf_bucket_t *bucket)
    */
   serf_hpack_context_t *ctx = bucket->data;
   serf_bucket_alloc_t *alloc = bucket->allocator;
+  serf_hpack_table_t *tbl = ctx->tbl;
 
   serf_hpack_entry_t *entry;
   serf_hpack_entry_t *next;
 
+  char *buffer = NULL;
+  apr_size_t offset = 0;
+  const apr_size_t chunksize = 2048; /* Wild guess */
+
   /* Put on our aggregate bucket cloak */
   serf_bucket_aggregate_become(bucket);
 
+  /* Is there a tablesize update queued? */
+  if (tbl && tbl->send_tablesize_update)
+    {
+      apr_size_t len;
+
+      buffer = serf_bucket_mem_alloc(alloc, chunksize);
+
+      hpack_int(0x20, 5, tbl->lr_max_table_size, buffer+offset, &len);
+      offset += len;
+      tbl->send_tablesize_update = FALSE;
+    }
+
   for (entry = ctx->first; entry; entry = next)
     {
-      char intbuf[10];
-      apr_size_t intbuf_len;
-      serf_bucket_t *v;
+      apr_size_t len;
 
       next = entry->next;
 
-      /* Literal header, no indexing (=has a name) */
-      hpack_int(0x40, 6, 0, intbuf, &intbuf_len);
+      /* Make 100% sure the next entry will fit.
+         ### Temporarily wastes ram
+       */
+      if (!buffer || (HPACK_ENTRY_SIZE(entry) > chunksize - offset))
+        {
+          if (offset)
+            {
+              serf_bucket_aggregate_append(
+                    bucket,
+                    serf_bucket_simple_own_create(buffer, offset, alloc));
+            }
+
+          buffer = serf_bucket_mem_alloc(alloc, MAX(chunksize,
+                                                    HPACK_ENTRY_SIZE(entry)));
+          offset = 0;
+        }
 
-      serf_bucket_aggregate_append(bucket,
-              serf_bucket_simple_copy_create(intbuf, intbuf_len, alloc));
+      /* Literal header, no indexing (=has a name) */
+      hpack_int(0x40, 6, 0, buffer+offset, &len);
+      offset += len;
 
       /* Name is literal, no huffman encoding */
-      hpack_int(0, 7, entry->key_len, intbuf, &intbuf_len);
-      serf_bucket_aggregate_append(bucket,
-              serf_bucket_simple_copy_create(intbuf, intbuf_len, alloc));
-
-      if (entry->free_key)
-        v = serf_bucket_simple_own_create(entry->key, entry->key_len,
-                                          alloc);
-      else
-        v = serf_bucket_simple_create(entry->key, entry->key_len, NULL, NULL,
-                                      alloc);
+      hpack_int(0, 7, entry->key_len, buffer+offset, &len);
+      offset += len;
 
-      serf_bucket_aggregate_append(bucket, v);
+      memcpy(buffer + offset, entry->key, entry->key_len);
+      offset += entry->key_len;
 
       /* Value is literal, no huffman encoding */
-      hpack_int(0, 7, entry->value_len, intbuf, &intbuf_len);
-      serf_bucket_aggregate_append(bucket,
-              serf_bucket_simple_copy_create(intbuf, intbuf_len, alloc));
-
-      if (entry->free_key)
-        v = serf_bucket_simple_own_create(entry->value, entry->value_len,
-                                          alloc);
-      else
-        v = serf_bucket_simple_create(entry->value, entry->value_len, NULL,
-                                      NULL, alloc);
+      hpack_int(0, 7, entry->value_len, buffer+offset, &len);
+      offset += len;
 
-      serf_bucket_aggregate_append(bucket, v);
+      memcpy(buffer + offset, entry->value, entry->value_len);
+      offset += entry->value_len;
 
-      /* We handed ownership of key and value, so we only have to free item */
-      serf_bucket_mem_free(alloc, entry);
+      hpack_free_entry(entry, bucket->allocator);
     }
   ctx->first = ctx->last = NULL;
 
+  if (buffer)
+    {
+      if (offset)
+        {
+          serf_bucket_aggregate_append(
+            bucket,
+            serf_bucket_simple_own_create(buffer, offset, alloc));
+        }
+      else
+        serf_bucket_mem_free(alloc, buffer);
+    }
+
   serf_bucket_mem_free(alloc, ctx);
 
   return APR_SUCCESS;
@@ -886,6 +913,20 @@ serf_hpack_peek(serf_bucket_t *bucket,
   return bucket->type->peek(bucket, data, len);
 }
 
+
+static apr_uint64_t
+serf_hpack_get_remaining(serf_bucket_t *bucket)
+{
+  apr_status_t status = serialize(bucket);
+
+  if (status)
+    return SERF_LENGTH_UNKNOWN;
+
+  /* This assumes that the aggregate is a v2 bucket */
+  return bucket->type->get_remaining(bucket);
+}
+
+
 static void
 serf_hpack_destroy_and_data(serf_bucket_t *bucket)
 {
@@ -913,9 +954,12 @@ const serf_bucket_type_t serf_bucket_typ
   serf_hpack_readline,
   serf_hpack_read_iovec,
   serf_default_read_for_sendfile,
-  serf_default_read_bucket,
+  serf_buckets_are_v2,
   serf_hpack_peek,
   serf_hpack_destroy_and_data,
+  serf_default_read_bucket,
+  serf_hpack_get_remaining,
+  serf_default_ignore_config,
 };
 
 /* ==================================================================== */

Modified: serf/trunk/buckets/http2_frame_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/http2_frame_buckets.c?rev=1712574&r1=1712573&r2=1712574&view=diff
==============================================================================
--- serf/trunk/buckets/http2_frame_buckets.c (original)
+++ serf/trunk/buckets/http2_frame_buckets.c Wed Nov  4 16:04:26 2015
@@ -710,11 +710,11 @@ http2_prepare_frame(serf_bucket_t *bucke
     {
       /* Our payload doesn't know how long it is. Our only option
          now is to create the actual data */
-      struct iovec vecs[512];
+      struct iovec vecs[IOV_MAX];
       apr_status_t status;
 
       status = serf_bucket_read_iovec(ctx->stream, ctx->max_payload_size,
-                                      512, vecs, &vecs_used);
+                                      IOV_MAX, vecs, &vecs_used);
 
       if (SERF_BUCKET_READ_ERROR(status))
         return status;
@@ -756,7 +756,7 @@ http2_prepare_frame(serf_bucket_t *bucke
               apr_size_t read;
               status = serf_bucket_read_iovec(ctx->stream,
                                               ctx->max_payload_size - total + 
1,
-                                              512, vecs, &vecs_used);
+                                              IOV_MAX, vecs, &vecs_used);
 
               if (SERF_BUCKET_READ_ERROR(status))
                 {


Reply via email to