From 41104f7b6359e98989e50121e331be39be5ec02e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 14 Dec 2013 17:02:59 +0900 Subject: [PATCH] Abandon DATA frame priority adjustment (again) We tried several times about this subject, but for the current HTTP/2.0 priority scheme, we think it is best to serve the highest priroty streams first (interleaving streams if there are several higest ones). There are an issue when aggregating several frontend connections to one connection in backend, but it is HTTP/2.0 spec issue, rather than implementation. --- lib/nghttp2_outbound_item.h | 2 -- lib/nghttp2_session.c | 30 ++---------------------------- tests/nghttp2_session_test.c | 6 +++--- 3 files changed, 5 insertions(+), 33 deletions(-) diff --git a/lib/nghttp2_outbound_item.h b/lib/nghttp2_outbound_item.h index a757e4e9..e9e6c00a 100644 --- a/lib/nghttp2_outbound_item.h +++ b/lib/nghttp2_outbound_item.h @@ -51,8 +51,6 @@ typedef struct { nghttp2_frame_category frame_cat; /* The priority used in priority comparion */ int32_t pri; - /* The initial priority */ - int32_t inipri; } nghttp2_outbound_item; /* diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 63a734fa..21f7769e 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -424,8 +424,7 @@ static int outbound_item_update_pri return 0; } } - /* We only update initial priority */ - item->inipri = stream->pri; + item->pri = stream->pri; return 1; } @@ -448,7 +447,7 @@ void nghttp2_session_reprioritize_stream nghttp2_pq_update(&session->ob_pq, update_stream_pri, stream); nghttp2_pq_update(&session->ob_ss_pq, update_stream_pri, stream); if(stream->deferred_data) { - stream->deferred_data->inipri = pri; + stream->deferred_data->pri = pri; } if(session->aob.item) { outbound_item_update_pri(session->aob.item, stream); @@ -549,7 +548,6 @@ int nghttp2_session_add_frame(nghttp2_session *session, free(item); return r; } - item->inipri = item->pri; return 0; } @@ -1389,29 +1387,6 @@ nghttp2_outbound_item* nghttp2_session_pop_next_ob_item } } -/* - * Adjust priority of the DATA frame |item|. In order to prevent the - * low priority streams from starving, lower the priority of the - * |item|. If the resulting priority exceeds NGHTTP2_PRI_DEFAULT, - * back to the original priority. - */ -static void adjust_data_pri(nghttp2_outbound_item *item) -{ - if(item->pri == NGHTTP2_PRI_LOWEST) { - item->pri = item->inipri; - return; - } - if(item->pri & 0x40000000) { - item->pri = NGHTTP2_PRI_LOWEST; - return; - } - if(item->pri == 0) { - item->pri = 1; - } else { - item->pri <<= 1; - } -} - /* * Called after a frame is sent. * @@ -1590,7 +1565,6 @@ static int nghttp2_session_after_frame_sent(nghttp2_session *session) } else { nghttp2_outbound_item* next_item; next_item = nghttp2_session_get_next_ob_item(session); - adjust_data_pri(session->aob.item); /* 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. */ diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 1d957ff2..1e6a8c13 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1802,13 +1802,13 @@ void test_nghttp2_session_reprioritize_stream(void) nghttp2_session_reprioritize_stream(session, stream, 120); CU_ASSERT(session->aob.item != NULL); - CU_ASSERT(120 == session->aob.item->inipri); + CU_ASSERT(120 == session->aob.item->pri); CU_ASSERT(120 == stream->pri); CU_ASSERT(5000 == nghttp2_session_get_stream(session, 1)->pri); item = nghttp2_session_get_next_ob_item(session); - CU_ASSERT(5000 == item->inipri); + CU_ASSERT(120 == item->pri); CU_ASSERT(NGHTTP2_HEADERS == OB_CTRL_TYPE(item)); - CU_ASSERT(1 == OB_CTRL(item)->hd.stream_id); + CU_ASSERT(3 == OB_CTRL(item)->hd.stream_id); nghttp2_session_del(session);