diff --git a/lib/nghttp2_outbound_item.h b/lib/nghttp2_outbound_item.h index b400babe..9d225ced 100644 --- a/lib/nghttp2_outbound_item.h +++ b/lib/nghttp2_outbound_item.h @@ -33,12 +33,12 @@ #include "nghttp2_frame.h" #include "nghttp2_mem.h" -/* A bit higher weight for non-DATA frames */ -#define NGHTTP2_OB_EX_WEIGHT 300 -/* Higher weight for SETTINGS */ -#define NGHTTP2_OB_SETTINGS_WEIGHT 301 -/* Highest weight for PING */ -#define NGHTTP2_OB_PING_WEIGHT 302 +/* A bit higher priority for non-DATA frames */ +#define NGHTTP2_OB_EX_CYCLE 2 +/* Even more higher priority for SETTINGS frame */ +#define NGHTTP2_OB_SETTINGS_CYCLE 1 +/* Highest priority for PING frame */ +#define NGHTTP2_OB_PING_CYCLE 0 /* struct used for HEADERS and PUSH_PROMISE frame */ typedef struct { @@ -108,12 +108,14 @@ typedef struct { nghttp2_frame frame; nghttp2_aux_data aux_data; int64_t seq; - /* Reset count of weight. See comment for last_cycle in - nghttp2_session.h */ + /* The priority used in priority comparion. Smaller is served + ealier. For PING, SETTINGS and non-DATA frames (excluding + response HEADERS frame) have dedicated cycle value defined above. + For DATA frame, cycle is computed by taking into account of + effective weight and frame payload length previously sent, so + that the amount of transmission is distributed across streams + proportional to effective weight (inside a tree). */ uint64_t cycle; - /* The priority used in priority comparion. Larger is served - ealier. */ - int32_t weight; /* nonzero if this object is queued. */ uint8_t queued; } nghttp2_outbound_item; diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index a8c4d857..02fe062c 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -228,12 +228,7 @@ static int outbound_item_compar(const void *lhsx, const void *rhsx) { rhs = (const nghttp2_outbound_item *)rhsx; if (lhs->cycle == rhs->cycle) { - if (lhs->weight == rhs->weight) { - return (lhs->seq < rhs->seq) ? -1 : ((lhs->seq > rhs->seq) ? 1 : 0); - } - - /* Larger weight has higher precedence */ - return rhs->weight - lhs->weight; + return (lhs->seq < rhs->seq) ? -1 : ((lhs->seq > rhs->seq) ? 1 : 0); } return (lhs->cycle < rhs->cycle) ? -1 : 1; @@ -367,7 +362,9 @@ static int session_new(nghttp2_session **session_ptr, nghttp2_stream_roots_init(&(*session_ptr)->roots); (*session_ptr)->next_seq = 0; - (*session_ptr)->last_cycle = 1; + /* Do +1 so that any HEADERS/DATA frames are scheduled after urgent + frames. */ + (*session_ptr)->last_cycle = NGHTTP2_OB_EX_CYCLE + 1; (*session_ptr)->remote_window_size = NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE; (*session_ptr)->recv_window_size = 0; @@ -694,9 +691,7 @@ nghttp2_session_reprioritize_stream(nghttp2_session *session, void nghttp2_session_outbound_item_init(nghttp2_session *session, nghttp2_outbound_item *item) { item->seq = session->next_seq++; - /* We use cycle for DATA only */ - item->cycle = 0; - item->weight = NGHTTP2_OB_EX_WEIGHT; + item->cycle = NGHTTP2_OB_EX_CYCLE; item->queued = 0; memset(&item->aux_data, 0, sizeof(nghttp2_aux_data)); @@ -723,12 +718,12 @@ int nghttp2_session_add_item(nghttp2_session *session, break; case NGHTTP2_SETTINGS: - item->weight = NGHTTP2_OB_SETTINGS_WEIGHT; + item->cycle = NGHTTP2_OB_SETTINGS_CYCLE; break; case NGHTTP2_PING: /* Ping has highest priority. */ - item->weight = NGHTTP2_OB_PING_WEIGHT; + item->cycle = NGHTTP2_OB_PING_CYCLE; break; default: @@ -752,7 +747,6 @@ int nghttp2_session_add_item(nghttp2_session *session, item->queued = 1; } else if (stream && (stream->state == NGHTTP2_STREAM_RESERVED || item->aux_data.headers.attach_stream)) { - item->weight = stream->effective_weight; item->cycle = session->last_cycle; rv = nghttp2_stream_attach_item(stream, item, session); @@ -790,7 +784,6 @@ int nghttp2_session_add_item(nghttp2_session *session, return NGHTTP2_ERR_DATA_EXIST; } - item->weight = stream->effective_weight; item->cycle = session->last_cycle; rv = nghttp2_stream_attach_item(stream, item, session); @@ -1986,7 +1979,8 @@ static int session_prep_frame(nghttp2_session *session, } rv = nghttp2_session_pack_data(session, &session->aob.framebufs, - next_readmax, frame, &item->aux_data.data); + next_readmax, frame, &item->aux_data.data, + stream); if (rv == NGHTTP2_ERR_DEFERRED) { rv = nghttp2_stream_defer_item(stream, NGHTTP2_STREAM_FLAG_DEFERRED_USER, session); @@ -2069,8 +2063,7 @@ nghttp2_session_get_next_ob_item(nghttp2_session *session) { headers_item = nghttp2_pq_top(&session->ob_ss_pq); if (session_is_outgoing_concurrent_streams_max(session) || - item->weight > headers_item->weight || - (item->weight == headers_item->weight && item->seq < headers_item->seq)) { + outbound_item_compar(item, headers_item) < 0) { return item; } @@ -2133,8 +2126,7 @@ nghttp2_session_pop_next_ob_item(nghttp2_session *session) { headers_item = nghttp2_pq_top(&session->ob_ss_pq); if (session_is_outgoing_concurrent_streams_max(session) || - item->weight > headers_item->weight || - (item->weight == headers_item->weight && item->seq < headers_item->seq)) { + outbound_item_compar(item, headers_item) < 0) { nghttp2_pq_pop(&session->ob_pq); item->queued = 0; @@ -2249,21 +2241,13 @@ static int session_close_stream_on_goaway(nghttp2_session *session, return 0; } -static void session_outbound_item_cycle_weight(nghttp2_session *session, - nghttp2_outbound_item *item, - int32_t ini_weight) { - if (item->weight == NGHTTP2_MIN_WEIGHT || item->weight > ini_weight) { +static void session_outbound_item_schedule(nghttp2_session *session, + nghttp2_outbound_item *item, + int32_t weight) { + size_t delta = item->frame.hd.length * NGHTTP2_MAX_WEIGHT / weight; - item->weight = ini_weight; - - if (item->cycle == session->last_cycle) { - item->cycle = ++session->last_cycle; - } else { - item->cycle = session->last_cycle; - } - } else { - --item->weight; - } + session->last_cycle = nghttp2_max(session->last_cycle, item->cycle); + item->cycle = session->last_cycle + delta; } /* @@ -2583,15 +2567,6 @@ static int session_after_frame_sent2(nghttp2_session *session) { assert(stream); next_item = nghttp2_session_get_next_ob_item(session); - /* Imagine we hit connection window size limit while sending DATA - frame. If we decrement weight here, its stream might get - inferior share because the other streams' weight is not - decremented because of flow control. */ - if (session->remote_window_size > 0 || stream->remote_window_size <= 0) { - session_outbound_item_cycle_weight(session, aob->item, - stream->effective_weight); - } - /* If priority of this stream is higher or equal to other stream waiting at the top of the queue, we continue to send this data. */ @@ -2634,7 +2609,7 @@ static int session_after_frame_sent2(nghttp2_session *session) { nghttp2_bufs_reset(framebufs); rv = nghttp2_session_pack_data(session, framebufs, next_readmax, frame, - aux_data); + aux_data, stream); if (nghttp2_is_fatal(rv)) { return rv; } @@ -6229,7 +6204,8 @@ int nghttp2_session_add_settings(nghttp2_session *session, uint8_t flags, int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_bufs *bufs, size_t datamax, nghttp2_frame *frame, - nghttp2_data_aux_data *aux_data) { + nghttp2_data_aux_data *aux_data, + nghttp2_stream *stream) { int rv; uint32_t data_flags; ssize_t payloadlen; @@ -6242,12 +6218,6 @@ int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_bufs *bufs, buf = &bufs->cur->buf; if (session->callbacks.read_length_callback) { - nghttp2_stream *stream; - - stream = nghttp2_session_get_stream(session, frame->hd.stream_id); - if (!stream) { - return NGHTTP2_ERR_INVALID_ARGUMENT; - } payloadlen = session->callbacks.read_length_callback( session, frame->hd.type, stream->stream_id, session->remote_window_size, @@ -6361,6 +6331,9 @@ int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_bufs *bufs, return rv; } + session_outbound_item_schedule(session, stream->item, + stream->effective_weight); + return 0; } diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 94f990cc..56709ec6 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -704,7 +704,8 @@ nghttp2_stream *nghttp2_session_get_stream_raw(nghttp2_session *session, */ int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_bufs *bufs, size_t datamax, nghttp2_frame *frame, - nghttp2_data_aux_data *aux_data); + nghttp2_data_aux_data *aux_data, + nghttp2_stream *stream); /* * Returns top of outbound frame queue. This function returns NULL if diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 09b70d79..16981efc 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -102,17 +102,15 @@ static int stream_push_item(nghttp2_stream *stream, nghttp2_session *session) { return 0; } - if (item->weight > stream->effective_weight) { - item->weight = stream->effective_weight; - } - - item->cycle = session->last_cycle; - switch (item->frame.hd.type) { case NGHTTP2_DATA: + /* Penalize low weight stream */ + item->cycle = + session->last_cycle + NGHTTP2_MAX_WEIGHT / stream->effective_weight; rv = nghttp2_pq_push(&session->ob_da_pq, item); break; case NGHTTP2_HEADERS: + item->cycle = session->last_cycle; if (stream->state == NGHTTP2_STREAM_RESERVED) { rv = nghttp2_pq_push(&session->ob_ss_pq, item); } else { diff --git a/src/nghttp.cc b/src/nghttp.cc index 05bd22ac..3d513f4f 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -2062,11 +2062,12 @@ int communicate( dep_stream_id = ANCHOR_ID_HIGH; } - nghttp2_priority_spec_init(&pri_spec, dep_stream_id, config.weight, 0); - for (auto req : requests) { for (int i = 0; i < config.multiply; ++i) { auto dep = std::make_shared(); + nghttp2_priority_spec_init(&pri_spec, dep_stream_id, + config.weight * (i + 1), 0); + client.add_request(std::get<0>(req), std::get<1>(req), std::get<2>(req), pri_spec, std::move(dep)); }