walkero
/
nghttp2
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

nghttp2_hd: Use single buffer for an name/value pair 

Previously we use 2 separate buffer for each name and value.  The
problem is we would waste buffer space for name because it is usually
small.  Also tuning buffer size for each buffer separately is not
elegant and current HTTP server practice is that one buffer for 1
name/value pair.  This commit unifies 2 buffers into 1.
This commit is contained in:
Tatsuhiro Tsujikawa 2014-05-28 23:02:23 +09:00
parent 53e75ff0d0
commit d113055899
4 changed files with 54 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
lib/nghttp2_hd.c
View File

@ -352,37 +352,29 @@ int nghttp2_hd_inflate_init(nghttp2_hd_inflater *inflater)
NGHTTP2_HD_DEFAULT_MAX_BUFFER_SIZE; NGHTTP2_HD_DEFAULT_MAX_BUFFER_SIZE;
inflater->ent_keep = NULL; inflater->ent_keep = NULL;
inflater->name_keep = NULL; inflater->nv_keep = NULL;
inflater->value_keep = NULL;
inflater->end_headers_index = 0; inflater->end_headers_index = 0;
inflater->opcode = NGHTTP2_HD_OPCODE_NONE; inflater->opcode = NGHTTP2_HD_OPCODE_NONE;
inflater->state = NGHTTP2_HD_STATE_OPCODE; inflater->state = NGHTTP2_HD_STATE_OPCODE;
rv = nghttp2_bufs_init(&inflater->namebufs, NGHTTP2_HD_MAX_NAME, 1); rv = nghttp2_bufs_init(&inflater->nvbufs, NGHTTP2_HD_MAX_NV / 2, 2);
if(rv != 0) { if(rv != 0) {
goto namebuf_fail; goto nvbufs_fail;
}
rv = nghttp2_bufs_init(&inflater->valuebufs, NGHTTP2_HD_MAX_VALUE / 2, 2);
if(rv != 0) {
goto valuebuf_fail;
} }
inflater->huffman_encoded = 0; inflater->huffman_encoded = 0;
inflater->index = 0; inflater->index = 0;
inflater->left = 0; inflater->left = 0;
inflater->newnamelen = 0;
inflater->index_required = 0; inflater->index_required = 0;
inflater->no_index = 0; inflater->no_index = 0;
inflater->ent_name = NULL; inflater->ent_name = NULL;
return 0; return 0;
valuebuf_fail: nvbufs_fail:
nghttp2_bufs_free(&inflater->namebufs);
namebuf_fail:
hd_context_free(&inflater->ctx); hd_context_free(&inflater->ctx);
fail: fail:
return rv; return rv;
@ -397,10 +389,9 @@ static void hd_inflate_keep_free(nghttp2_hd_inflater *inflater)
} }
inflater->ent_keep = NULL; inflater->ent_keep = NULL;
} }
free(inflater->name_keep);
free(inflater->value_keep); free(inflater->nv_keep);
inflater->name_keep = NULL; inflater->nv_keep = NULL;
inflater->value_keep = NULL;
} }
void nghttp2_hd_deflate_free(nghttp2_hd_deflater *deflater) void nghttp2_hd_deflate_free(nghttp2_hd_deflater *deflater)
@ -411,8 +402,7 @@ void nghttp2_hd_deflate_free(nghttp2_hd_deflater *deflater)
void nghttp2_hd_inflate_free(nghttp2_hd_inflater *inflater) void nghttp2_hd_inflate_free(nghttp2_hd_inflater *inflater)
{ {
hd_inflate_keep_free(inflater); hd_inflate_keep_free(inflater);
nghttp2_bufs_free(&inflater->namebufs); nghttp2_bufs_free(&inflater->nvbufs);
nghttp2_bufs_free(&inflater->valuebufs);
hd_context_free(&inflater->ctx); hd_context_free(&inflater->ctx);
} }
@ -1488,27 +1478,27 @@ static int hd_inflate_remove_bufs(nghttp2_hd_inflater *inflater,
nghttp2_nv *nv, int value_only) nghttp2_nv *nv, int value_only)
{ {
ssize_t rv; ssize_t rv;
size_t buflen;
uint8_t *buf;
if(value_only) { rv = nghttp2_bufs_remove(&inflater->nvbufs, &buf);
nv->name = NULL;
} else {
rv = nghttp2_bufs_remove(&inflater->namebufs, &nv->name);
if(rv < 0) {
return NGHTTP2_ERR_NOMEM;
}
nv->namelen = rv;
}
rv = nghttp2_bufs_remove(&inflater->valuebufs, &nv->value);
if(rv < 0) { if(rv < 0) {
free(nv->name);
return NGHTTP2_ERR_NOMEM; return NGHTTP2_ERR_NOMEM;
} }
nv->valuelen = rv; buflen = rv;
if(value_only) {
nv->name = NULL;
nv->namelen = 0;
} else {
nv->name = buf;
nv->namelen = inflater->newnamelen;
}
nv->value = buf + nv->namelen;
nv->valuelen = buflen - nv->namelen;
return 0; return 0;
} }
@ -1545,9 +1535,10 @@ static int hd_inflate_commit_newname(nghttp2_hd_inflater *inflater,
nghttp2_hd_entry *new_ent; nghttp2_hd_entry *new_ent;
uint8_t ent_flags; uint8_t ent_flags;
ent_flags = /* nv->value points to the middle of the buffer pointed by
NGHTTP2_HD_FLAG_NAME_ALLOC | NGHTTP2_HD_FLAG_VALUE_ALLOC | nv->name. So we just need to keep track of nv->name for memory
NGHTTP2_HD_FLAG_NAME_GIFT | NGHTTP2_HD_FLAG_VALUE_GIFT; management. */
ent_flags = NGHTTP2_HD_FLAG_NAME_ALLOC | NGHTTP2_HD_FLAG_NAME_GIFT;
new_ent = add_hd_table_incremental(&inflater->ctx, NULL, &nv, ent_flags); new_ent = add_hd_table_incremental(&inflater->ctx, NULL, &nv, ent_flags);
@ -1559,15 +1550,13 @@ static int hd_inflate_commit_newname(nghttp2_hd_inflater *inflater,
} }
free(nv.name); free(nv.name);
free(nv.value);
return NGHTTP2_ERR_NOMEM; return NGHTTP2_ERR_NOMEM;
} }
emit_literal_header(nv_out, &nv); emit_literal_header(nv_out, &nv);
inflater->name_keep = nv.name; inflater->nv_keep = nv.name;
inflater->value_keep = nv.value;
return 0; return 0;
} }
@ -1642,7 +1631,7 @@ static int hd_inflate_commit_indname(nghttp2_hd_inflater *inflater,
emit_literal_header(nv_out, &nv); emit_literal_header(nv_out, &nv);
inflater->value_keep = nv.value; inflater->nv_keep = nv.value;
return 0; return 0;
} }
@ -1793,7 +1782,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater,
case NGHTTP2_HD_STATE_NEWNAME_READ_NAMELEN: case NGHTTP2_HD_STATE_NEWNAME_READ_NAMELEN:
rfin = 0; rfin = 0;
rv = hd_inflate_read_len(inflater, &rfin, in, last, 7, rv = hd_inflate_read_len(inflater, &rfin, in, last, 7,
NGHTTP2_HD_MAX_NAME); NGHTTP2_HD_MAX_NV);
if(rv < 0) { if(rv < 0) {
goto fail; goto fail;
} }
@ -1815,7 +1804,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater,
} }
break; break;
case NGHTTP2_HD_STATE_NEWNAME_READ_NAMEHUFF: case NGHTTP2_HD_STATE_NEWNAME_READ_NAMEHUFF:
rv = hd_inflate_read_huff(inflater, &inflater->namebufs, in, last); rv = hd_inflate_read_huff(inflater, &inflater->nvbufs, in, last);
if(rv < 0) { if(rv < 0) {
goto fail; goto fail;
} }
@ -1831,11 +1820,13 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater,
goto almost_ok; goto almost_ok;
} }
inflater->newnamelen = nghttp2_bufs_len(&inflater->nvbufs);
inflater->state = NGHTTP2_HD_STATE_CHECK_VALUELEN; inflater->state = NGHTTP2_HD_STATE_CHECK_VALUELEN;
break; break;
case NGHTTP2_HD_STATE_NEWNAME_READ_NAME: case NGHTTP2_HD_STATE_NEWNAME_READ_NAME:
rv = hd_inflate_read(inflater, &inflater->namebufs, in, last); rv = hd_inflate_read(inflater, &inflater->nvbufs, in, last);
if(rv < 0) { if(rv < 0) {
goto fail; goto fail;
} }
@ -1850,6 +1841,8 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater,
goto almost_ok; goto almost_ok;
} }
inflater->newnamelen = nghttp2_bufs_len(&inflater->nvbufs);
inflater->state = NGHTTP2_HD_STATE_CHECK_VALUELEN; inflater->state = NGHTTP2_HD_STATE_CHECK_VALUELEN;
break; break;
@ -1863,7 +1856,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater,
case NGHTTP2_HD_STATE_READ_VALUELEN: case NGHTTP2_HD_STATE_READ_VALUELEN:
rfin = 0; rfin = 0;
rv = hd_inflate_read_len(inflater, &rfin, in, last, 7, rv = hd_inflate_read_len(inflater, &rfin, in, last, 7,
NGHTTP2_HD_MAX_VALUE); NGHTTP2_HD_MAX_NV);
if(rv < 0) { if(rv < 0) {
goto fail; goto fail;
} }
@ -1898,7 +1891,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater,
} }
break; break;
case NGHTTP2_HD_STATE_READ_VALUEHUFF: case NGHTTP2_HD_STATE_READ_VALUEHUFF:
rv = hd_inflate_read_huff(inflater, &inflater->valuebufs, in, last); rv = hd_inflate_read_huff(inflater, &inflater->nvbufs, in, last);
if(rv < 0) { if(rv < 0) {
goto fail; goto fail;
} }
@ -1929,7 +1922,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater,
return in - first; return in - first;
case NGHTTP2_HD_STATE_READ_VALUE: case NGHTTP2_HD_STATE_READ_VALUE:
rv = hd_inflate_read(inflater, &inflater->valuebufs, in, last); rv = hd_inflate_read(inflater, &inflater->nvbufs, in, last);
if(rv < 0) { if(rv < 0) {
DEBUGF(fprintf(stderr, "inflatehd: value read failure %zd: %s\n", DEBUGF(fprintf(stderr, "inflatehd: value read failure %zd: %s\n",
rv, nghttp2_strerror(rv))); rv, nghttp2_strerror(rv)));

24
lib/nghttp2_hd.h
View File

@ -37,10 +37,10 @@
#define NGHTTP2_HD_DEFAULT_MAX_BUFFER_SIZE NGHTTP2_DEFAULT_HEADER_TABLE_SIZE #define NGHTTP2_HD_DEFAULT_MAX_BUFFER_SIZE NGHTTP2_DEFAULT_HEADER_TABLE_SIZE
#define NGHTTP2_HD_ENTRY_OVERHEAD 32 #define NGHTTP2_HD_ENTRY_OVERHEAD 32
/* The maximum value length of name/value pair. This is not specified /* The maximum length of one name/value pair. This is the sum of the
by the spec. We just chose the arbitrary size */ length of name and value. This is not specified by the spec. We
#define NGHTTP2_HD_MAX_NAME 256 just chose the arbitrary size */
#define NGHTTP2_HD_MAX_VALUE 8192 #define NGHTTP2_HD_MAX_NV 8192
/* Default size of maximum table buffer size for encoder. Even if /* Default size of maximum table buffer size for encoder. Even if
remote decoder notifies larger buffer size for its decoding, remote decoder notifies larger buffer size for its decoding,
@ -146,20 +146,17 @@ struct nghttp2_hd_deflater {
struct nghttp2_hd_inflater { struct nghttp2_hd_inflater {
nghttp2_hd_context ctx; nghttp2_hd_context ctx;
/* header name buffer */ /* header buffer */
nghttp2_bufs namebufs; nghttp2_bufs nvbufs;
/* header value buffer */
nghttp2_bufs valuebufs;
/* Stores current state of huffman decoding */ /* Stores current state of huffman decoding */
nghttp2_hd_huff_decode_context huff_decode_ctx; nghttp2_hd_huff_decode_context huff_decode_ctx;
/* Pointer to the nghttp2_hd_entry which is used current header /* Pointer to the nghttp2_hd_entry which is used current header
emission. This is required because in some cases the emission. This is required because in some cases the
ent_keep->ref == 0 and we have to keep track of it. */ ent_keep->ref == 0 and we have to keep track of it. */
nghttp2_hd_entry *ent_keep; nghttp2_hd_entry *ent_keep;
/* Pointers to the name/value pair which are used current header /* Pointer to the name/value pair buffer which is used in the
emission. They are usually used to keep track of malloc'ed memory current header emission. */
for huffman decoding. */ uint8_t *nv_keep;
uint8_t *name_keep, *value_keep;
/* Pointers to the name/value pair which is referred as indexed /* Pointers to the name/value pair which is referred as indexed
name. This entry must be in header table. */ name. This entry must be in header table. */
nghttp2_hd_entry *ent_name; nghttp2_hd_entry *ent_name;
@ -170,6 +167,9 @@ struct nghttp2_hd_inflater {
/* The index of header table to toggle off the entry from reference /* The index of header table to toggle off the entry from reference
set at the end of decompression. */ set at the end of decompression. */
size_t end_headers_index; size_t end_headers_index;
/* The length of new name encoded in literal. For huffman encoded
string, this is the length after it is decoded. */
size_t newnamelen;
/* The maximum header table size the inflater supports. This is the /* The maximum header table size the inflater supports. This is the
same value transmitted in SETTINGS_HEADER_TABLE_SIZE */ same value transmitted in SETTINGS_HEADER_TABLE_SIZE */
size_t settings_hd_table_bufsize_max; size_t settings_hd_table_bufsize_max;

2
tests/nghttp2_frame_test.c
View File

@ -173,7 +173,7 @@ void test_nghttp2_frame_pack_headers_frame_too_large(void)
nghttp2_bufs bufs; nghttp2_bufs bufs;
nghttp2_nv *nva; nghttp2_nv *nva;
ssize_t nvlen; ssize_t nvlen;
size_t big_vallen = NGHTTP2_HD_MAX_VALUE; size_t big_vallen = NGHTTP2_HD_MAX_NV;
nghttp2_nv big_hds[16]; nghttp2_nv big_hds[16];
size_t big_hdslen = ARRLEN(big_hds); size_t big_hdslen = ARRLEN(big_hds);
size_t i; size_t i;

2
tests/nghttp2_session_test.c
View File

@ -2374,7 +2374,7 @@ void test_nghttp2_session_send_headers_header_comp_error(void)
nghttp2_frame *frame = malloc(sizeof(nghttp2_frame)); nghttp2_frame *frame = malloc(sizeof(nghttp2_frame));
nghttp2_nv *nva; nghttp2_nv *nva;
ssize_t nvlen; ssize_t nvlen;
size_t vallen = NGHTTP2_HD_MAX_VALUE; size_t vallen = NGHTTP2_HD_MAX_NV;
nghttp2_nv nv[28]; nghttp2_nv nv[28];
size_t nnv = ARRLEN(nv); size_t nnv = ARRLEN(nv);
size_t i; size_t i;