From 72f815d5355de82919818ee5f42c6463715b9e84 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 7 Dec 2015 22:43:15 +0900 Subject: [PATCH] Update descendant_last_cycle on nghttp2_stream_next_outbound_item Previously, we updated descendant_last_cycle in nghttp2_stream_reschedule, which is called after non-zero DATA frame. But this was not optimal since we still had old descendant_last_cycle, and new stream was scheduled based on it. Now descendant_last_cycle is updated in nghttp2_stream_next_outbound_item, which is called when stream with highest priority is selected from queue. And new stream is scheduled based on it. This commit also removes 0-reset of descendant_last_cycle and cycle in nghttp2_stream_reschedule. This could help making them lower, so that they are not overflow. But there is a pattern that it doesn't work, and we are not sure they are really useful at this moment. --- lib/nghttp2_stream.c | 28 +++++++++++----------------- lib/nghttp2_stream.h | 3 ++- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 54219a4d..06b64850 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -207,25 +207,13 @@ void nghttp2_stream_reschedule(nghttp2_stream *stream) { dep_stream = stream->dep_prev; for (; dep_stream; stream = dep_stream, dep_stream = dep_stream->dep_prev) { - if (nghttp2_pq_size(&dep_stream->obq) == 1) { - dep_stream->descendant_last_cycle = 0; - stream->cycle = 0; - } else { - /* We update descendant_last_cycle here, and we don't do it when - no data is written for stream. This effectively means that - we treat these streams as if they are not scheduled at all. - This does not cause disruption in scheduling machinery. It - just makes new streams scheduled a bit early. */ - dep_stream->descendant_last_cycle = stream->cycle; + nghttp2_pq_remove(&dep_stream->obq, &stream->pq_entry); - nghttp2_pq_remove(&dep_stream->obq, &stream->pq_entry); + stream->cycle = + stream_next_cycle(stream, dep_stream->descendant_last_cycle); + stream->seq = dep_stream->descendant_next_seq++; - stream->cycle = - stream_next_cycle(stream, dep_stream->descendant_last_cycle); - stream->seq = dep_stream->descendant_next_seq++; - - nghttp2_pq_push(&dep_stream->obq, &stream->pq_entry); - } + nghttp2_pq_push(&dep_stream->obq, &stream->pq_entry); DEBUGF(fprintf(stderr, "stream: stream=%d obq resched cycle=%ld\n", stream->stream_id, stream->cycle)); @@ -861,9 +849,15 @@ int nghttp2_stream_in_dep_tree(nghttp2_stream *stream) { nghttp2_outbound_item * nghttp2_stream_next_outbound_item(nghttp2_stream *stream) { nghttp2_pq_entry *ent; + nghttp2_stream *si; for (;;) { if (stream_active(stream)) { + /* Update ascendant's descendant_last_cycle here, so that we can + assure that new stream is scheduled based on it. */ + for (si = stream; si->dep_prev; si = si->dep_prev) { + si->dep_prev->descendant_last_cycle = si->cycle; + } return stream->item; } ent = nghttp2_pq_top(&stream->obq); diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index e2dddefa..bbb8b9f3 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -419,7 +419,8 @@ int nghttp2_stream_in_dep_tree(nghttp2_stream *stream); void nghttp2_stream_reschedule(nghttp2_stream *stream); /* - * Returns a stream which has highest priority. + * Returns a stream which has highest priority, updating + * descendant_last_cycle of selected stream's ancestors. */ nghttp2_outbound_item * nghttp2_stream_next_outbound_item(nghttp2_stream *stream);