From eb05777d8823f4f6a8ce385f97bf98efef639b5a Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 24 Apr 2015 00:16:14 +0900 Subject: [PATCH 01/17] clang-format --- examples/libevent-client.c | 3 +-- src/asio_io_service_pool.cc | 6 +++--- src/asio_server.cc | 10 ++-------- src/asio_server_http2.cc | 18 +++++------------- src/asio_server_http2_impl.cc | 12 ++++-------- src/asio_server_http2_impl.h | 8 +++----- src/comp_helper.c | 2 +- src/util.cc | 4 ++-- 8 files changed, 21 insertions(+), 42 deletions(-) diff --git a/examples/libevent-client.c b/examples/libevent-client.c index 98c194a6..4cda9b6f 100644 --- a/examples/libevent-client.c +++ b/examples/libevent-client.c @@ -258,8 +258,7 @@ static int on_data_chunk_recv_callback(nghttp2_session *session _U_, stream), if it is closed, we send GOAWAY and tear down the session */ static int on_stream_close_callback(nghttp2_session *session, int32_t stream_id, - uint32_t error_code, - void *user_data) { + uint32_t error_code, void *user_data) { http2_session_data *session_data = (http2_session_data *)user_data; int rv; diff --git a/src/asio_io_service_pool.cc b/src/asio_io_service_pool.cc index b6735df0..ba405412 100644 --- a/src/asio_io_service_pool.cc +++ b/src/asio_io_service_pool.cc @@ -58,9 +58,9 @@ void io_service_pool::run(bool asynchronous) { // Create a pool of threads to run all of the io_services. for (std::size_t i = 0; i < io_services_.size(); ++i) { futures_.push_back(std::async(std::launch::async, - (size_t (boost::asio::io_service::*)(void)) & - boost::asio::io_service::run, - io_services_[i])); + (size_t (boost::asio::io_service::*)(void)) & + boost::asio::io_service::run, + io_services_[i])); } if (!asynchronous) { diff --git a/src/asio_server.cc b/src/asio_server.cc index 1f10cd5f..310e672c 100644 --- a/src/asio_server.cc +++ b/src/asio_server.cc @@ -156,15 +156,9 @@ void server::start_accept(tcp::acceptor &acceptor, serve_mux &mux) { }); } -void server::stop() -{ - io_service_pool_.stop(); -} +void server::stop() { io_service_pool_.stop(); } -void server::join() -{ - io_service_pool_.join(); -} +void server::join() { io_service_pool_.join(); } } // namespace server } // namespace asio_http2 diff --git a/src/asio_server_http2.cc b/src/asio_server_http2.cc index 85e50221..7d162e34 100644 --- a/src/asio_server_http2.cc +++ b/src/asio_server_http2.cc @@ -59,11 +59,9 @@ boost::system::error_code http2::listen_and_serve(boost::system::error_code &ec, return impl_->listen_and_serve(ec, nullptr, address, port, asynchronous); } -boost::system::error_code -http2::listen_and_serve(boost::system::error_code &ec, - boost::asio::ssl::context &tls_context, - const std::string &address, const std::string &port, - bool asynchronous) { +boost::system::error_code http2::listen_and_serve( + boost::system::error_code &ec, boost::asio::ssl::context &tls_context, + const std::string &address, const std::string &port, bool asynchronous) { return impl_->listen_and_serve(ec, &tls_context, address, port, asynchronous); } @@ -75,15 +73,9 @@ bool http2::handle(std::string pattern, request_cb cb) { return impl_->handle(std::move(pattern), std::move(cb)); } -void http2::stop() -{ - impl_->stop(); -} +void http2::stop() { impl_->stop(); } -void http2::join() -{ - return impl_->join(); -} +void http2::join() { return impl_->join(); } } // namespace server diff --git a/src/asio_server_http2_impl.cc b/src/asio_server_http2_impl.cc index 142f418c..79994a83 100644 --- a/src/asio_server_http2_impl.cc +++ b/src/asio_server_http2_impl.cc @@ -43,7 +43,8 @@ boost::system::error_code http2_impl::listen_and_serve( boost::system::error_code &ec, boost::asio::ssl::context *tls_context, const std::string &address, const std::string &port, bool asynchronous) { server_.reset(new server(num_threads_)); - return server_->listen_and_serve(ec, tls_context, address, port, backlog_, mux_, asynchronous); + return server_->listen_and_serve(ec, tls_context, address, port, backlog_, + mux_, asynchronous); } void http2_impl::num_threads(size_t num_threads) { num_threads_ = num_threads; } @@ -54,14 +55,9 @@ bool http2_impl::handle(std::string pattern, request_cb cb) { return mux_.handle(std::move(pattern), std::move(cb)); } -void http2_impl::stop() { - return server_->stop(); -} +void http2_impl::stop() { return server_->stop(); } -void http2_impl::join() -{ - return server_->join(); -} +void http2_impl::join() { return server_->join(); } } // namespace server diff --git a/src/asio_server_http2_impl.h b/src/asio_server_http2_impl.h index 244486fa..e4068694 100644 --- a/src/asio_server_http2_impl.h +++ b/src/asio_server_http2_impl.h @@ -42,11 +42,9 @@ class server; class http2_impl { public: http2_impl(); - boost::system::error_code - listen_and_serve(boost::system::error_code &ec, - boost::asio::ssl::context *tls_context, - const std::string &address, const std::string &port, - bool asynchronous); + boost::system::error_code listen_and_serve( + boost::system::error_code &ec, boost::asio::ssl::context *tls_context, + const std::string &address, const std::string &port, bool asynchronous); void num_threads(size_t num_threads); void backlog(int backlog); bool handle(std::string pattern, request_cb cb); diff --git a/src/comp_helper.c b/src/comp_helper.c index d697c6a0..8e0c03dd 100644 --- a/src/comp_helper.c +++ b/src/comp_helper.c @@ -58,7 +58,7 @@ json_t *dump_header(const uint8_t *name, size_t namelen, const uint8_t *value, json_t *nv_pair = json_object(); char *cname = malloc(namelen + 1); if (cname == NULL) { - return NULL; + return NULL; } memcpy(cname, name, namelen); cname[namelen] = '\0'; diff --git a/src/util.cc b/src/util.cc index acaf1f9f..2926ae9e 100644 --- a/src/util.cc +++ b/src/util.cc @@ -738,14 +738,14 @@ char *get_exec_path(int argc, char **const argv, const char *cwd) { if (argv0[0] == '/') { path = static_cast(malloc(len + 1)); if (path == nullptr) { - return nullptr; + return nullptr; } memcpy(path, argv0, len + 1); } else { auto cwdlen = strlen(cwd); path = static_cast(malloc(len + 1 + cwdlen + 1)); if (path == nullptr) { - return nullptr; + return nullptr; } memcpy(path, cwd, cwdlen); path[cwdlen] = '/'; From f2cf2b625c6e9307bd2a34a6f22db2cf65553131 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 24 Apr 2015 22:30:32 +0900 Subject: [PATCH 02/17] Replace priority queue with linear queue where possible After reviewing codebase, only queue for DATA frames requires priorities. Other frames can be replaced multiple linear queues. Replacing priority queue with linear queue allows us to simplify codebase a bit; for example, now nghttp2_session.next_seq is gone. --- lib/nghttp2_int.h | 3 +- lib/nghttp2_outbound_item.c | 29 ++++ lib/nghttp2_outbound_item.h | 41 +++-- lib/nghttp2_pq.c | 8 +- lib/nghttp2_pq.h | 6 +- lib/nghttp2_session.c | 308 ++++++++++++----------------------- lib/nghttp2_session.h | 24 +-- lib/nghttp2_stream.c | 26 +-- tests/nghttp2_pq_test.c | 12 +- tests/nghttp2_session_test.c | 22 +-- 10 files changed, 216 insertions(+), 263 deletions(-) diff --git a/lib/nghttp2_int.h b/lib/nghttp2_int.h index 60070176..c26c8e99 100644 --- a/lib/nghttp2_int.h +++ b/lib/nghttp2_int.h @@ -39,7 +39,8 @@ } while (0) #endif -typedef int (*nghttp2_compar)(const void *lhs, const void *rhs); +/* "less" function, return nonzero if |lhs| is less than |rhs|. */ +typedef int (*nghttp2_less)(const void *lhs, const void *rhs); /* Internal error code. They must be in the range [-499, -100], inclusive. */ diff --git a/lib/nghttp2_outbound_item.c b/lib/nghttp2_outbound_item.c index 2aaff377..263dd55d 100644 --- a/lib/nghttp2_outbound_item.c +++ b/lib/nghttp2_outbound_item.c @@ -65,3 +65,32 @@ void nghttp2_outbound_item_free(nghttp2_outbound_item *item, nghttp2_mem *mem) { break; } } + +void nghttp2_outbound_queue_init(nghttp2_outbound_queue *q) { + q->head = q->tail = NULL; + q->n = 0; +} + +void nghttp2_outbound_queue_push(nghttp2_outbound_queue *q, + nghttp2_outbound_item *item) { + if (q->tail) { + q->tail = q->tail->qnext = item; + } else { + q->head = q->tail = item; + } + ++q->n; +} + +void nghttp2_outbound_queue_pop(nghttp2_outbound_queue *q) { + nghttp2_outbound_item *item; + if (!q->head) { + return; + } + item = q->head; + q->head = q->head->qnext; + item->qnext = NULL; + if (!q->head) { + q->tail = NULL; + } + --q->n; +} diff --git a/lib/nghttp2_outbound_item.h b/lib/nghttp2_outbound_item.h index 9d225ced..74164499 100644 --- a/lib/nghttp2_outbound_item.h +++ b/lib/nghttp2_outbound_item.h @@ -33,13 +33,6 @@ #include "nghttp2_frame.h" #include "nghttp2_mem.h" -/* 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 { nghttp2_data_provider data_prd; @@ -104,10 +97,12 @@ typedef union { nghttp2_goaway_aux_data goaway; } nghttp2_aux_data; -typedef struct { +struct nghttp2_outbound_item; +typedef struct nghttp2_outbound_item nghttp2_outbound_item; + +struct nghttp2_outbound_item { nghttp2_frame frame; nghttp2_aux_data aux_data; - int64_t seq; /* 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. @@ -116,9 +111,10 @@ typedef struct { that the amount of transmission is distributed across streams proportional to effective weight (inside a tree). */ uint64_t cycle; + nghttp2_outbound_item *qnext; /* nonzero if this object is queued. */ uint8_t queued; -} nghttp2_outbound_item; +}; /* * Deallocates resource for |item|. If |item| is NULL, this function @@ -126,4 +122,29 @@ typedef struct { */ void nghttp2_outbound_item_free(nghttp2_outbound_item *item, nghttp2_mem *mem); +/* + * queue for nghttp2_outbound_item. + */ +typedef struct { + nghttp2_outbound_item *head, *tail; + /* number of items in this queue. */ + size_t n; +} nghttp2_outbound_queue; + +void nghttp2_outbound_queue_init(nghttp2_outbound_queue *q); + +/* Pushes |item| into |q| */ +void nghttp2_outbound_queue_push(nghttp2_outbound_queue *q, + nghttp2_outbound_item *item); + +/* Pops |item| at the top from |q|. If |q| is empty, nothing + happens. */ +void nghttp2_outbound_queue_pop(nghttp2_outbound_queue *q); + +/* Returns the top item. */ +#define nghttp2_outbound_queue_top(Q) ((Q)->head) + +/* Returns the size of the queue */ +#define nghttp2_outbound_queue_size(Q) ((Q)->n) + #endif /* NGHTTP2_OUTBOUND_ITEM_H */ diff --git a/lib/nghttp2_pq.c b/lib/nghttp2_pq.c index 4656fc42..f04c189f 100644 --- a/lib/nghttp2_pq.c +++ b/lib/nghttp2_pq.c @@ -24,7 +24,7 @@ */ #include "nghttp2_pq.h" -int nghttp2_pq_init(nghttp2_pq *pq, nghttp2_compar compar, nghttp2_mem *mem) { +int nghttp2_pq_init(nghttp2_pq *pq, nghttp2_less less, nghttp2_mem *mem) { pq->mem = mem; pq->capacity = 128; pq->q = nghttp2_mem_malloc(mem, pq->capacity * sizeof(void *)); @@ -32,7 +32,7 @@ int nghttp2_pq_init(nghttp2_pq *pq, nghttp2_compar compar, nghttp2_mem *mem) { return NGHTTP2_ERR_NOMEM; } pq->length = 0; - pq->compar = compar; + pq->less = less; return 0; } @@ -52,7 +52,7 @@ static void bubble_up(nghttp2_pq *pq, size_t index) { return; } else { size_t parent = (index - 1) / 2; - if (pq->compar(pq->q[parent], pq->q[index]) > 0) { + if (pq->less(pq->q[index], pq->q[parent])) { swap(pq, parent, index); bubble_up(pq, parent); } @@ -93,7 +93,7 @@ static void bubble_down(nghttp2_pq *pq, size_t index) { if (j >= pq->length) { break; } - if (pq->compar(pq->q[minindex], pq->q[j]) > 0) { + if (pq->less(pq->q[j], pq->q[minindex])) { minindex = j; } } diff --git a/lib/nghttp2_pq.h b/lib/nghttp2_pq.h index efcd2557..1775d038 100644 --- a/lib/nghttp2_pq.h +++ b/lib/nghttp2_pq.h @@ -45,8 +45,8 @@ typedef struct { /* The maximum number of items this pq can store. This is automatically extended when length is reached to this value. */ size_t capacity; - /* The compare function between items */ - nghttp2_compar compar; + /* The less function between items */ + nghttp2_less less; } nghttp2_pq; /* @@ -58,7 +58,7 @@ typedef struct { * NGHTTP2_ERR_NOMEM * Out of memory. */ -int nghttp2_pq_init(nghttp2_pq *pq, nghttp2_compar cmp, nghttp2_mem *mem); +int nghttp2_pq_init(nghttp2_pq *pq, nghttp2_less less, nghttp2_mem *mem); /* * Deallocates any resources allocated for |pq|. The stored items are diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index d0937c7c..f1e5a6bd 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -221,17 +221,13 @@ nghttp2_stream *nghttp2_session_get_stream_raw(nghttp2_session *session, return (nghttp2_stream *)nghttp2_map_find(&session->streams, stream_id); } -static int outbound_item_compar(const void *lhsx, const void *rhsx) { +static int outbound_item_less(const void *lhsx, const void *rhsx) { const nghttp2_outbound_item *lhs, *rhs; lhs = (const nghttp2_outbound_item *)lhsx; rhs = (const nghttp2_outbound_item *)rhsx; - if (lhs->cycle == rhs->cycle) { - return (lhs->seq < rhs->seq) ? -1 : ((lhs->seq > rhs->seq) ? 1 : 0); - } - - return (lhs->cycle < rhs->cycle) ? -1 : 1; + return (lhs->cycle < rhs->cycle) ? 1 : 0; } static void session_inbound_frame_reset(nghttp2_session *session) { @@ -333,15 +329,7 @@ static int session_new(nghttp2_session **session_ptr, /* next_stream_id is initialized in either nghttp2_session_client_new2 or nghttp2_session_server_new2 */ - rv = nghttp2_pq_init(&(*session_ptr)->ob_pq, outbound_item_compar, mem); - if (rv != 0) { - goto fail_ob_pq; - } - rv = nghttp2_pq_init(&(*session_ptr)->ob_ss_pq, outbound_item_compar, mem); - if (rv != 0) { - goto fail_ob_ss_pq; - } - rv = nghttp2_pq_init(&(*session_ptr)->ob_da_pq, outbound_item_compar, mem); + rv = nghttp2_pq_init(&(*session_ptr)->ob_da_pq, outbound_item_less, mem); if (rv != 0) { goto fail_ob_da_pq; } @@ -361,11 +349,6 @@ static int session_new(nghttp2_session **session_ptr, nghttp2_stream_roots_init(&(*session_ptr)->roots); - (*session_ptr)->next_seq = 0; - /* 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; (*session_ptr)->consumed_size = 0; @@ -454,10 +437,6 @@ fail_hd_inflater: fail_hd_deflater: nghttp2_pq_free(&(*session_ptr)->ob_da_pq); fail_ob_da_pq: - nghttp2_pq_free(&(*session_ptr)->ob_ss_pq); -fail_ob_ss_pq: - nghttp2_pq_free(&(*session_ptr)->ob_pq); -fail_ob_pq: nghttp2_mem_free(mem, *session_ptr); fail_session: return rv; @@ -563,6 +542,16 @@ static void ob_pq_free(nghttp2_pq *pq, nghttp2_mem *mem) { nghttp2_pq_free(pq); } +static void ob_q_free(nghttp2_outbound_queue *q, nghttp2_mem *mem) { + nghttp2_outbound_item *item, *next; + for (item = q->head; item;) { + next = item->qnext; + nghttp2_outbound_item_free(item, mem); + nghttp2_mem_free(mem, item); + item = next; + } +} + void nghttp2_session_del(nghttp2_session *session) { nghttp2_mem *mem; @@ -581,8 +570,9 @@ void nghttp2_session_del(nghttp2_session *session) { nghttp2_map_each_free(&session->streams, free_streams, session); nghttp2_map_free(&session->streams); - ob_pq_free(&session->ob_pq, mem); - ob_pq_free(&session->ob_ss_pq, mem); + ob_q_free(&session->ob_urgent, mem); + ob_q_free(&session->ob_reg, mem); + ob_q_free(&session->ob_syn, mem); ob_pq_free(&session->ob_da_pq, mem); active_outbound_item_reset(&session->aob, mem); session_inbound_frame_reset(session); @@ -690,8 +680,8 @@ 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++; - item->cycle = NGHTTP2_OB_EX_CYCLE; + item->cycle = 0; + item->qnext = NULL; item->queued = 0; memset(&item->aux_data, 0, sizeof(nghttp2_aux_data)); @@ -711,42 +701,21 @@ int nghttp2_session_add_item(nghttp2_session *session, if (frame->hd.type != NGHTTP2_DATA) { switch (frame->hd.type) { - case NGHTTP2_RST_STREAM: - if (stream) { - stream->state = NGHTTP2_STREAM_CLOSING; - } - - break; - case NGHTTP2_SETTINGS: - item->cycle = NGHTTP2_OB_SETTINGS_CYCLE; - - break; - case NGHTTP2_PING: - /* Ping has highest priority. */ - item->cycle = NGHTTP2_OB_PING_CYCLE; - - break; - default: - break; - } - - if (frame->hd.type == NGHTTP2_HEADERS) { + case NGHTTP2_HEADERS: /* We push request HEADERS and push response HEADERS to dedicated queue because their transmission is affected by SETTINGS_MAX_CONCURRENT_STREAMS */ /* TODO If 2 HEADERS are submitted for reserved stream, then - both of them are queued into ob_ss_pq, which is not + both of them are queued into ob_syn, which is not desirable. */ if (frame->headers.cat == NGHTTP2_HCAT_REQUEST) { - rv = nghttp2_pq_push(&session->ob_ss_pq, item); - - if (rv != 0) { - return rv; - } - + nghttp2_outbound_queue_push(&session->ob_syn, item); item->queued = 1; - } else if (stream && (stream->state == NGHTTP2_STREAM_RESERVED || - item->aux_data.headers.attach_stream)) { + break; + } + + if (stream && (stream->state == NGHTTP2_STREAM_RESERVED || + item->aux_data.headers.attach_stream)) { item->cycle = session->last_cycle; rv = nghttp2_stream_attach_item(stream, item, session); @@ -754,22 +723,25 @@ int nghttp2_session_add_item(nghttp2_session *session, if (rv != 0) { return rv; } - } else { - rv = nghttp2_pq_push(&session->ob_pq, item); - if (rv != 0) { - return rv; - } - - item->queued = 1; - } - } else { - rv = nghttp2_pq_push(&session->ob_pq, item); - - if (rv != 0) { - return rv; + break; } + nghttp2_outbound_queue_push(&session->ob_reg, item); + item->queued = 1; + break; + case NGHTTP2_SETTINGS: + case NGHTTP2_PING: + nghttp2_outbound_queue_push(&session->ob_urgent, item); + item->queued = 1; + break; + case NGHTTP2_RST_STREAM: + if (stream) { + stream->state = NGHTTP2_STREAM_CLOSING; + } + /* fall through */ + default: + nghttp2_outbound_queue_push(&session->ob_reg, item); item->queued = 1; } @@ -795,30 +767,6 @@ int nghttp2_session_add_item(nghttp2_session *session, return 0; } -typedef struct { - int32_t stream_id; - uint32_t error_code; -} nghttp2_rst_target; - -static int cancel_pending_request(void *pq_item, void *arg) { - nghttp2_outbound_item *item; - nghttp2_rst_target *t; - nghttp2_headers_aux_data *aux_data; - - item = pq_item; - t = arg; - aux_data = &item->aux_data.headers; - - if (item->frame.hd.stream_id != t->stream_id || aux_data->canceled) { - return 0; - } - - aux_data->error_code = t->error_code; - aux_data->canceled = 1; - - return 1; -} - int nghttp2_session_add_rst_stream(nghttp2_session *session, int32_t stream_id, uint32_t error_code) { int rv; @@ -826,7 +774,6 @@ int nghttp2_session_add_rst_stream(nghttp2_session *session, int32_t stream_id, nghttp2_frame *frame; nghttp2_stream *stream; nghttp2_mem *mem; - nghttp2_rst_target t = {stream_id, error_code}; mem = &session->mem; stream = nghttp2_session_get_stream(session, stream_id); @@ -834,21 +781,35 @@ int nghttp2_session_add_rst_stream(nghttp2_session *session, int32_t stream_id, return 0; } - /* Cancel pending request HEADERS in ob_ss_pq if this RST_STREAM + /* Cancel pending request HEADERS in ob_syn if this RST_STREAM refers to that stream. */ if (!session->server && nghttp2_session_is_my_stream_id(session, stream_id) && - nghttp2_pq_top(&session->ob_ss_pq)) { - nghttp2_outbound_item *top; + nghttp2_outbound_queue_top(&session->ob_syn)) { + nghttp2_headers_aux_data *aux_data; nghttp2_frame *headers_frame; - top = nghttp2_pq_top(&session->ob_ss_pq); - headers_frame = &top->frame; - + headers_frame = &nghttp2_outbound_queue_top(&session->ob_syn)->frame; assert(headers_frame->hd.type == NGHTTP2_HEADERS); if (headers_frame->hd.stream_id <= stream_id && (uint32_t)stream_id < session->next_stream_id) { - if (nghttp2_pq_each(&session->ob_ss_pq, cancel_pending_request, &t)) { + + for (item = session->ob_syn.head; item; item = item->qnext) { + aux_data = &item->aux_data.headers; + + if (item->frame.hd.stream_id < stream_id) { + continue; + } + + /* stream_id in ob_syn queue must be strictly increasing. If + we found larger ID, then we can break here. */ + if (item->frame.hd.stream_id > stream_id || aux_data->canceled) { + break; + } + + aux_data->error_code = error_code; + aux_data->canceled = 1; + return 0; } } @@ -2022,123 +1983,66 @@ static int session_prep_frame(nghttp2_session *session, } } -/* Used only for tests */ -nghttp2_outbound_item *nghttp2_session_get_ob_pq_top(nghttp2_session *session) { - return (nghttp2_outbound_item *)nghttp2_pq_top(&session->ob_pq); -} - nghttp2_outbound_item * nghttp2_session_get_next_ob_item(nghttp2_session *session) { - nghttp2_outbound_item *item, *headers_item; + if (nghttp2_outbound_queue_top(&session->ob_urgent)) { + return nghttp2_outbound_queue_top(&session->ob_urgent); + } - if (nghttp2_pq_empty(&session->ob_pq)) { - if (nghttp2_pq_empty(&session->ob_ss_pq)) { - if (session->remote_window_size == 0 || - nghttp2_pq_empty(&session->ob_da_pq)) { - return NULL; - } + if (nghttp2_outbound_queue_top(&session->ob_reg)) { + return nghttp2_outbound_queue_top(&session->ob_reg); + } - return nghttp2_pq_top(&session->ob_da_pq); + if (!session_is_outgoing_concurrent_streams_max(session)) { + if (nghttp2_outbound_queue_top(&session->ob_syn)) { + return nghttp2_outbound_queue_top(&session->ob_syn); } - - /* Return item only when concurrent connection limit is not - reached */ - if (session_is_outgoing_concurrent_streams_max(session)) { - if (session->remote_window_size == 0 || - nghttp2_pq_empty(&session->ob_da_pq)) { - return NULL; - } - - return nghttp2_pq_top(&session->ob_da_pq); - } - - return nghttp2_pq_top(&session->ob_ss_pq); } - if (nghttp2_pq_empty(&session->ob_ss_pq)) { - return nghttp2_pq_top(&session->ob_pq); + if (session->remote_window_size > 0 && + !nghttp2_pq_empty(&session->ob_da_pq)) { + return nghttp2_pq_top(&session->ob_da_pq); } - item = nghttp2_pq_top(&session->ob_pq); - headers_item = nghttp2_pq_top(&session->ob_ss_pq); - - if (session_is_outgoing_concurrent_streams_max(session) || - outbound_item_compar(item, headers_item) < 0) { - return item; - } - - return headers_item; + return NULL; } nghttp2_outbound_item * nghttp2_session_pop_next_ob_item(nghttp2_session *session) { - nghttp2_outbound_item *item, *headers_item; + nghttp2_outbound_item *item; - if (nghttp2_pq_empty(&session->ob_pq)) { - if (nghttp2_pq_empty(&session->ob_ss_pq)) { - if (session->remote_window_size == 0 || - nghttp2_pq_empty(&session->ob_da_pq)) { - return NULL; - } + item = nghttp2_outbound_queue_top(&session->ob_urgent); + if (item) { + nghttp2_outbound_queue_pop(&session->ob_urgent); + item->queued = 0; + return item; + } - item = nghttp2_pq_top(&session->ob_da_pq); - nghttp2_pq_pop(&session->ob_da_pq); + item = nghttp2_outbound_queue_top(&session->ob_reg); + if (item) { + nghttp2_outbound_queue_pop(&session->ob_reg); + item->queued = 0; + return item; + } + if (!session_is_outgoing_concurrent_streams_max(session)) { + item = nghttp2_outbound_queue_top(&session->ob_syn); + if (item) { + nghttp2_outbound_queue_pop(&session->ob_syn); item->queued = 0; - return item; } + } - /* Pop item only when concurrent connection limit is not - reached */ - if (session_is_outgoing_concurrent_streams_max(session)) { - if (session->remote_window_size == 0 || - nghttp2_pq_empty(&session->ob_da_pq)) { - return NULL; - } - - item = nghttp2_pq_top(&session->ob_da_pq); - nghttp2_pq_pop(&session->ob_da_pq); - - item->queued = 0; - - return item; - } - - item = nghttp2_pq_top(&session->ob_ss_pq); - nghttp2_pq_pop(&session->ob_ss_pq); - + if (session->remote_window_size > 0 && + !nghttp2_pq_empty(&session->ob_da_pq)) { + item = nghttp2_pq_top(&session->ob_da_pq); + nghttp2_pq_pop(&session->ob_da_pq); item->queued = 0; - return item; } - if (nghttp2_pq_empty(&session->ob_ss_pq)) { - item = nghttp2_pq_top(&session->ob_pq); - nghttp2_pq_pop(&session->ob_pq); - - item->queued = 0; - - return item; - } - - item = nghttp2_pq_top(&session->ob_pq); - headers_item = nghttp2_pq_top(&session->ob_ss_pq); - - if (session_is_outgoing_concurrent_streams_max(session) || - outbound_item_compar(item, headers_item) < 0) { - nghttp2_pq_pop(&session->ob_pq); - - item->queued = 0; - - return item; - } - - nghttp2_pq_pop(&session->ob_ss_pq); - - headers_item->queued = 0; - - return headers_item; + return NULL; } static int session_call_before_frame_send(nghttp2_session *session, @@ -2579,7 +2483,8 @@ static int session_after_frame_sent2(nghttp2_session *session) { waiting at the top of the queue, we continue to send this data. */ if (stream->dpri == NGHTTP2_STREAM_DPRI_TOP && - (next_item == NULL || outbound_item_compar(item, next_item) < 0)) { + (next_item == NULL || (next_item->frame.hd.type == NGHTTP2_DATA && + outbound_item_less(item, next_item)))) { size_t next_readmax; next_readmax = nghttp2_session_next_data_read(session, stream); @@ -6021,10 +5926,12 @@ int nghttp2_session_want_write(nghttp2_session *session) { * want to write them. */ - if (session->aob.item == NULL && nghttp2_pq_empty(&session->ob_pq) && + if (session->aob.item == NULL && + nghttp2_outbound_queue_top(&session->ob_urgent) == NULL && + nghttp2_outbound_queue_top(&session->ob_reg) == NULL && (nghttp2_pq_empty(&session->ob_da_pq) || session->remote_window_size == 0) && - (nghttp2_pq_empty(&session->ob_ss_pq) || + (nghttp2_outbound_queue_top(&session->ob_syn) == NULL || session_is_outgoing_concurrent_streams_max(session))) { return 0; } @@ -6425,8 +6332,9 @@ int nghttp2_session_resume_data(nghttp2_session *session, int32_t stream_id) { } size_t nghttp2_session_get_outbound_queue_size(nghttp2_session *session) { - return nghttp2_pq_size(&session->ob_pq) + - nghttp2_pq_size(&session->ob_ss_pq) + + return nghttp2_outbound_queue_size(&session->ob_urgent) + + nghttp2_outbound_queue_size(&session->ob_reg) + + nghttp2_outbound_queue_size(&session->ob_syn) + nghttp2_pq_size(&session->ob_da_pq); } diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 56709ec6..f4b99ab1 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -142,12 +142,15 @@ typedef enum { struct nghttp2_session { nghttp2_map /* */ streams; nghttp2_stream_roots roots; - /* Queue for outbound frames other than stream-creating HEADERS and - DATA */ - nghttp2_pq /* */ ob_pq; - /* Queue for outbound stream-creating HEADERS frame */ - nghttp2_pq /* */ ob_ss_pq; - /* QUeue for DATA frame */ + /* Queue for outbound urgent frames (PING and SETTINGS) */ + nghttp2_outbound_queue ob_urgent; + /* Queue for non-DATA frames */ + nghttp2_outbound_queue ob_reg; + /* Queue for outbound stream-creating HEADERS (request or push + response) frame, which are subject to + SETTINGS_MAX_CONCURRENT_STREAMS limit. */ + nghttp2_outbound_queue ob_syn; + /* Queue for DATA frame */ nghttp2_pq /* */ ob_da_pq; nghttp2_active_outbound_item aob; nghttp2_inbound_frame iframe; @@ -156,9 +159,6 @@ struct nghttp2_session { nghttp2_session_callbacks callbacks; /* Memory allocator */ nghttp2_mem mem; - /* Sequence number of outbound frame to maintain the order of - enqueue if priority is equal. */ - int64_t next_seq; /* Reset count of nghttp2_outbound_item's weight. We decrements weight each time DATA is sent to simulate resource sharing. We use priority queue and larger weight has the precedence. If @@ -707,12 +707,6 @@ int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_bufs *bufs, nghttp2_data_aux_data *aux_data, nghttp2_stream *stream); -/* - * Returns top of outbound frame queue. This function returns NULL if - * queue is empty. - */ -nghttp2_outbound_item *nghttp2_session_get_ob_pq_top(nghttp2_session *session); - /* * Pops and returns next item to send. If there is no such item, * returns NULL. This function takes into account max concurrent diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 6725eb0d..753932f6 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -101,22 +101,26 @@ static int stream_push_item(nghttp2_stream *stream, nghttp2_session *session) { return 0; } - /* Penalize item by delaying scheduling according to effective - weight. This will delay low priority stream, which is good. - OTOH, this may incur delay for high priority item. Will see. */ - item->cycle = - session->last_cycle + - NGHTTP2_DATA_PAYLOADLEN * NGHTTP2_MAX_WEIGHT / stream->effective_weight; - switch (item->frame.hd.type) { case NGHTTP2_DATA: + /* Penalize item by delaying scheduling according to effective + weight. This will delay low priority stream, which is good. + OTOH, this may incur delay for high priority item. Will + see. */ + item->cycle = + session->last_cycle + + NGHTTP2_DATA_PAYLOADLEN * NGHTTP2_MAX_WEIGHT / stream->effective_weight; + rv = nghttp2_pq_push(&session->ob_da_pq, item); + if (rv != 0) { + return rv; + } break; case NGHTTP2_HEADERS: if (stream->state == NGHTTP2_STREAM_RESERVED) { - rv = nghttp2_pq_push(&session->ob_ss_pq, item); + nghttp2_outbound_queue_push(&session->ob_syn, item); } else { - rv = nghttp2_pq_push(&session->ob_pq, item); + nghttp2_outbound_queue_push(&session->ob_reg, item); } break; default: @@ -124,10 +128,6 @@ static int stream_push_item(nghttp2_stream *stream, nghttp2_session *session) { assert(0); } - if (rv != 0) { - return rv; - } - item->queued = 1; return 0; diff --git a/tests/nghttp2_pq_test.c b/tests/nghttp2_pq_test.c index 1cef4d0e..a88f75b7 100644 --- a/tests/nghttp2_pq_test.c +++ b/tests/nghttp2_pq_test.c @@ -28,14 +28,14 @@ #include "nghttp2_pq.h" -static int pq_compar(const void *lhs, const void *rhs) { - return strcmp(lhs, rhs); +static int pq_less(const void *lhs, const void *rhs) { + return strcmp(lhs, rhs) < 0; } void test_nghttp2_pq(void) { int i; nghttp2_pq pq; - nghttp2_pq_init(&pq, pq_compar, nghttp2_mem_default()); + nghttp2_pq_init(&pq, pq_less, nghttp2_mem_default()); CU_ASSERT(nghttp2_pq_empty(&pq)); CU_ASSERT(0 == nghttp2_pq_size(&pq)); CU_ASSERT(0 == nghttp2_pq_push(&pq, (void *)"foo")); @@ -80,10 +80,10 @@ typedef struct { int val; } node; -static int node_compar(const void *lhs, const void *rhs) { +static int node_less(const void *lhs, const void *rhs) { node *ln = (node *)lhs; node *rn = (node *)rhs; - return ln->key - rn->key; + return ln->key < rn->key; } static int node_update(void *item, void *arg _U_) { @@ -103,7 +103,7 @@ void test_nghttp2_pq_update(void) { node *nd; int ans[] = {-8, -6, -4, -2, 0, 1, 3, 5, 7, 9}; - nghttp2_pq_init(&pq, node_compar, nghttp2_mem_default()); + nghttp2_pq_init(&pq, node_less, nghttp2_mem_default()); for (i = 0; i < (int)(sizeof(nodes) / sizeof(nodes[0])); ++i) { nodes[i].key = i; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 0a465d4f..19722b1c 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1641,7 +1641,7 @@ void test_nghttp2_session_add_frame(void) { session->next_stream_id += 2; CU_ASSERT(0 == nghttp2_session_add_item(session, item)); - CU_ASSERT(0 == nghttp2_pq_empty(&session->ob_ss_pq)); + CU_ASSERT(NULL != nghttp2_outbound_queue_top(&session->ob_syn)); CU_ASSERT(0 == nghttp2_session_send(session)); CU_ASSERT(NGHTTP2_HEADERS == acc.buf[3]); CU_ASSERT((NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY) == acc.buf[4]); @@ -2484,16 +2484,16 @@ void test_nghttp2_session_on_ping_received(void) { CU_ASSERT(0 == nghttp2_session_on_ping_received(session, &frame)); CU_ASSERT(1 == user_data.frame_recv_cb_called); - /* Since this ping frame has PONG flag set, no further action is + /* Since this ping frame has ACK flag set, no further action is performed. */ - CU_ASSERT(NULL == nghttp2_session_get_ob_pq_top(session)); + CU_ASSERT(NULL == nghttp2_outbound_queue_top(&session->ob_urgent)); /* Clear the flag, and receive it again */ frame.hd.flags = NGHTTP2_FLAG_NONE; CU_ASSERT(0 == nghttp2_session_on_ping_received(session, &frame)); CU_ASSERT(2 == user_data.frame_recv_cb_called); - top = nghttp2_session_get_ob_pq_top(session); + top = nghttp2_outbound_queue_top(&session->ob_urgent); CU_ASSERT(NGHTTP2_PING == top->frame.hd.type); CU_ASSERT(NGHTTP2_FLAG_ACK == top->frame.hd.flags); CU_ASSERT(memcmp(opaque_data, top->frame.ping.opaque_data, 8) == 0); @@ -2664,7 +2664,7 @@ void test_nghttp2_session_on_data_received(void) { frame.hd.stream_id = 4; CU_ASSERT(0 == nghttp2_session_on_data_received(session, &frame)); - CU_ASSERT(NULL == nghttp2_session_get_ob_pq_top(session)); + CU_ASSERT(NULL == nghttp2_outbound_queue_top(&session->ob_reg)); /* Check INVALID_STREAM case: DATA frame with stream ID which does not exist. */ @@ -2672,7 +2672,7 @@ void test_nghttp2_session_on_data_received(void) { frame.hd.stream_id = 6; CU_ASSERT(0 == nghttp2_session_on_data_received(session, &frame)); - top = nghttp2_session_get_ob_pq_top(session); + top = nghttp2_outbound_queue_top(&session->ob_reg); /* DATA against nonexistent stream is just ignored for now */ CU_ASSERT(top == NULL); /* CU_ASSERT(NGHTTP2_RST_STREAM == top->frame.hd.type); */ @@ -4513,7 +4513,7 @@ void test_nghttp2_session_pop_next_ob_item(void) { CU_ASSERT(0 == nghttp2_submit_headers(session, NGHTTP2_FLAG_END_STREAM, 2, NULL, NULL, 0, NULL)); CU_ASSERT(NULL == nghttp2_session_pop_next_ob_item(session)); - CU_ASSERT(1 == nghttp2_pq_size(&session->ob_ss_pq)); + CU_ASSERT(1 == nghttp2_outbound_queue_size(&session->ob_syn)); nghttp2_session_del(session); } @@ -4559,7 +4559,7 @@ void test_nghttp2_session_max_concurrent_streams(void) { CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == nghttp2_session_on_request_headers_received(session, &frame)); - item = nghttp2_session_get_ob_pq_top(session); + item = nghttp2_outbound_queue_top(&session->ob_reg); CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type); CU_ASSERT(NGHTTP2_REFUSED_STREAM == item->frame.rst_stream.error_code); @@ -4572,7 +4572,7 @@ void test_nghttp2_session_max_concurrent_streams(void) { CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == nghttp2_session_on_request_headers_received(session, &frame)); - item = nghttp2_session_get_ob_pq_top(session); + item = nghttp2_outbound_queue_top(&session->ob_reg); CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type); CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == item->frame.goaway.error_code); @@ -6937,7 +6937,7 @@ void test_nghttp2_session_cancel_reserved_remote(void) { /* No RST_STREAM or GOAWAY is generated since stream should be in NGHTTP2_STREAM_CLOSING and push response should be ignored. */ - CU_ASSERT(0 == nghttp2_pq_size(&session->ob_pq)); + CU_ASSERT(0 == nghttp2_outbound_queue_size(&session->ob_reg)); /* Check that we can receive push response HEADERS while RST_STREAM is just queued. */ @@ -6960,7 +6960,7 @@ void test_nghttp2_session_cancel_reserved_remote(void) { CU_ASSERT(nghttp2_buf_len(&bufs.head->buf) == rv); - CU_ASSERT(1 == nghttp2_pq_size(&session->ob_pq)); + CU_ASSERT(1 == nghttp2_outbound_queue_size(&session->ob_reg)); nghttp2_frame_headers_free(&frame.headers, mem); From e38dd37667dabbe65e0ce41b051d350498af5d06 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 25 Apr 2015 01:00:02 +0900 Subject: [PATCH 03/17] Update doc --- lib/nghttp2_session.h | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index f4b99ab1..25f67895 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -159,19 +159,8 @@ struct nghttp2_session { nghttp2_session_callbacks callbacks; /* Memory allocator */ nghttp2_mem mem; - /* Reset count of nghttp2_outbound_item's weight. We decrements - weight each time DATA is sent to simulate resource sharing. We - use priority queue and larger weight has the precedence. If - weight is reached to lowest weight, it resets to its initial - weight. If this happens, other items which have the lower weight - currently but same initial weight cannot send DATA until item - having large weight is decreased. To avoid this, we use this - cycle variable. Initally, this is set to 1. If weight gets - lowest weight, and if item's cycle == last_cycle, we increments - last_cycle and assigns it to item's cycle. Otherwise, just - assign last_cycle. In priority queue comparator, we first - compare items' cycle value. Lower cycle value has the - precedence. */ + /* Base value when we schedule next DATA frame write. This is + updated when one frame was written. */ uint64_t last_cycle; void *user_data; /* Points to the latest closed stream. NULL if there is no closed From c41f4139783c58764470345558a6e63749148ee6 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 25 Apr 2015 02:23:01 +0900 Subject: [PATCH 04/17] Fix compile error with --enable-werror --- lib/nghttp2_outbound_item.c | 9 +++++++++ lib/nghttp2_outbound_item.h | 7 +++++++ lib/nghttp2_session.c | 19 +++++-------------- lib/nghttp2_session.h | 8 -------- lib/nghttp2_submit.c | 8 ++++---- tests/nghttp2_session_test.c | 18 +++++++++--------- 6 files changed, 34 insertions(+), 35 deletions(-) diff --git a/lib/nghttp2_outbound_item.c b/lib/nghttp2_outbound_item.c index 263dd55d..c4ecab90 100644 --- a/lib/nghttp2_outbound_item.c +++ b/lib/nghttp2_outbound_item.c @@ -25,6 +25,15 @@ #include "nghttp2_outbound_item.h" #include +#include + +void nghttp2_outbound_item_init(nghttp2_outbound_item *item) { + item->cycle = 0; + item->qnext = NULL; + item->queued = 0; + + memset(&item->aux_data, 0, sizeof(nghttp2_aux_data)); +} void nghttp2_outbound_item_free(nghttp2_outbound_item *item, nghttp2_mem *mem) { nghttp2_frame *frame; diff --git a/lib/nghttp2_outbound_item.h b/lib/nghttp2_outbound_item.h index 74164499..110e0577 100644 --- a/lib/nghttp2_outbound_item.h +++ b/lib/nghttp2_outbound_item.h @@ -116,6 +116,13 @@ struct nghttp2_outbound_item { uint8_t queued; }; +/* + * Initializes |item|. No memory allocation is done in this function. + * Don't call nghttp2_outbound_item_free() until frame member is + * initialized. + */ +void nghttp2_outbound_item_init(nghttp2_outbound_item *item); + /* * Deallocates resource for |item|. If |item| is NULL, this function * does nothing. diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index f1e5a6bd..54a3fe6d 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -678,15 +678,6 @@ nghttp2_session_reprioritize_stream(nghttp2_session *session, return 0; } -void nghttp2_session_outbound_item_init(nghttp2_session *session, - nghttp2_outbound_item *item) { - item->cycle = 0; - item->qnext = NULL; - item->queued = 0; - - memset(&item->aux_data, 0, sizeof(nghttp2_aux_data)); -} - int nghttp2_session_add_item(nghttp2_session *session, nghttp2_outbound_item *item) { /* TODO Return error if stream is not found for the frame requiring @@ -820,7 +811,7 @@ int nghttp2_session_add_rst_stream(nghttp2_session *session, int32_t stream_id, return NGHTTP2_ERR_NOMEM; } - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -5955,7 +5946,7 @@ int nghttp2_session_add_ping(nghttp2_session *session, uint8_t flags, return NGHTTP2_ERR_NOMEM; } - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -6004,7 +5995,7 @@ int nghttp2_session_add_goaway(nghttp2_session *session, int32_t last_stream_id, return NGHTTP2_ERR_NOMEM; } - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -6041,7 +6032,7 @@ int nghttp2_session_add_window_update(nghttp2_session *session, uint8_t flags, return NGHTTP2_ERR_NOMEM; } - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -6112,7 +6103,7 @@ int nghttp2_session_add_settings(nghttp2_session *session, uint8_t flags, session->inflight_niv = niv; } - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 25f67895..fb08c652 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -280,14 +280,6 @@ typedef struct { int nghttp2_session_is_my_stream_id(nghttp2_session *session, int32_t stream_id); -/* - * Initializes |item|. No memory allocation is done in this function. - * Don't call nghttp2_outbound_item_free() until frame member is - * initialized. - */ -void nghttp2_session_outbound_item_init(nghttp2_session *session, - nghttp2_outbound_item *item); - /* * Adds |item| to the outbound queue in |session|. When this function * succeeds, it takes ownership of |item|. So caller must not free it diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index bde78a20..54e8be00 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -62,7 +62,7 @@ static int32_t submit_headers_shared(nghttp2_session *session, uint8_t flags, goto fail; } - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); if (data_prd != NULL && data_prd->read_callback != NULL) { item->aux_data.headers.data_prd = *data_prd; @@ -212,7 +212,7 @@ int nghttp2_submit_priority(nghttp2_session *session, uint8_t flags _U_, return NGHTTP2_ERR_NOMEM; } - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -299,7 +299,7 @@ int32_t nghttp2_submit_push_promise(nghttp2_session *session, uint8_t flags _U_, return NGHTTP2_ERR_NOMEM; } - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); item->aux_data.headers.stream_user_data = promised_stream_user_data; @@ -453,7 +453,7 @@ int nghttp2_submit_data(nghttp2_session *session, uint8_t flags, return NGHTTP2_ERR_NOMEM; } - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; aux_data = &item->aux_data.data; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 19722b1c..42ca1e64 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1627,7 +1627,7 @@ void test_nghttp2_session_add_frame(void) { item = mem->malloc(sizeof(nghttp2_outbound_item), NULL); - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -2698,7 +2698,7 @@ void test_nghttp2_session_send_headers_start_stream(void) { item = mem->malloc(sizeof(nghttp2_outbound_item), NULL); - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -2734,7 +2734,7 @@ void test_nghttp2_session_send_headers_reply(void) { item = mem->malloc(sizeof(nghttp2_outbound_item), NULL); - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -2784,7 +2784,7 @@ void test_nghttp2_session_send_headers_frame_size_error(void) { item = mem->malloc(sizeof(nghttp2_outbound_item), NULL); - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -2829,7 +2829,7 @@ void test_nghttp2_session_send_headers_push_reply(void) { item = mem->malloc(sizeof(nghttp2_outbound_item), NULL); - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -2863,7 +2863,7 @@ void test_nghttp2_session_send_rst_stream(void) { item = mem->malloc(sizeof(nghttp2_outbound_item), NULL); - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -2897,7 +2897,7 @@ void test_nghttp2_session_send_push_promise(void) { item = mem->malloc(sizeof(nghttp2_outbound_item), NULL); - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -2925,7 +2925,7 @@ void test_nghttp2_session_send_push_promise(void) { item = mem->malloc(sizeof(nghttp2_outbound_item), NULL); - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; @@ -2948,7 +2948,7 @@ void test_nghttp2_session_send_push_promise(void) { &pri_spec_default, NGHTTP2_STREAM_OPENING, NULL); item = mem->malloc(sizeof(nghttp2_outbound_item), NULL); - nghttp2_session_outbound_item_init(session, item); + nghttp2_outbound_item_init(item); frame = &item->frame; From 02468b1ca14aef0ee85ddef99be9af757e20a9e7 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 26 Apr 2015 18:43:24 +0900 Subject: [PATCH 05/17] Allocate field name and value in the same buffer if indname to dynamic table --- lib/nghttp2_buf.c | 18 +++++++++++ lib/nghttp2_buf.h | 11 +++++++ lib/nghttp2_hd.c | 81 +++++++++++++++++++++++++++++++++++------------ 3 files changed, 90 insertions(+), 20 deletions(-) diff --git a/lib/nghttp2_buf.c b/lib/nghttp2_buf.c index 415c414d..0941f78b 100644 --- a/lib/nghttp2_buf.c +++ b/lib/nghttp2_buf.c @@ -432,6 +432,24 @@ ssize_t nghttp2_bufs_remove(nghttp2_bufs *bufs, uint8_t **out) { return (ssize_t)len; } +size_t nghttp2_bufs_remove_copy(nghttp2_bufs *bufs, uint8_t *out) { + size_t len; + nghttp2_buf_chain *chain; + nghttp2_buf *buf; + nghttp2_buf resbuf; + + len = nghttp2_bufs_len(bufs); + + nghttp2_buf_wrap_init(&resbuf, out, len); + + for (chain = bufs->head; chain; chain = chain->next) { + buf = &chain->buf; + resbuf.last = nghttp2_cpymem(resbuf.last, buf->pos, nghttp2_buf_len(buf)); + } + + return len; +} + void nghttp2_bufs_reset(nghttp2_bufs *bufs) { nghttp2_buf_chain *chain, *ci; size_t k; diff --git a/lib/nghttp2_buf.h b/lib/nghttp2_buf.h index 9b6d149c..9e986c69 100644 --- a/lib/nghttp2_buf.h +++ b/lib/nghttp2_buf.h @@ -324,6 +324,17 @@ int nghttp2_bufs_orb_hold(nghttp2_bufs *bufs, uint8_t b); */ ssize_t nghttp2_bufs_remove(nghttp2_bufs *bufs, uint8_t **out); +/* + * Copies all data stored in |bufs| to |out|. This function assumes + * that the buffer space pointed by |out| has at least + * nghttp2_bufs(bufs) bytes. + * + * The contents of |bufs| is left unchanged. + * + * This function returns the length of copied data. + */ +size_t nghttp2_bufs_remove_copy(nghttp2_bufs *bufs, uint8_t *out); + /* * Resets |bufs| and makes the buffers empty. */ diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index 3a97ac71..31b563dd 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -1730,6 +1730,42 @@ static int hd_inflate_remove_bufs(nghttp2_hd_inflater *inflater, nghttp2_nv *nv, return 0; } +static int hd_inflate_remove_bufs_with_name(nghttp2_hd_inflater *inflater, + nghttp2_nv *nv, + nghttp2_hd_entry *ent_name) { + size_t rv; + size_t buflen; + uint8_t *buf; + nghttp2_mem *mem; + + mem = inflater->ctx.mem; + + /* Allocate buffer including name in ent_name, plus terminating + NULL. */ + buflen = ent_name->nv.namelen + 1 + nghttp2_bufs_len(&inflater->nvbufs); + + buf = nghttp2_mem_malloc(mem, buflen); + if (buf == NULL) { + return NGHTTP2_ERR_NOMEM; + } + + /* Copy including terminal NULL */ + memcpy(buf, ent_name->nv.name, ent_name->nv.namelen + 1); + rv = nghttp2_bufs_remove_copy(&inflater->nvbufs, + buf + ent_name->nv.namelen + 1); + assert(ent_name->nv.namelen + 1 + rv == buflen); + + nghttp2_bufs_reset(&inflater->nvbufs); + + nv->name = buf; + nv->namelen = ent_name->nv.namelen; + + nv->value = buf + nv->namelen + 1; + nv->valuelen = buflen - nv->namelen - 2; + + return 0; +} + /* * Finalize literal header representation - new name- reception. If * header is emitted, |*nv_out| is filled with that value and 0 is @@ -1813,11 +1849,6 @@ static int hd_inflate_commit_indname(nghttp2_hd_inflater *inflater, mem = inflater->ctx.mem; - rv = hd_inflate_remove_bufs(inflater, &nv, 1 /* value only */); - if (rv != 0) { - return NGHTTP2_ERR_NOMEM; - } - if (inflater->no_index) { nv.flags = NGHTTP2_NV_FLAG_NO_INDEX; } else { @@ -1826,31 +1857,33 @@ static int hd_inflate_commit_indname(nghttp2_hd_inflater *inflater, ent_name = nghttp2_hd_table_get(&inflater->ctx, inflater->index); - nv.name = ent_name->nv.name; - nv.namelen = ent_name->nv.namelen; - if (inflater->index_required) { nghttp2_hd_entry *new_ent; uint8_t ent_flags; - int static_name; - ent_flags = NGHTTP2_HD_FLAG_VALUE_ALLOC | NGHTTP2_HD_FLAG_VALUE_GIFT; - static_name = inflater->index < NGHTTP2_STATIC_TABLE_LENGTH; + if (inflater->index < NGHTTP2_STATIC_TABLE_LENGTH) { + /* We don't copy name in static table */ + rv = hd_inflate_remove_bufs(inflater, &nv, 1 /* value only */); + if (rv != 0) { + return NGHTTP2_ERR_NOMEM; + } + nv.name = ent_name->nv.name; + nv.namelen = ent_name->nv.namelen; - if (!static_name) { - ent_flags |= NGHTTP2_HD_FLAG_NAME_ALLOC; - /* For entry in static table, we must not touch ref, because it - is shared by threads */ - ++ent_name->ref; + ent_flags = NGHTTP2_HD_FLAG_VALUE_ALLOC | NGHTTP2_HD_FLAG_VALUE_GIFT; + } else { + rv = hd_inflate_remove_bufs_with_name(inflater, &nv, ent_name); + if (rv != 0) { + return NGHTTP2_ERR_NOMEM; + } + /* nv->name and nv->value are in the same buffer. */ + ent_flags = NGHTTP2_HD_FLAG_NAME_ALLOC | NGHTTP2_HD_FLAG_NAME_GIFT; } new_ent = add_hd_table_incremental(&inflater->ctx, &nv, ent_name->token, ent_flags); - if (!static_name && --ent_name->ref == 0) { - nghttp2_hd_entry_free(ent_name, mem); - nghttp2_mem_free(mem, ent_name); - } + /* At this point, ent_name might be deleted. */ if (new_ent) { emit_indexed_header(nv_out, token_out, new_ent); @@ -1865,6 +1898,14 @@ static int hd_inflate_commit_indname(nghttp2_hd_inflater *inflater, return NGHTTP2_ERR_NOMEM; } + rv = hd_inflate_remove_bufs(inflater, &nv, 1 /* value only */); + if (rv != 0) { + return NGHTTP2_ERR_NOMEM; + } + + nv.name = ent_name->nv.name; + nv.namelen = ent_name->nv.namelen; + emit_literal_header(nv_out, token_out, &nv); if (nv.value != inflater->nvbufs.head->buf.pos) { From bbdff112a35a78570423cd19173d2c21e2760282 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 26 Apr 2015 19:37:57 +0900 Subject: [PATCH 06/17] Make huffman decoding slightly faster using evil looking macro --- lib/nghttp2_hd_huffman.c | 59 +++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/lib/nghttp2_hd_huffman.c b/lib/nghttp2_hd_huffman.c index 6c4b2b64..c8946ab0 100644 --- a/lib/nghttp2_hd_huffman.c +++ b/lib/nghttp2_hd_huffman.c @@ -168,6 +168,23 @@ void nghttp2_hd_huff_decode_context_init(nghttp2_hd_huff_decode_context *ctx) { ctx->accept = 1; } +/* Use macro to make the code simpler..., but error case is tricky. + We spent most of the CPU in decoding, so we are doing this + thing. */ +#define hd_huff_decode_sym_emit(bufs, sym, avail) \ + do { \ + if ((avail)) { \ + nghttp2_bufs_fast_addb((bufs), (sym)); \ + --(avail); \ + } else { \ + rv = nghttp2_bufs_addb((bufs), (sym)); \ + if (rv != 0) { \ + return rv; \ + } \ + (avail) = nghttp2_bufs_cur_avail((bufs)); \ + } \ + } while (0) + ssize_t nghttp2_hd_huff_decode(nghttp2_hd_huff_decode_context *ctx, nghttp2_bufs *bufs, const uint8_t *src, size_t srclen, int final) { @@ -180,30 +197,28 @@ ssize_t nghttp2_hd_huff_decode(nghttp2_hd_huff_decode_context *ctx, /* We use the decoding algorithm described in http://graphics.ics.uci.edu/pub/Prefix.pdf */ for (i = 0; i < srclen; ++i) { - uint8_t in = src[i] >> 4; - for (j = 0; j < 2; ++j) { - const nghttp2_huff_decode *t; + const nghttp2_huff_decode *t; - t = &huff_decode_table[ctx->state][in]; - if (t->flags & NGHTTP2_HUFF_FAIL) { - return NGHTTP2_ERR_HEADER_COMP; - } - if (t->flags & NGHTTP2_HUFF_SYM) { - if (avail) { - nghttp2_bufs_fast_addb(bufs, t->sym); - --avail; - } else { - rv = nghttp2_bufs_addb(bufs, t->sym); - if (rv != 0) { - return rv; - } - avail = nghttp2_bufs_cur_avail(bufs); - } - } - ctx->state = t->state; - ctx->accept = (t->flags & NGHTTP2_HUFF_ACCEPTED) != 0; - in = src[i] & 0xf; + t = &huff_decode_table[ctx->state][src[i] >> 4]; + if (t->flags & NGHTTP2_HUFF_FAIL) { + return NGHTTP2_ERR_HEADER_COMP; } + if (t->flags & NGHTTP2_HUFF_SYM) { + /* this is macro, and may return from this function on error */ + hd_huff_decode_sym_emit(bufs, t->sym, avail); + } + + t = &huff_decode_table[t->state][src[i] & 0xf]; + if (t->flags & NGHTTP2_HUFF_FAIL) { + return NGHTTP2_ERR_HEADER_COMP; + } + if (t->flags & NGHTTP2_HUFF_SYM) { + /* this is macro, and may return from this function on error */ + hd_huff_decode_sym_emit(bufs, t->sym, avail); + } + + ctx->state = t->state; + ctx->accept = (t->flags & NGHTTP2_HUFF_ACCEPTED) != 0; } if (final && !ctx->accept) { return NGHTTP2_ERR_HEADER_COMP; From c8f67788e01db95e0f95e5250ee0811d93072f42 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 26 Apr 2015 19:47:14 +0900 Subject: [PATCH 07/17] Remove unused local variable --- lib/nghttp2_hd_huffman.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/nghttp2_hd_huffman.c b/lib/nghttp2_hd_huffman.c index c8946ab0..4df1cd04 100644 --- a/lib/nghttp2_hd_huffman.c +++ b/lib/nghttp2_hd_huffman.c @@ -188,7 +188,7 @@ void nghttp2_hd_huff_decode_context_init(nghttp2_hd_huff_decode_context *ctx) { ssize_t nghttp2_hd_huff_decode(nghttp2_hd_huff_decode_context *ctx, nghttp2_bufs *bufs, const uint8_t *src, size_t srclen, int final) { - size_t i, j; + size_t i; int rv; size_t avail; From 42b2430fe17b94845cd430a2713365e20740add2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 26 Apr 2015 22:32:54 +0900 Subject: [PATCH 08/17] Remove unnecessary assignment to item->cycle --- lib/nghttp2_session.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 54a3fe6d..5a0fc76c 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -707,8 +707,6 @@ int nghttp2_session_add_item(nghttp2_session *session, if (stream && (stream->state == NGHTTP2_STREAM_RESERVED || item->aux_data.headers.attach_stream)) { - item->cycle = session->last_cycle; - rv = nghttp2_stream_attach_item(stream, item, session); if (rv != 0) { @@ -747,8 +745,6 @@ int nghttp2_session_add_item(nghttp2_session *session, return NGHTTP2_ERR_DATA_EXIST; } - item->cycle = session->last_cycle; - rv = nghttp2_stream_attach_item(stream, item, session); if (rv != 0) { From b41835f19b5c2099e4579f1dec5ecc2feb3f1e76 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 27 Apr 2015 21:23:01 +0900 Subject: [PATCH 09/17] h2load: Effectively disable flow control by setting large window size Previously h2load used default flow control window as described in HTTP/2 and SPDY specification. The window size is 64KiB, which is a bit small, and cannot utilize full server performance when response size is not too small. Basically, we do this kind of benchmarking test to measure server's throughput, and optimal performance. Smaller window certainly degrades performance even in local testing because server is so fast that it has to wait for WINDOW_UPDATE from h2load. To make default behaviour suitable for peak performance test, we decided to disable flow control in h2load by setting large enough window size. Most users used h2load without -w or -W options, so they were implicitly throttled by flow control and the result was affected by that negatively. Now flow control is disabled by default, the result may improve depending on the implementations. --- doc/h2load.h2r | 9 +++++++++ src/h2load.cc | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/doc/h2load.h2r b/doc/h2load.h2r index ccefdd1d..10cc5d56 100644 --- a/doc/h2load.h2r +++ b/doc/h2load.h2r @@ -49,6 +49,15 @@ time for request The fraction of the number of requests within standard deviation range (mean +/- sd) against total number of successful requests. +FLOW CONTROL +------------ + +h2load sets large flow control window by default, and effectively +disables flow control to avoid under utilization of server +performance. To set smaller flow control window, use :option:`-w` and +:option:`-W` options. For example, use ``-w16 -W16`` to set default +window size described in HTTP/2 and SPDY protocol specification. + SEE ALSO -------- diff --git a/src/h2load.cc b/src/h2load.cc index 5daff7b0..37517d3e 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -69,7 +69,7 @@ namespace h2load { Config::Config() : data_length(-1), addrs(nullptr), nreqs(1), nclients(1), nthreads(1), - max_concurrent_streams(-1), window_bits(16), connection_window_bits(16), + max_concurrent_streams(-1), window_bits(30), connection_window_bits(30), no_tls_proto(PROTO_HTTP2), data_fd(-1), port(0), default_port(0), verbose(false) {} @@ -974,11 +974,13 @@ Options: -w, --window-bits= Sets the stream level initial window size to (2**)-1. For SPDY, 2** is used instead. + Default: )" << config.window_bits << R"( -W, --connection-window-bits= Sets the connection level initial window size to (2**)-1. For SPDY, if is strictly less than 16, this option is ignored. Otherwise 2** is used for SPDY. + Default: )" << config.connection_window_bits << R"( -H, --header=
Add/Override a header to the requests. -p, --no-tls-proto= From 2436acbd23f9adca7bf60133f4f19431ba154cc6 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 27 Apr 2015 22:36:03 +0900 Subject: [PATCH 10/17] Update doc about h2load flow control --- doc/sources/h2load-howto.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/sources/h2load-howto.rst b/doc/sources/h2load-howto.rst index 698c3e7f..9660e07d 100644 --- a/doc/sources/h2load-howto.rst +++ b/doc/sources/h2load-howto.rst @@ -50,8 +50,9 @@ Flow Control ------------ HTTP/2 and SPDY/3 or later employ flow control and it may affect -benchmarking results. To adjust receiver flow control window size, -there is following options: +benchmarking results. By default, h2load uses large enough flow +control window, which effectively disables flow control. To adjust +receiver flow control window size, there are following options: ``-w`` Sets the stream level initial window size to From df707df21ba7f096f1636116134d420d82d61c23 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 27 Apr 2015 22:36:34 +0900 Subject: [PATCH 11/17] Update man pages --- doc/h2load.1 | 13 ++++++++++++- doc/h2load.1.rst | 13 +++++++++++++ doc/nghttp.1 | 2 +- doc/nghttpd.1 | 2 +- doc/nghttpx.1 | 2 +- 5 files changed, 28 insertions(+), 4 deletions(-) diff --git a/doc/h2load.1 b/doc/h2load.1 index 740c3ff1..4f821e33 100644 --- a/doc/h2load.1 +++ b/doc/h2load.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "H2LOAD" "1" "April 19, 2015" "0.7.12" "nghttp2" +.TH "H2LOAD" "1" "April 27, 2015" "0.7.13-DEV" "nghttp2" .SH NAME h2load \- HTTP/2 benchmarking tool . @@ -93,6 +93,8 @@ Default: \fBauto\fP .B \-w, \-\-window\-bits= Sets the stream level initial window size to (2**)\-1. For SPDY, 2** is used instead. +.sp +Default: \fB30\fP .UNINDENT .INDENT 0.0 .TP @@ -101,6 +103,8 @@ Sets the connection level initial window size to (2**)\-1. For SPDY, if is strictly less than 16, this option is ignored. Otherwise 2** is used for SPDY. +.sp +Default: \fB30\fP .UNINDENT .INDENT 0.0 .TP @@ -208,6 +212,13 @@ The fraction of the number of requests within standard deviation range (mean +/\- sd) against total number of successful requests. .UNINDENT .UNINDENT +.SH FLOW CONTROL +.sp +h2load sets large flow control window by default, and effectively +disables flow control to avoid under utilization of server +performance. To set smaller flow control window, use \fI\%\-w\fP and +\fI\%\-W\fP options. For example, use \fB\-w16 \-W16\fP to set default +window size described in HTTP/2 and SPDY protocol specification. .SH SEE ALSO .sp \fInghttp(1)\fP, \fInghttpd(1)\fP, \fInghttpx(1)\fP diff --git a/doc/h2load.1.rst b/doc/h2load.1.rst index ff1115c8..fc70f438 100644 --- a/doc/h2load.1.rst +++ b/doc/h2load.1.rst @@ -67,6 +67,8 @@ OPTIONS Sets the stream level initial window size to (2\*\*)-1. For SPDY, 2** is used instead. + Default: ``30`` + .. option:: -W, --connection-window-bits= Sets the connection level initial window size to @@ -74,6 +76,8 @@ OPTIONS this option is ignored. Otherwise 2\*\* is used for SPDY. + Default: ``30`` + .. option:: -H, --header=
Add/Override a header to the requests. @@ -154,6 +158,15 @@ time for request The fraction of the number of requests within standard deviation range (mean +/- sd) against total number of successful requests. +FLOW CONTROL +------------ + +h2load sets large flow control window by default, and effectively +disables flow control to avoid under utilization of server +performance. To set smaller flow control window, use :option:`-w` and +:option:`-W` options. For example, use ``-w16 -W16`` to set default +window size described in HTTP/2 and SPDY protocol specification. + SEE ALSO -------- diff --git a/doc/nghttp.1 b/doc/nghttp.1 index ad6d288a..b9ea4d74 100644 --- a/doc/nghttp.1 +++ b/doc/nghttp.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTP" "1" "April 19, 2015" "0.7.12" "nghttp2" +.TH "NGHTTP" "1" "April 27, 2015" "0.7.13-DEV" "nghttp2" .SH NAME nghttp \- HTTP/2 experimental client . diff --git a/doc/nghttpd.1 b/doc/nghttpd.1 index ba26ef6c..2eb1fe3f 100644 --- a/doc/nghttpd.1 +++ b/doc/nghttpd.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPD" "1" "April 19, 2015" "0.7.12" "nghttp2" +.TH "NGHTTPD" "1" "April 27, 2015" "0.7.13-DEV" "nghttp2" .SH NAME nghttpd \- HTTP/2 experimental server . diff --git a/doc/nghttpx.1 b/doc/nghttpx.1 index c28d83a3..0cba2e19 100644 --- a/doc/nghttpx.1 +++ b/doc/nghttpx.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPX" "1" "April 19, 2015" "0.7.12" "nghttp2" +.TH "NGHTTPX" "1" "April 27, 2015" "0.7.13-DEV" "nghttp2" .SH NAME nghttpx \- HTTP/2 experimental proxy . From ee354ee6c86149d47f93d45063300ada60f4d9cf Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 27 Apr 2015 23:06:14 +0900 Subject: [PATCH 12/17] Bump up version number to 0.7.13, LT revision to 13:2:8 --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 8cb65717..93bd2062 100644 --- a/configure.ac +++ b/configure.ac @@ -25,13 +25,13 @@ dnl Do not change user variables! dnl http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html AC_PREREQ(2.61) -AC_INIT([nghttp2], [0.7.13-DEV], [t-tujikawa@users.sourceforge.net]) +AC_INIT([nghttp2], [0.7.13], [t-tujikawa@users.sourceforge.net]) LT_PREREQ([2.2.6]) LT_INIT() dnl See versioning rule: dnl http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html AC_SUBST(LT_CURRENT, 13) -AC_SUBST(LT_REVISION, 1) +AC_SUBST(LT_REVISION, 2) AC_SUBST(LT_AGE, 8) major=`echo $PACKAGE_VERSION |cut -d. -f1 | sed -e "s/[^0-9]//g"` From dbc613e0d0ff9175b1399f66b9b665f8931d5b51 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 27 Apr 2015 23:09:00 +0900 Subject: [PATCH 13/17] Update man pages --- doc/h2load.1 | 2 +- doc/nghttp.1 | 2 +- doc/nghttpd.1 | 2 +- doc/nghttpx.1 | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/h2load.1 b/doc/h2load.1 index 4f821e33..e2d9b3de 100644 --- a/doc/h2load.1 +++ b/doc/h2load.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "H2LOAD" "1" "April 27, 2015" "0.7.13-DEV" "nghttp2" +.TH "H2LOAD" "1" "April 27, 2015" "0.7.13" "nghttp2" .SH NAME h2load \- HTTP/2 benchmarking tool . diff --git a/doc/nghttp.1 b/doc/nghttp.1 index b9ea4d74..6167c112 100644 --- a/doc/nghttp.1 +++ b/doc/nghttp.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTP" "1" "April 27, 2015" "0.7.13-DEV" "nghttp2" +.TH "NGHTTP" "1" "April 27, 2015" "0.7.13" "nghttp2" .SH NAME nghttp \- HTTP/2 experimental client . diff --git a/doc/nghttpd.1 b/doc/nghttpd.1 index 2eb1fe3f..086eb9df 100644 --- a/doc/nghttpd.1 +++ b/doc/nghttpd.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPD" "1" "April 27, 2015" "0.7.13-DEV" "nghttp2" +.TH "NGHTTPD" "1" "April 27, 2015" "0.7.13" "nghttp2" .SH NAME nghttpd \- HTTP/2 experimental server . diff --git a/doc/nghttpx.1 b/doc/nghttpx.1 index 0cba2e19..8aa24018 100644 --- a/doc/nghttpx.1 +++ b/doc/nghttpx.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPX" "1" "April 27, 2015" "0.7.13-DEV" "nghttp2" +.TH "NGHTTPX" "1" "April 27, 2015" "0.7.13" "nghttp2" .SH NAME nghttpx \- HTTP/2 experimental proxy . From 63630690a851a7dc36ebe0c6065df72c13ee4a93 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 27 Apr 2015 23:52:36 +0900 Subject: [PATCH 14/17] Fix `make -j3 distcheck` error --- doc/Makefile.am | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index deead9d4..ed3b9ed1 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -201,11 +201,28 @@ help: @echo " linkcheck to check all external links for integrity" @echo " doctest to run all doctests embedded in the documentation (if enabled)" -apiref.rst: $(top_builddir)/lib/includes/nghttp2/nghttp2ver.h \ +apiref.rst macros.rst enums.rst types.rst: \ + $(top_builddir)/lib/includes/nghttp2/nghttp2ver.h \ $(top_builddir)/lib/includes/nghttp2/nghttp2.h $(PYTHON) $(top_srcdir)/doc/mkapiref.py \ $@ macros.rst enums.rst types.rst . $^ +# Inspired by +# http://www.gnu.org/savannah-checkouts/gnu/automake/manual/html_node/Multiple-Outputs.html +apidoc.stamp: $(top_builddir)/lib/includes/nghttp2/nghttp2ver.h \ + $(top_builddir)/lib/includes/nghttp2/nghttp2.h + @rm -f apidoc.tmp + @touch apidoc.tmp + $(PYTHON) $(top_srcdir)/doc/mkapiref.py \ + $@ macros.rst enums.rst types.rst . $^ + @mv -f apidoc.tmp $@ +$(APIDOC): apidoc.stamp +## Recover from the removal of $@ + @if test -f $@; then :; else \ + rm -f apidoc.stamp; \ + $(MAKE) $(AM_MAKEFLAGS) apidoc.stamp; \ + fi + clean-local: -rm $(APIDOCS) -rm -rf $(BUILDDIR)/* From 54bff9176292970dbfddfd2c62f747cab82f84ac Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 28 Apr 2015 00:01:09 +0900 Subject: [PATCH 15/17] Bump up version number to 0.7.14-DEV --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 93bd2062..0d2220b3 100644 --- a/configure.ac +++ b/configure.ac @@ -25,7 +25,7 @@ dnl Do not change user variables! dnl http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html AC_PREREQ(2.61) -AC_INIT([nghttp2], [0.7.13], [t-tujikawa@users.sourceforge.net]) +AC_INIT([nghttp2], [0.7.14-DEV], [t-tujikawa@users.sourceforge.net]) LT_PREREQ([2.2.6]) LT_INIT() dnl See versioning rule: From 9e1b068a4bae3302e2c382c627ed35345c2f3764 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 28 Apr 2015 21:38:52 +0900 Subject: [PATCH 16/17] Fix bug that promised stream was not reset on decompression error --- lib/nghttp2_session.c | 6 ++-- tests/nghttp2_session_test.c | 62 +++++++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 5a0fc76c..495d4102 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3131,12 +3131,12 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, } if (proclen < 0) { if (session->iframe.state == NGHTTP2_IB_READ_HEADER_BLOCK) { - if (stream && stream->state != NGHTTP2_STREAM_CLOSING) { + if (subject_stream && subject_stream->state != NGHTTP2_STREAM_CLOSING) { /* Adding RST_STREAM here is very important. It prevents from invoking subsequent callbacks for the same stream ID. */ - rv = nghttp2_session_add_rst_stream(session, frame->hd.stream_id, - NGHTTP2_COMPRESSION_ERROR); + rv = nghttp2_session_add_rst_stream( + session, subject_stream->stream_id, NGHTTP2_COMPRESSION_ERROR); if (nghttp2_is_fatal(rv)) { return rv; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 42ca1e64..ccf42ce7 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1093,9 +1093,6 @@ void test_nghttp2_session_recv_headers_with_priority(void) { void test_nghttp2_session_recv_premature_headers(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; - nghttp2_nv *nva; - size_t nvlen; - nghttp2_frame frame; nghttp2_bufs bufs; nghttp2_buf *buf; ssize_t rv; @@ -1103,45 +1100,72 @@ void test_nghttp2_session_recv_premature_headers(void) { nghttp2_hd_deflater deflater; nghttp2_outbound_item *item; nghttp2_mem *mem; + uint32_t payloadlen; mem = nghttp2_mem_default(); frame_pack_bufs_init(&bufs); memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.send_callback = null_send_callback; nghttp2_session_server_new(&session, &callbacks, &ud); nghttp2_hd_deflate_init(&deflater, mem); - nvlen = ARRLEN(reqnv); - nghttp2_nv_array_copy(&nva, reqnv, nvlen, mem); - nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 1, - NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); - rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); - - CU_ASSERT(0 == rv); - CU_ASSERT(nghttp2_bufs_len(&bufs) > 0); - - nghttp2_frame_headers_free(&frame.headers, mem); + rv = pack_headers(&bufs, &deflater, 1, NGHTTP2_FLAG_END_HEADERS, reqnv, + ARRLEN(reqnv), mem); buf = &bufs.head->buf; - assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf)); - /* Intentionally feed payload cutting last 1 byte off */ - nghttp2_put_uint32be(buf->pos, - (uint32_t)(((frame.hd.length - 1) << 8) + buf->pos[3])); + payloadlen = nghttp2_get_uint32(buf->pos) >> 8; + nghttp2_put_uint32be(buf->pos, ((payloadlen - 1) << 8) + buf->pos[3]); rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf) - 1); - CU_ASSERT((ssize_t)(nghttp2_buf_len(buf) - 1) == rv); + CU_ASSERT(rv == nghttp2_buf_len(buf) - 1); item = nghttp2_session_get_next_ob_item(session); + CU_ASSERT(NULL != item); CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type); CU_ASSERT(NGHTTP2_COMPRESSION_ERROR == item->frame.rst_stream.error_code); + CU_ASSERT(1 == item->frame.hd.stream_id); + CU_ASSERT(0 == nghttp2_session_send(session)); - nghttp2_bufs_free(&bufs); + nghttp2_bufs_reset(&bufs); nghttp2_hd_deflate_free(&deflater); nghttp2_session_del(session); + + /* Test for PUSH_PROMISE */ + nghttp2_session_client_new(&session, &callbacks, &ud); + nghttp2_hd_deflate_init(&deflater, mem); + + nghttp2_session_open_stream(session, 1, NGHTTP2_STREAM_FLAG_NONE, + &pri_spec_default, NGHTTP2_STREAM_OPENING, NULL); + + rv = pack_push_promise(&bufs, &deflater, 1, NGHTTP2_FLAG_END_HEADERS, 2, + reqnv, ARRLEN(reqnv), mem); + + CU_ASSERT(0 == rv); + + buf = &bufs.head->buf; + payloadlen = nghttp2_get_uint32(buf->pos) >> 8; + /* Intentionally feed payload cutting last 1 byte off */ + nghttp2_put_uint32be(buf->pos, ((payloadlen - 1) << 8) + buf->pos[3]); + rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf) - 1); + + CU_ASSERT(rv == nghttp2_buf_len(buf) - 1); + + item = nghttp2_session_get_next_ob_item(session); + + CU_ASSERT(NULL != item); + CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type); + CU_ASSERT(NGHTTP2_COMPRESSION_ERROR == item->frame.rst_stream.error_code); + CU_ASSERT(2 == item->frame.hd.stream_id); + CU_ASSERT(0 == nghttp2_session_send(session)); + + nghttp2_hd_deflate_free(&deflater); + nghttp2_session_del(session); + nghttp2_bufs_free(&bufs); } void test_nghttp2_session_recv_unknown_frame(void) { From f05a4830c5bd99942161663bde216347ab89e5b3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 28 Apr 2015 21:51:28 +0900 Subject: [PATCH 17/17] nghttp: Fix assertion error if very large value is given to -t --- src/nghttp.cc | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index 56e09ec9..49e1825b 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -2315,8 +2315,9 @@ Options: filename is dereived from URI. If URI ends with '/', 'index.html' is used as a filename. Not implemented yet. - -t, --timeout= - Timeout each request after seconds. + -t, --timeout= + Timeout each request after . Set 0 to disable + timeout. -w, --window-bits= Sets the stream level initial window size to 2**-1. -W, --connection-window-bits= @@ -2386,7 +2387,12 @@ Options: -- The argument is an integer and an optional unit (e.g., 10K is - 10 * 1024). Units are K, M and G (powers of 1024).)" << std::endl; + 10 * 1024). Units are K, M and G (powers of 1024). + + The argument is an integer and an optional unit (e.g., 1s + is 1 second and 500ms is 500 milliseconds). Units are h, m, s or ms + (hours, minutes, seconds and milliseconds, respectively). If a unit + is omitted, a second is used as unit.)" << std::endl; } } // namespace @@ -2472,7 +2478,11 @@ int main(int argc, char **argv) { ++config.verbose; break; case 't': - config.timeout = atoi(optarg); + config.timeout = util::parse_duration_with_unit(optarg); + if (config.timeout == std::numeric_limits::infinity()) { + std::cerr << "-t: bad timeout value: " << optarg << std::endl; + exit(EXIT_FAILURE); + } break; case 'u': config.upgrade = true;