diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 4701f5b3..0875c426 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -686,7 +686,7 @@ int nghttp2_session_add_frame(nghttp2_session *session, stream = nghttp2_session_get_stream(session, data_frame->hd.stream_id); if(stream) { - item->weight = stream->stream_group->weight; + item->weight = nghttp2_stream_group_shared_wait(stream->stream_group); rv = nghttp2_stream_attach_data(stream, item, &session->ob_pq); } @@ -2087,7 +2087,8 @@ static int nghttp2_session_after_frame_sent(nghttp2_session *session) assert(stream); next_item = nghttp2_session_get_next_ob_item(session); - outbound_item_cycle_weight(aob->item, stream->stream_group->weight); + outbound_item_cycle_weight + (aob->item, nghttp2_stream_group_shared_wait(stream->stream_group)); /* If priority of this stream is higher or equal to other stream waiting at the top of the queue, we continue to send this diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 0cec7682..5c1832a6 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -27,6 +27,8 @@ #include #include +#include "nghttp2_helper.h" + void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, uint8_t flags, nghttp2_stream_state initial_state, @@ -58,6 +60,7 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, stream->stream_group = NULL; stream->dpri = NGHTTP2_STREAM_DPRI_NO_DATA; stream->num_substreams = 1; + stream->num_subtop = 0; } void nghttp2_stream_free(nghttp2_stream *stream) @@ -101,6 +104,23 @@ static nghttp2_stream* stream_update_dep_length(nghttp2_stream *stream, return stream; } +static nghttp2_stream* stream_update_dep_both_length(nghttp2_stream *stream, + ssize_t delta_stream, + ssize_t delta_top) +{ + stream->num_substreams += delta_stream; + stream->num_subtop += delta_top; + + stream = stream_first_sib(stream); + + if(stream->dep_prev) { + return stream_update_dep_both_length(stream->dep_prev, delta_stream, + delta_top); + } + + return stream; +} + static void stream_update_dep_set_rest_stream_group (nghttp2_stream *stream, nghttp2_stream_group *stream_group) { @@ -129,6 +149,8 @@ static void stream_update_dep_set_rest(nghttp2_stream *stream) return; } + stream->num_subtop = 0; + if(stream->dpri == NGHTTP2_STREAM_DPRI_TOP) { stream->dpri = NGHTTP2_STREAM_DPRI_REST; @@ -145,22 +167,33 @@ static void stream_update_dep_set_rest(nghttp2_stream *stream) * Performs dfs starting |stream|, search stream which can become * NGHTTP2_STREAM_DPRI_TOP and queues its data. * - * This function returns 0 if it succeeds, or one of the following - * negative error codes: + * This function returns the number of stream marked as + * NGHTTP2_STREAM_DPRI_TOP (including already marked as such) if it + * succeeds, or one of the following negative error codes: * * NGHTTP2_ERR_NOMEM * Out of memory. */ -static int stream_update_dep_set_top(nghttp2_stream *stream, nghttp2_pq *pq) +static ssize_t stream_update_dep_set_top(nghttp2_stream *stream, + nghttp2_pq *pq) { - int rv; + ssize_t rv; + ssize_t num_top; if(stream == NULL) { return 0; } if(stream->dpri == NGHTTP2_STREAM_DPRI_TOP) { - return stream_update_dep_set_top(stream->sib_next, pq); + rv = stream_update_dep_set_top(stream->sib_next, pq); + + if(rv < 0) { + return rv; + } + + stream->num_subtop = 1; + + return stream->num_subtop + rv; } if(stream->dpri == NGHTTP2_STREAM_DPRI_REST) { @@ -180,25 +213,44 @@ static int stream_update_dep_set_top(nghttp2_stream *stream, nghttp2_pq *pq) stream->dpri = NGHTTP2_STREAM_DPRI_TOP; - return stream_update_dep_set_top(stream->sib_next, pq); + rv = stream_update_dep_set_top(stream->sib_next, pq); + + if(rv < 0) { + return rv; + } + + stream->num_subtop = 1; + + return stream->num_subtop + rv; } assert(stream->dpri == NGHTTP2_STREAM_DPRI_NO_DATA); rv = stream_update_dep_set_top(stream->sib_next, pq); - if(rv != 0) { + if(rv < 0) { return rv; } - return stream_update_dep_set_top(stream->dep_next, pq); + num_top = rv; + + rv = stream_update_dep_set_top(stream->dep_next, pq); + + if(rv < 0) { + return rv; + } + + stream->num_subtop = rv; + + return stream->num_subtop + num_top; } -static int stream_update_dep_on_attach_data(nghttp2_stream *stream, - nghttp2_pq *pq) +static ssize_t stream_update_dep_on_attach_data(nghttp2_stream *stream, + nghttp2_pq *pq) { - int rv; + ssize_t rv; nghttp2_stream *root_stream; + ssize_t old_num_subtop; stream->dpri = NGHTTP2_STREAM_DPRI_REST; @@ -208,18 +260,27 @@ static int stream_update_dep_on_attach_data(nghttp2_stream *stream, DEBUGF(fprintf(stderr, "root=%p, stream=%p\n", root_stream, stream)); + old_num_subtop = root_stream->num_subtop; + rv = stream_update_dep_set_top(root_stream, pq); - if(rv != 0) { + if(rv < 0) { return rv; } + nghttp2_stream_group_update_num_top + (root_stream->stream_group, root_stream->num_subtop - old_num_subtop); + return 0; } static int stream_update_dep_on_detach_data(nghttp2_stream *stream, nghttp2_pq *pq) { + ssize_t rv; + nghttp2_stream *root_stream; + ssize_t old_num_subtop; + if(stream->dpri != NGHTTP2_STREAM_DPRI_TOP) { stream->dpri = NGHTTP2_STREAM_DPRI_NO_DATA; @@ -228,7 +289,20 @@ static int stream_update_dep_on_detach_data(nghttp2_stream *stream, stream->dpri = NGHTTP2_STREAM_DPRI_NO_DATA; - return stream_update_dep_set_top(stream->dep_next, pq); + root_stream = nghttp2_stream_get_dep_root(stream); + + old_num_subtop = root_stream->num_subtop; + + rv = stream_update_dep_set_top(root_stream, pq); + + if(rv < 0) { + return rv; + } + + nghttp2_stream_group_update_num_top + (root_stream->stream_group, root_stream->num_subtop - old_num_subtop); + + return 0; } int nghttp2_stream_attach_data(nghttp2_stream *stream, @@ -509,6 +583,8 @@ int nghttp2_stream_dep_insert_subtree(nghttp2_stream *dep_stream, nghttp2_stream *root_stream; nghttp2_stream *si; size_t delta_substreams; + ssize_t old_num_subtop; + ssize_t rv; DEBUGF(fprintf(stderr, "stream: dep_insert_subtree dep_stream(%p)=%d " "stream(%p)=%d\n", @@ -517,6 +593,9 @@ int nghttp2_stream_dep_insert_subtree(nghttp2_stream *dep_stream, delta_substreams = stream->num_substreams; + nghttp2_stream_group_update_num_top + (stream->stream_group, -stream->num_subtop); + stream_update_dep_set_rest_stream_group(stream, dep_stream->stream_group); if(dep_stream->dep_next) { @@ -549,7 +628,18 @@ int nghttp2_stream_dep_insert_subtree(nghttp2_stream *dep_stream, root_stream = stream_update_dep_length(dep_stream, delta_substreams); - return stream_update_dep_set_top(root_stream, pq); + old_num_subtop = root_stream->num_subtop; + + rv = stream_update_dep_set_top(root_stream, pq); + + if(rv < 0) { + return rv; + } + + nghttp2_stream_group_update_num_top + (root_stream->stream_group, root_stream->num_subtop - old_num_subtop); + + return 0; } int nghttp2_stream_dep_add_subtree(nghttp2_stream *dep_stream, @@ -558,12 +648,17 @@ int nghttp2_stream_dep_add_subtree(nghttp2_stream *dep_stream, { nghttp2_stream *last_sib; nghttp2_stream *root_stream; + ssize_t old_num_subtop; + ssize_t rv; DEBUGF(fprintf(stderr, "stream: dep_add_subtree dep_stream(%p)=%d " "stream(%p)=%d\n", dep_stream, dep_stream->stream_id, stream, stream->stream_id)); + nghttp2_stream_group_update_num_top + (stream->stream_group, -stream->num_subtop); + stream_update_dep_set_rest_stream_group(stream, dep_stream->stream_group); if(dep_stream->dep_next) { @@ -578,13 +673,29 @@ int nghttp2_stream_dep_add_subtree(nghttp2_stream *dep_stream, root_stream = stream_update_dep_length(dep_stream, stream->num_substreams); - return stream_update_dep_set_top(root_stream, pq); + old_num_subtop = root_stream->num_subtop; + + rv = stream_update_dep_set_top(root_stream, pq); + + if(rv < 0) { + return rv; + } + + nghttp2_stream_group_update_num_top + (root_stream->stream_group, root_stream->num_subtop - old_num_subtop); + + return 0; } void nghttp2_stream_dep_remove_subtree(nghttp2_stream *stream) { nghttp2_stream *prev, *next; + DEBUGF(fprintf(stderr, "stream: dep_remove_subtree stream(%p)=%d\n", + stream, stream->stream_id)); + + /* Removing subtree does not change stream_group->num_top */ + if(stream->sib_prev) { prev = stream->sib_prev; @@ -595,7 +706,8 @@ void nghttp2_stream_dep_remove_subtree(nghttp2_stream *stream) prev = stream_first_sib(prev); if(prev->dep_prev) { - stream_update_dep_length(prev->dep_prev, -stream->num_substreams); + stream_update_dep_both_length(prev->dep_prev, -stream->num_substreams, + -stream->num_subtop); } } else if(stream->dep_prev) { prev = stream->dep_prev; @@ -609,7 +721,8 @@ void nghttp2_stream_dep_remove_subtree(nghttp2_stream *stream) next->sib_prev = NULL; } - stream_update_dep_length(prev, -stream->num_substreams); + stream_update_dep_both_length(prev, -stream->num_substreams, + -stream->num_subtop); } stream->sib_prev = NULL; @@ -621,9 +734,29 @@ int nghttp2_stream_dep_make_root(nghttp2_stream_group *stream_group, nghttp2_stream *stream, nghttp2_pq *pq) { + ssize_t rv; + + DEBUGF(fprintf(stderr, "stream: dep_make_root new_stream_group(%p)=%d, " + "old_stream_group(%p)=%d, stream(%p)=%d\n", + stream_group, stream_group->pri_group_id, + stream->stream_group, stream->stream_group->pri_group_id, + stream, stream->stream_id)); + + /* First update num_top of old stream_group */ + nghttp2_stream_group_update_num_top + (stream->stream_group, -stream->num_subtop); + stream_update_dep_set_rest_stream_group(stream, stream_group); - return stream_update_dep_set_top(stream, pq); + rv = stream_update_dep_set_top(stream, pq); + + if(rv < 0) { + return rv; + } + + nghttp2_stream_group_update_num_top(stream_group, stream->num_subtop); + + return 0; } void nghttp2_stream_group_init(nghttp2_stream_group *stream_group, @@ -633,6 +766,7 @@ void nghttp2_stream_group_init(nghttp2_stream_group *stream_group, nghttp2_map_entry_init(&stream_group->map_entry, pri_group_id); stream_group->num_streams = 0; + stream_group->num_top = 0; stream_group->pri_group_id = pri_group_id; stream_group->weight = weight; } @@ -665,3 +799,24 @@ void nghttp2_stream_group_remove_stream(nghttp2_stream_group *stream_group, --stream_group->num_streams; } + +void nghttp2_stream_group_update_num_top(nghttp2_stream_group *stream_group, + ssize_t delta) +{ + DEBUGF(fprintf(stderr, "stream_group: stream_group(%p)=%d " + "update num_top current=%zd, delta=%zd, after=%zd\n", + stream_group->num_top, delta, stream_group->num_top + delta)); + + stream_group->num_top += delta; + + assert(stream_group->num_top >= 0); +} + +size_t nghttp2_stream_group_shared_wait(nghttp2_stream_group *stream_group) +{ + if(stream_group->num_top == 0) { + return 1; + } + + return nghttp2_max(1, stream_group->weight / stream_group->num_top); +} diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index c43f9e75..fa894fb5 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -138,8 +138,11 @@ struct nghttp2_stream { /* categorized priority of this stream. Only stream bearing NGHTTP2_STREAM_DPRI_TOP can send DATA frame. */ nghttp2_stream_dpri dpri; - /* the number of nodes in subtree */ + /* the number of streams in subtree */ size_t num_substreams; + /* the number of streams marked as NGHTTP2_STREAM_DPRI_TOP in + subtree */ + ssize_t num_subtop; /* Current remote window size. This value is computed against the current initial window size of remote endpoint. */ int32_t remote_window_size; @@ -322,7 +325,8 @@ int nghttp2_stream_dep_add_subtree(nghttp2_stream *dep_stream, /* * Removes subtree whose root stream is |stream|. Removing subtree - * does not change dpri values. + * does not change dpri values and removed subtree is still in the + * same stream_group. * * This function returns 0 if it succeeds, or one of the following * negative error codes: @@ -354,6 +358,8 @@ struct nghttp2_stream_group { nghttp2_map_entry map_entry; /* The number of streams this priority group contains */ size_t num_streams; + /* The number of streams marked as NGHTTP2_STREAM_DPRI_TOP */ + ssize_t num_top; /* The priority group ID */ int32_t pri_group_id; /* The weight of this group */ @@ -378,4 +384,15 @@ void nghttp2_stream_group_add_stream(nghttp2_stream_group *stream_group, void nghttp2_stream_group_remove_stream(nghttp2_stream_group *stream_group, nghttp2_stream *stream); +/* + * Updates |stream_group->num_top| += |delta| + */ +void nghttp2_stream_group_update_num_top(nghttp2_stream_group *stream_group, + ssize_t delta); + +/* + * Returns shared weight among the streams belongs to |stream_group|. + */ +size_t nghttp2_stream_group_shared_wait(nghttp2_stream_group *stream_group); + #endif /* NGHTTP2_STREAM */ diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 290ecc50..b1ba18de 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -5224,6 +5224,12 @@ void test_nghttp2_session_stream_attach_data(void) CU_ASSERT(1 == db->queued); + CU_ASSERT(1 == a->stream_group->num_top); + CU_ASSERT(1 == a->num_subtop); + CU_ASSERT(1 == b->num_subtop); + CU_ASSERT(0 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + dc = create_data_ob_item(); nghttp2_stream_attach_data(c, dc, &session->ob_pq); @@ -5235,6 +5241,12 @@ void test_nghttp2_session_stream_attach_data(void) CU_ASSERT(1 == dc->queued); + CU_ASSERT(2 == a->stream_group->num_top); + CU_ASSERT(2 == a->num_subtop); + CU_ASSERT(1 == b->num_subtop); + CU_ASSERT(1 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + da = create_data_ob_item(); nghttp2_stream_attach_data(a, da, &session->ob_pq); @@ -5246,6 +5258,12 @@ void test_nghttp2_session_stream_attach_data(void) CU_ASSERT(1 == da->queued); + CU_ASSERT(1 == a->stream_group->num_top); + CU_ASSERT(1 == a->num_subtop); + CU_ASSERT(0 == b->num_subtop); + CU_ASSERT(0 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + nghttp2_stream_detach_data(a, &session->ob_pq); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == a->dpri); @@ -5253,6 +5271,12 @@ void test_nghttp2_session_stream_attach_data(void) CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == c->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == d->dpri); + CU_ASSERT(2 == a->stream_group->num_top); + CU_ASSERT(2 == a->num_subtop); + CU_ASSERT(1 == b->num_subtop); + CU_ASSERT(1 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + dd = create_data_ob_item(); nghttp2_stream_attach_data(d, dd, &session->ob_pq); @@ -5264,6 +5288,12 @@ void test_nghttp2_session_stream_attach_data(void) CU_ASSERT(0 == dd->queued); + CU_ASSERT(2 == a->stream_group->num_top); + CU_ASSERT(2 == a->num_subtop); + CU_ASSERT(1 == b->num_subtop); + CU_ASSERT(1 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + nghttp2_stream_detach_data(c, &session->ob_pq); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == a->dpri); @@ -5273,6 +5303,12 @@ void test_nghttp2_session_stream_attach_data(void) CU_ASSERT(1 == dd->queued); + CU_ASSERT(2 == a->stream_group->num_top); + CU_ASSERT(2 == a->num_subtop); + CU_ASSERT(1 == b->num_subtop); + CU_ASSERT(1 == c->num_subtop); + CU_ASSERT(1 == d->num_subtop); + nghttp2_session_del(session); } @@ -5280,8 +5316,8 @@ void test_nghttp2_session_stream_attach_data_subtree(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; - nghttp2_stream *a, *b, *c, *d, *e, *f; - nghttp2_outbound_item *db, *de; + nghttp2_stream *a, *b, *c, *d, *e, *f, *g, *h; + nghttp2_outbound_item *db, *dd, *de; (void)d; @@ -5296,8 +5332,9 @@ void test_nghttp2_session_stream_attach_data_subtree(void) e = open_stream(session, 9); f = open_stream_with_dep(session, 11, e); - - /* a e + /* gr.1 gr.9 + * + * a e * | | * b--c f * | @@ -5312,9 +5349,16 @@ void test_nghttp2_session_stream_attach_data_subtree(void) nghttp2_stream_attach_data(b, db, &session->ob_pq); + CU_ASSERT(1 == a->stream_group->num_top); + CU_ASSERT(1 == e->stream_group->num_top); + + /* Insert subtree e under a */ + nghttp2_stream_dep_insert_subtree(a, e, &session->ob_pq); - /* a + /* gr.1 + * + * a * | * e * | @@ -5327,12 +5371,185 @@ void test_nghttp2_session_stream_attach_data_subtree(void) CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == e->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == f->dpri); + CU_ASSERT(1 == a->stream_group->num_top); + CU_ASSERT(1 == a->num_subtop); + CU_ASSERT(1 == e->num_subtop); + CU_ASSERT(0 == f->num_subtop); + CU_ASSERT(0 == b->num_subtop); + CU_ASSERT(0 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + + /* Remove subtree b */ + nghttp2_stream_dep_remove_subtree(b); nghttp2_stream_dep_make_root(b->stream_group, b, &session->ob_pq); + /* gr.1 gr.1 + * + * a b + * | + * e + * | + * f--c + * | + * d + */ + CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == b->dpri); + /* a and b are still same group */ + CU_ASSERT(b->stream_group == a->stream_group); + + CU_ASSERT(2 == b->stream_group->num_top); + CU_ASSERT(1 == b->num_subtop); + + CU_ASSERT(2 == a->stream_group->num_top); + CU_ASSERT(1 == a->num_subtop); + CU_ASSERT(1 == e->num_subtop); + CU_ASSERT(0 == f->num_subtop); + CU_ASSERT(0 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + + /* Remove subtree a */ + + nghttp2_stream_dep_remove_subtree(a); + + nghttp2_stream_dep_make_root(a->stream_group, a, &session->ob_pq); + + CU_ASSERT(2 == a->stream_group->num_top); + CU_ASSERT(1 == a->num_subtop); + CU_ASSERT(1 == e->num_subtop); + CU_ASSERT(0 == f->num_subtop); + CU_ASSERT(0 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + + /* Remove subtree c */ + + nghttp2_stream_dep_remove_subtree(c); + + nghttp2_stream_dep_make_root(c->stream_group, c, &session->ob_pq); + + /* gr.1 gr.1 gr.1 + * + * a b c + * | | + * e d + * | + * f + */ + + CU_ASSERT(2 == a->stream_group->num_top); + CU_ASSERT(1 == a->num_subtop); + CU_ASSERT(1 == e->num_subtop); + CU_ASSERT(0 == f->num_subtop); + + CU_ASSERT(0 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + + dd = create_data_ob_item(); + + nghttp2_stream_attach_data(d, dd, &session->ob_pq); + + CU_ASSERT(3 == a->stream_group->num_top); + CU_ASSERT(1 == c->num_subtop); + CU_ASSERT(1 == d->num_subtop); + + /* Add subtree c to a */ + + nghttp2_stream_dep_add_subtree(a, c, &session->ob_pq); + + /* gr.1 gr.1 + * + * a b + * | + * e--c + * | | + * f d + */ + + CU_ASSERT(3 == a->stream_group->num_top); + CU_ASSERT(2 == a->num_subtop); + CU_ASSERT(1 == e->num_subtop); + CU_ASSERT(0 == f->num_subtop); + CU_ASSERT(1 == c->num_subtop); + CU_ASSERT(1 == d->num_subtop); + + /* Insert b under a */ + + nghttp2_stream_dep_insert_subtree(a, b, &session->ob_pq); + + /* gr.1 + * + * a + * | + * b + * | + * e--c + * | | + * f d + */ + + CU_ASSERT(1 == a->stream_group->num_top); + CU_ASSERT(1 == a->num_subtop); + CU_ASSERT(1 == b->num_subtop); + CU_ASSERT(0 == e->num_subtop); + CU_ASSERT(0 == f->num_subtop); + CU_ASSERT(0 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + + g = open_stream(session, 13); + h = open_stream(session, 15); + + nghttp2_stream_dep_make_root(a->stream_group, h, &session->ob_pq); + + nghttp2_stream_dep_make_root(g->stream_group, a, &session->ob_pq); + + /* gr.13 gr.13 gr.1 + * + * a g h + * | + * b + * | + * e--c + * | | + * f d + */ + + CU_ASSERT(g->stream_group == a->stream_group); + + CU_ASSERT(0 == h->stream_group->num_top); + + CU_ASSERT(1 == a->stream_group->num_top); + CU_ASSERT(1 == a->num_subtop); + CU_ASSERT(1 == b->num_subtop); + CU_ASSERT(0 == e->num_subtop); + CU_ASSERT(0 == f->num_subtop); + CU_ASSERT(0 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + + /* Remove subtree b */ + + nghttp2_stream_dep_remove_subtree(b); + + /* gr.13 gr.13 gr.13 gr.1 + * + * b a g h + * | + * e--c + * | | + * f d + */ + + CU_ASSERT(1 == b->stream_group->num_top); + CU_ASSERT(1 == b->num_subtop); + CU_ASSERT(0 == e->num_subtop); + CU_ASSERT(0 == f->num_subtop); + CU_ASSERT(0 == c->num_subtop); + CU_ASSERT(0 == d->num_subtop); + + CU_ASSERT(0 == a->num_subtop); + nghttp2_session_del(session); }