From 03c4092862dc64a1e8e90b40a13b78d4792da7ae Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 7 Oct 2014 23:52:36 +0900 Subject: [PATCH] Distribute closed or blocked stream's weight to its siblings This also means that at least one stream whose dpri is NGHTTP2_STREAM_DPRI_TOP exists, its siblings descendants have no chance to send streams, even if their parent stream has NGHTTP2_STREAM_DPRI_NODATA. --- lib/nghttp2_stream.c | 76 ++++++++++++++++++++++++++++++------ lib/nghttp2_stream.h | 3 ++ tests/nghttp2_session_test.c | 10 ++--- 3 files changed, 70 insertions(+), 19 deletions(-) diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index f4d54e1b..3c4c896e 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -65,6 +65,7 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, stream->effective_weight = stream->weight; stream->sum_dep_weight = 0; stream->sum_norest_weight = 0; + stream->sum_top_weight = 0; stream->roots = roots; stream->root_prev = NULL; @@ -157,6 +158,20 @@ int32_t nghttp2_stream_dep_distributed_effective_weight return nghttp2_max(1, weight); } +static int32_t stream_dep_distributed_top_effective_weight +(nghttp2_stream *stream, int32_t weight) +{ + if(stream->sum_top_weight == 0) { + return stream->effective_weight; + } + + weight = stream->effective_weight * weight / stream->sum_top_weight; + + return nghttp2_max(1, weight); +} + +static void stream_update_dep_set_rest(nghttp2_stream *stream); + /* Updates effective_weight of descendant streams in subtree of |stream|. We assume that stream->effective_weight is already set right. */ @@ -165,9 +180,10 @@ static void stream_update_dep_effective_weight(nghttp2_stream *stream) nghttp2_stream *si; DEBUGF(fprintf(stderr, "stream: update_dep_effective_weight " - "stream(%p)=%d, weight=%d, sum_norest_weight=%d\n", + "stream(%p)=%d, weight=%d, sum_norest_weight=%d, " + "sum_top_weight=%d\n", stream, stream->stream_id, stream->weight, - stream->sum_norest_weight)); + stream->sum_norest_weight, stream->sum_top_weight)); /* stream->sum_norest_weight == 0 means there is no NGHTTP2_STREAM_DPRI_TOP under stream */ @@ -176,13 +192,38 @@ static void stream_update_dep_effective_weight(nghttp2_stream *stream) return; } + /* If there is no direct descendant whose dpri is + NGHTTP2_STREAM_DPRI_TOP, indirect descendants have the chance to + send data, so recursively set weight for descendants. */ + if(stream->sum_top_weight == 0) { + for(si = stream->dep_next; si; si = si->sib_next) { + if(si->dpri != NGHTTP2_STREAM_DPRI_REST) { + si->effective_weight = nghttp2_stream_dep_distributed_effective_weight + (stream, si->weight); + } + + stream_update_dep_effective_weight(si); + } + return; + } + + /* If there is at least one direct descendant whose dpri is + NGHTTP2_STREAM_DPRI_TOP, we won't give a chance to indirect + descendants, since closed or blocked stream's weight is + distributed among its siblings */ for(si = stream->dep_next; si; si = si->sib_next) { - if(si->dpri != NGHTTP2_STREAM_DPRI_REST) { - si->effective_weight = nghttp2_stream_dep_distributed_effective_weight + if(si->dpri == NGHTTP2_STREAM_DPRI_TOP) { + si->effective_weight = stream_dep_distributed_top_effective_weight (stream, si->weight); + + continue; } - stream_update_dep_effective_weight(si); + if(si->dpri == NGHTTP2_STREAM_DPRI_NO_DATA) { + /* Since we marked NGHTTP2_STREAM_DPRI_TOP under si, we make + them NGHTTP2_STREAM_DPRI_REST again. */ + stream_update_dep_set_rest(si->dep_next); + } } } @@ -257,14 +298,18 @@ static int stream_update_dep_set_top(nghttp2_stream *stream, nghttp2_pq *pq, } /* - * Updates stream->sum_norest_weight recursively. We have to gather - * effective sum of weight of descendants. If stream->dpri == - * NGHTTP2_STREAM_DPRI_NO_DATA, we have to go deeper and check that - * any of its descendants has dpri value of NGHTTP2_STREAM_DPRI_TOP. - * If so, we have to add weight of its direct descendants to - * stream->sum_norest_weight. To make this work, this function - * returns 1 if any of its descendants has dpri value of - * NGHTTP2_STREAM_DPRI_TOP, otherwise 0. + * Updates stream->sum_norest_weight and stream->sum_top_weight + * recursively. We have to gather effective sum of weight of + * descendants. If stream->dpri == NGHTTP2_STREAM_DPRI_NO_DATA, we + * have to go deeper and check that any of its descendants has dpri + * value of NGHTTP2_STREAM_DPRI_TOP. If so, we have to add weight of + * its direct descendants to stream->sum_norest_weight. To make this + * work, this function returns 1 if any of its descendants has dpri + * value of NGHTTP2_STREAM_DPRI_TOP, otherwise 0. + * + * Calculating stream->sum_top-weight is very simple compared to + * stream->sum_norest_weight. It just adds up the weight of direct + * descendants whose dpri is NGHTTP2_STREAM_DPRI_TOP. */ static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream) { @@ -272,6 +317,7 @@ static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream) int rv; stream->sum_norest_weight = 0; + stream->sum_top_weight = 0; if(stream->dpri == NGHTTP2_STREAM_DPRI_TOP) { return 1; @@ -289,6 +335,10 @@ static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream) rv = 1; stream->sum_norest_weight += si->weight; } + + if(si->dpri == NGHTTP2_STREAM_DPRI_TOP) { + stream->sum_top_weight += si->weight; + } } return rv; diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index bc0648ef..e82ac3ed 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -172,6 +172,9 @@ struct nghttp2_stream { descendant with dpri == NGHTTP2_STREAM_DPRI_TOP. We use this value to calculate effective weight. */ int32_t sum_norest_weight; + /* sum of weight of direct descendants whose dpri value is + NGHTTP2_STREAM_DPRI_TOP */ + int32_t sum_top_weight; nghttp2_stream_state state; /* This is bitwise-OR of 0 or more of nghttp2_stream_flag. */ uint8_t flags; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index c42a530f..68948790 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -6080,10 +6080,9 @@ void test_nghttp2_session_stream_attach_data(void) CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == a->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == b->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == c->dpri); - CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == d->dpri); + CU_ASSERT(NGHTTP2_STREAM_DPRI_REST == d->dpri); - CU_ASSERT(16 * 16 / 32 == b->effective_weight); - CU_ASSERT(16 * 16 / 32 == d->effective_weight); + CU_ASSERT(16 * 16 / 16 == b->effective_weight); CU_ASSERT(1 == dd->queued); @@ -6240,13 +6239,12 @@ void test_nghttp2_session_stream_attach_data_subtree(void) CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == a->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == b->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == c->dpri); - CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == d->dpri); + CU_ASSERT(NGHTTP2_STREAM_DPRI_REST == d->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == e->dpri); CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == f->dpri); CU_ASSERT(16 == b->effective_weight); - CU_ASSERT(16 * 16 / 32 == e->effective_weight); - CU_ASSERT(16 * 16 / 32 == d->effective_weight); + CU_ASSERT(16 * 16 / 16 == e->effective_weight); CU_ASSERT(32 == a->sum_norest_weight); CU_ASSERT(16 == c->sum_norest_weight);