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

@@ -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<const xmlChar *>(uri),
       reinterpret_cast<const xmlChar *>(parser_data->base_uri.c_str()));
   if (u) {
     parser_data->links.push_back(
-        std::make_pair(reinterpret_cast<char *>(u), pri));
+        std::make_pair(reinterpret_cast<char *>(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<std::pair<std::string, RequestPriority>> &
+const std::vector<std::pair<std::string, ResourceType>> &
 HtmlParser::get_links() const {
   return parser_data_.links;
 }

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<std::pair<std::string, RequestPriority>> links;
+  std::vector<std::pair<std::string, ResourceType>> 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<std::pair<std::string, RequestPriority>> &get_links() const;
+  const std::vector<std::pair<std::string, ResourceType>> &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<std::pair<std::string, RequestPriority>> &
-  get_links() const {
+  const std::vector<std::pair<std::string, ResourceType>> &get_links() const {
     return links_;
   }
   void clear_links() {}
 
 private:
-  std::vector<std::pair<std::string, RequestPriority>> links_;
+  std::vector<std::pair<std::string, ResourceType>> links_;
 };
 
 #endif // !HAVE_LIBXML2

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<Dependency> 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) {
-    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;
-  }
+    nghttp2_priority_spec_default_init(&pri_spec);
 
-  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;
+  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;
   }
 
-  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<Dependency> 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<Request>(uri, u, data_prd, data_length, pri_spec,
-                                        std::move(dep), pri, level));
+  reqvec.push_back(
+      make_unique<Request>(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<Request *>{req};
-      req->dep->deps.insert(itr, std::move(v));
-
-      break;
-    }
-  }
-
-  if (itr == std::end(req->dep->deps)) {
-    req->dep->deps.push_back(std::vector<Request *>{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<Request>("", u, nullptr, 0, pri_spec, nullptr);
+    auto req = make_unique<Request>("", 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<Dependency>();
         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;

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<std::vector<Request *>> 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<Dependency> 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<Dependency> 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<Dependency> 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();