From 436595df9818a2aa463f8eaece9bfc92355fdf2f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Apr 2015 21:31:11 +0900 Subject: [PATCH] nghttp: Remove --dep-idle option In this commit, we made --dep-idle behaviour by default. This is because the previous default behaviour is not reflect current usage of dependency priority and never will be because of fragility of tree due to stream closure. --- src/HtmlParser.cc | 6 +- src/HtmlParser.h | 12 ++-- src/nghttp.cc | 160 +++++++++++++--------------------------------- src/nghttp.h | 20 +----- 4 files changed, 53 insertions(+), 145 deletions(-) diff --git a/src/HtmlParser.cc b/src/HtmlParser.cc index 51f6a12b..bcbfd13f 100644 --- a/src/HtmlParser.cc +++ b/src/HtmlParser.cc @@ -53,13 +53,13 @@ const char *get_attr(const xmlChar **attrs, const char *name) { } // namespace namespace { -void add_link(ParserData *parser_data, const char *uri, RequestPriority pri) { +void add_link(ParserData *parser_data, const char *uri, ResourceType res_type) { auto u = xmlBuildURI( reinterpret_cast(uri), reinterpret_cast(parser_data->base_uri.c_str())); if (u) { parser_data->links.push_back( - std::make_pair(reinterpret_cast(u), pri)); + std::make_pair(reinterpret_cast(u), res_type)); free(u); } } @@ -177,7 +177,7 @@ int HtmlParser::parse_chunk_internal(const char *chunk, size_t size, int fin) { } } -const std::vector> & +const std::vector> & HtmlParser::get_links() const { return parser_data_.links; } diff --git a/src/HtmlParser.h b/src/HtmlParser.h index 694b6c9b..fc2f2e57 100644 --- a/src/HtmlParser.h +++ b/src/HtmlParser.h @@ -38,8 +38,7 @@ namespace nghttp2 { -// Lower value has higher priority -enum RequestPriority { +enum ResourceType { REQ_CSS = 1, REQ_JS, REQ_UNBLOCK_JS, @@ -49,7 +48,7 @@ enum RequestPriority { struct ParserData { std::string base_uri; - std::vector> links; + std::vector> links; // > 0 if we are inside "head" element. int inside_head; ParserData(const std::string &base_uri); @@ -62,7 +61,7 @@ public: HtmlParser(const std::string &base_uri); ~HtmlParser(); int parse_chunk(const char *chunk, size_t size, int fin); - const std::vector> &get_links() const; + const std::vector> &get_links() const; void clear_links(); private: @@ -79,14 +78,13 @@ class HtmlParser { public: HtmlParser(const std::string &base_uri) {} int parse_chunk(const char *chunk, size_t size, int fin) { return 0; } - const std::vector> & - get_links() const { + const std::vector> &get_links() const { return links_; } void clear_links() {} private: - std::vector> links_; + std::vector> links_; }; #endif // !HAVE_LIBXML2 diff --git a/src/nghttp.cc b/src/nghttp.cc index d4b02c3c..41994de9 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -61,9 +61,9 @@ namespace nghttp2 { -// The anchor stream nodes when --dep-idle is enabled. The stream ID -// = 1 is excluded since it is used as first stream in upgrade case. -// We follows the same dependency anchor nodes as Firefox does. +// The anchor stream nodes when --no-dep is not used. The stream ID = +// 1 is excluded since it is used as first stream in upgrade case. We +// follows the same dependency anchor nodes as Firefox does. struct Anchor { int32_t stream_id; // stream ID this anchor depends on @@ -95,7 +95,7 @@ Config::Config() timeout(0.), window_bits(-1), connection_window_bits(-1), verbose(0), null_out(false), remote_name(false), get_assets(false), stat(false), upgrade(false), continuation(false), no_content_length(false), - no_dep(false), dep_idle(false), hexdump(false) { + no_dep(false), hexdump(false) { nghttp2_option_new(&http2_option); nghttp2_option_set_peer_max_concurrent_streams(http2_option, peer_max_concurrent_streams); @@ -127,12 +127,10 @@ std::string strip_fragment(const char *raw_uri) { Request::Request(const std::string &uri, const http_parser_url &u, const nghttp2_data_provider *data_prd, int64_t data_length, - const nghttp2_priority_spec &pri_spec, - std::shared_ptr dep, int pri, int level) - : uri(uri), u(u), dep(std::move(dep)), pri_spec(pri_spec), - data_length(data_length), data_offset(0), response_len(0), - inflater(nullptr), html_parser(nullptr), data_prd(data_prd), - stream_id(-1), status(0), level(level), pri(pri), + const nghttp2_priority_spec &pri_spec, int level) + : uri(uri), u(u), pri_spec(pri_spec), data_length(data_length), + data_offset(0), response_len(0), inflater(nullptr), html_parser(nullptr), + data_prd(data_prd), stream_id(-1), status(0), level(level), expect_final_response(false) { http2::init_hdidx(res_hdidx); http2::init_hdidx(req_hdidx); @@ -171,83 +169,41 @@ std::string Request::make_reqpath() const { return path; } -int32_t Request::find_dep_stream_id(int start) { - for (auto i = start; i >= 0; --i) { - for (auto req : dep->deps[i]) { - return req->stream_id; - } - } - return -1; -} - -nghttp2_priority_spec Request::resolve_dep(int32_t pri) { +namespace { +nghttp2_priority_spec resolve_dep(int res_type) { nghttp2_priority_spec pri_spec; - int exclusive = 0; - int32_t stream_id = -1; - - nghttp2_priority_spec_default_init(&pri_spec); if (config.no_dep) { + nghttp2_priority_spec_default_init(&pri_spec); + return pri_spec; } - if (config.dep_idle) { - int32_t anchor_id; - int32_t weight; - switch (pri) { - case REQ_CSS: - case REQ_JS: - anchor_id = anchors[ANCHOR_LEADERS].stream_id; - weight = 2; - break; - case REQ_UNBLOCK_JS: - anchor_id = anchors[ANCHOR_UNBLOCKED].stream_id; - weight = 2; - break; - case REQ_IMG: - anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; - weight = 12; - break; - default: - anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; - weight = 2; - } - - nghttp2_priority_spec_init(&pri_spec, anchor_id, weight, 0); - return pri_spec; + int32_t anchor_id; + int32_t weight; + switch (res_type) { + case REQ_CSS: + case REQ_JS: + anchor_id = anchors[ANCHOR_LEADERS].stream_id; + weight = 2; + break; + case REQ_UNBLOCK_JS: + anchor_id = anchors[ANCHOR_UNBLOCKED].stream_id; + weight = 2; + break; + case REQ_IMG: + anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; + weight = 12; + break; + default: + anchor_id = anchors[ANCHOR_FOLLOWERS].stream_id; + weight = 2; } - if (pri == 0) { - return pri_spec; - } - - auto start = std::min(pri, (int)dep->deps.size() - 1); - - for (auto i = start; i >= 0; --i) { - if (dep->deps[i][0]->pri < pri) { - stream_id = find_dep_stream_id(i); - - if (i != (int)dep->deps.size() - 1) { - exclusive = 1; - } - - break; - } else if (dep->deps[i][0]->pri == pri) { - stream_id = find_dep_stream_id(i - 1); - - break; - } - } - - if (stream_id == -1) { - return pri_spec; - } - - nghttp2_priority_spec_init(&pri_spec, stream_id, NGHTTP2_DEFAULT_WEIGHT, - exclusive); - + nghttp2_priority_spec_init(&pri_spec, anchor_id, weight, 0); return pri_spec; } +} // namespace bool Request::is_ipv6_literal_addr() const { if (util::has_uri_field(u, UF_HOST)) { @@ -999,7 +955,7 @@ int HttpClient::connection_made() { return -1; } } - if (!config.no_dep && config.dep_idle) { + if (!config.no_dep) { // Create anchor stream nodes nghttp2_priority_spec pri_spec; @@ -1256,9 +1212,7 @@ void HttpClient::update_hostport() { bool HttpClient::add_request(const std::string &uri, const nghttp2_data_provider *data_prd, int64_t data_length, - const nghttp2_priority_spec &pri_spec, - std::shared_ptr dep, int pri, - int level) { + const nghttp2_priority_spec &pri_spec, int level) { http_parser_url u; memset(&u, 0, sizeof(u)); if (http_parser_parse_url(uri.c_str(), uri.size(), 0, &u) != 0) { @@ -1272,8 +1226,8 @@ bool HttpClient::add_request(const std::string &uri, path_cache.insert(uri); } - reqvec.push_back(make_unique(uri, u, data_prd, data_length, pri_spec, - std::move(dep), pri, level)); + reqvec.push_back( + make_unique(uri, u, data_prd, data_length, pri_spec, level)); return true; } @@ -1294,26 +1248,6 @@ void HttpClient::request_done(Request *req) { if (req->stream_id % 2 == 0) { return; } - - auto itr = std::begin(req->dep->deps); - for (; itr != std::end(req->dep->deps); ++itr) { - if ((*itr)[0]->pri == req->pri) { - (*itr).push_back(req); - - break; - } - - if ((*itr)[0]->pri > req->pri) { - auto v = std::vector{req}; - req->dep->deps.insert(itr, std::move(v)); - - break; - } - } - - if (itr == std::end(req->dep->deps)) { - req->dep->deps.push_back(std::vector{req}); - } } #ifdef HAVE_JANSSON @@ -1496,7 +1430,7 @@ void update_html_parser(HttpClient *client, Request *req, const uint8_t *data, for (auto &p : req->html_parser->get_links()) { auto uri = strip_fragment(p.first.c_str()); - auto pri = p.second; + auto res_type = p.second; http_parser_url u; memset(&u, 0, sizeof(u)); @@ -1505,10 +1439,9 @@ void update_html_parser(HttpClient *client, Request *req, const uint8_t *data, util::fieldeq(uri.c_str(), u, req->uri.c_str(), req->u, UF_HOST) && util::porteq(uri.c_str(), u, req->uri.c_str(), req->u)) { // No POST data for assets - auto pri_spec = req->resolve_dep(pri); + auto pri_spec = resolve_dep(res_type); - if (client->add_request(uri, nullptr, 0, pri_spec, req->dep, pri, - req->level + 1)) { + if (client->add_request(uri, nullptr, 0, pri_spec, req->level + 1)) { submit_request(client, config.headers, client->reqvec.back().get()); } @@ -1671,7 +1604,7 @@ int on_begin_headers_callback(nghttp2_session *session, nghttp2_priority_spec_default_init(&pri_spec); - auto req = make_unique("", u, nullptr, 0, pri_spec, nullptr); + auto req = make_unique("", u, nullptr, 0, pri_spec); req->stream_id = stream_id; nghttp2_session_set_stream_user_data(session, stream_id, req.get()); @@ -2069,7 +2002,7 @@ int communicate( nghttp2_priority_spec pri_spec; int32_t dep_stream_id = 0; - if (!config.no_dep && config.dep_idle) { + if (!config.no_dep) { dep_stream_id = anchors[ANCHOR_FOLLOWERS].stream_id; } @@ -2077,9 +2010,8 @@ int communicate( for (auto req : requests) { for (int i = 0; i < config.multiply; ++i) { - auto dep = std::make_shared(); client.add_request(std::get<0>(req), std::get<1>(req), std::get<2>(req), - pri_spec, std::move(dep)); + pri_spec); } } client.update_hostport(); @@ -2436,7 +2368,6 @@ Options: --no-content-length Don't send content-length header field. --no-dep Don't send dependency based priority hint to server. - --dep-idle Use idle streams as anchor nodes to express priority. --hexdump Display the incoming traffic in hexadecimal (Canonical hex+ASCII display). If SSL/TLS is used, decrypted data are used. @@ -2480,7 +2411,6 @@ int main(int argc, char **argv) { {"version", no_argument, &flag, 5}, {"no-content-length", no_argument, &flag, 6}, {"no-dep", no_argument, &flag, 7}, - {"dep-idle", no_argument, &flag, 8}, {"trailer", required_argument, &flag, 9}, {"hexdump", no_argument, &flag, 10}, {nullptr, 0, nullptr, 0}}; @@ -2638,10 +2568,6 @@ int main(int argc, char **argv) { // no-dep option config.no_dep = true; break; - case 8: - // dep-idle option - config.dep_idle = true; - break; case 9: { // trailer option auto header = optarg; diff --git a/src/nghttp.h b/src/nghttp.h index bb2fc449..02421fc5 100644 --- a/src/nghttp.h +++ b/src/nghttp.h @@ -83,7 +83,6 @@ struct Config { bool continuation; bool no_content_length; bool no_dep; - bool dep_idle; bool hexdump; }; @@ -103,18 +102,11 @@ struct RequestTiming { RequestTiming() : state(RequestState::INITIAL) {} }; -struct Request; - -struct Dependency { - std::vector> deps; -}; - struct Request { // For pushed request, |uri| is empty and |u| is zero-cleared. Request(const std::string &uri, const http_parser_url &u, const nghttp2_data_provider *data_prd, int64_t data_length, - const nghttp2_priority_spec &pri_spec, - std::shared_ptr dep, int pri = 0, int level = 0); + const nghttp2_priority_spec &pri_spec, int level = 0); ~Request(); void init_inflater(); @@ -124,10 +116,6 @@ struct Request { std::string make_reqpath() const; - int32_t find_dep_stream_id(int start); - - nghttp2_priority_spec resolve_dep(int32_t pri); - bool is_ipv6_literal_addr() const; bool response_pseudo_header_allowed(int16_t token) const; @@ -145,7 +133,6 @@ struct Request { // URI without fragment std::string uri; http_parser_url u; - std::shared_ptr dep; nghttp2_priority_spec pri_spec; RequestTiming timing; int64_t data_length; @@ -159,8 +146,6 @@ struct Request { int status; // Recursion level: 0: first entity, 1: entity linked from first entity int level; - // RequestPriority value defined in HtmlParser.h - int pri; http2::HeaderIndex res_hdidx; // used for incoming PUSH_PROMISE http2::HeaderIndex req_hdidx; @@ -220,8 +205,7 @@ struct HttpClient { void update_hostport(); bool add_request(const std::string &uri, const nghttp2_data_provider *data_prd, int64_t data_length, - const nghttp2_priority_spec &pri_spec, - std::shared_ptr dep, int pri = 0, int level = 0); + const nghttp2_priority_spec &pri_spec, int level = 0); void record_start_time(); void record_domain_lookup_end_time();