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)) {