Commit 497ffc63 authored by Tatsuhiro Tsujikawa's avatar Tatsuhiro Tsujikawa

nghttpx: Change pushed stream's priority

By default, as RFC 7540 calls for, pushed stream depends on its
associated (parent) stream.  There are some situations that this is
sub-optimal.  For example, if associated stream is HTML, and server is
configured to push css and javascript files which are in critical
rendering path.  Then the default priority scheme is sub-optimal,
since browser typically blocks rendering while waiting for critical
resources.  In this case, it is better to at least give pushed stream
the same priority of associated stream, and interleave these streams.

This change gives pushed stream the same priority of associated stream
if pushed stream has content-type "application/javascript" or
"text/css".  The pushed stream now depends on the stream which
associated stream depends on.  We use the same weight of associated
stream.
parent 5a3ca7e7
......@@ -30,6 +30,7 @@ HEADERS = [
"cache-control",
"user-agent",
"date",
"content-type",
# disallowed h1 headers
'connection',
'keep-alive',
......
......@@ -673,6 +673,15 @@ int lookup_token(const uint8_t *name, size_t namelen) {
break;
}
break;
case 12:
switch (name[11]) {
case 'e':
if (util::streq_l("content-typ", name, 11)) {
return HD_CONTENT_TYPE;
}
break;
}
break;
case 13:
switch (name[12]) {
case 'l':
......
......@@ -239,6 +239,7 @@ enum {
HD_CACHE_CONTROL,
HD_CONNECTION,
HD_CONTENT_LENGTH,
HD_CONTENT_TYPE,
HD_COOKIE,
HD_DATE,
HD_EXPECT,
......
......@@ -1020,7 +1020,7 @@ void fill_default_config() {
downstreamconf.connections_per_host = 8;
downstreamconf.request_buffer_size = 16_k;
downstreamconf.response_buffer_size = 16_k;
downstreamconf.response_buffer_size = 128_k;
}
}
......
......@@ -117,7 +117,7 @@ Downstream::Downstream(Upstream *upstream, MemchunkPool *mcpool,
request_start_time_(std::chrono::high_resolution_clock::now()),
request_buf_(mcpool), response_buf_(mcpool), upstream_(upstream),
blocked_link_(nullptr), num_retry_(0), stream_id_(stream_id),
downstream_stream_id_(-1),
assoc_stream_id_(-1), downstream_stream_id_(-1),
response_rst_stream_error_code_(NGHTTP2_NO_ERROR),
request_state_(INITIAL), response_state_(INITIAL),
dispatch_state_(DISPATCH_NONE), upgraded_(false), chunked_request_(false),
......@@ -897,4 +897,10 @@ DefaultMemchunks Downstream::pop_response_buf() {
return std::move(response_buf_);
}
void Downstream::set_assoc_stream_id(int32_t stream_id) {
assoc_stream_id_ = stream_id;
}
int32_t Downstream::get_assoc_stream_id() const { return assoc_stream_id_; }
} // namespace shrpx
......@@ -201,6 +201,8 @@ public:
Upstream *get_upstream() const;
void set_stream_id(int32_t stream_id);
int32_t get_stream_id() const;
void set_assoc_stream_id(int32_t stream_id);
int32_t get_assoc_stream_id() const;
void pause_read(IOCtrlReason reason);
int resume_read(IOCtrlReason reason, size_t consumed);
void force_resume_read();
......@@ -406,6 +408,9 @@ private:
size_t num_retry_;
// The stream ID in frontend connection
int32_t stream_id_;
// The associated stream ID in frontend connection if this is pushed
// stream.
int32_t assoc_stream_id_;
// stream ID in backend connection
int32_t downstream_stream_id_;
// RST_STREAM error_code from downstream HTTP2 connection
......
......@@ -555,18 +555,19 @@ int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame,
return 0;
}
auto downstream = make_unique<Downstream>(upstream, handler->get_mcpool(),
promised_stream_id);
auto &req = downstream->request();
auto promised_downstream = make_unique<Downstream>(
upstream, handler->get_mcpool(), promised_stream_id);
auto &req = promised_downstream->request();
// As long as we use nghttp2_session_mem_send(), setting stream
// user data here should not fail. This is because this callback
// is called just after frame was serialized. So no worries about
// hanging Downstream.
nghttp2_session_set_stream_user_data(session, promised_stream_id,
downstream.get());
promised_downstream.get());
downstream->disable_upstream_rtimer();
promised_downstream->set_assoc_stream_id(frame->hd.stream_id);
promised_downstream->disable_upstream_rtimer();
req.http_major = 2;
req.http_minor = 0;
......@@ -592,14 +593,14 @@ int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame,
nv.flags & NGHTTP2_NV_FLAG_NO_INDEX, token);
}
downstream->inspect_http2_request();
promised_downstream->inspect_http2_request();
downstream->set_request_state(Downstream::MSG_COMPLETE);
promised_downstream->set_request_state(Downstream::MSG_COMPLETE);
// a bit weird but start_downstream() expects that given
// downstream is in pending queue.
auto ptr = downstream.get();
upstream->add_pending_downstream(std::move(downstream));
auto ptr = promised_downstream.get();
upstream->add_pending_downstream(std::move(promised_downstream));
#ifdef HAVE_MRUBY
auto worker = handler->get_worker();
......@@ -1499,6 +1500,13 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) {
return 0;
}
if (downstream->get_assoc_stream_id() != -1) {
rv = adjust_pushed_stream_priority(downstream);
if (rv != 0) {
return -1;
}
}
http2::copy_headers_to_nva_nocopy(nva, resp.fs.headers());
if (!get_config()->http2_proxy && !get_config()->client_proxy) {
......@@ -1611,6 +1619,66 @@ int Http2Upstream::on_downstream_body(Downstream *downstream,
return 0;
}
int Http2Upstream::adjust_pushed_stream_priority(Downstream *downstream) {
int rv;
// We only change pushed stream. The pushed stream has
// assoc_stream_id which is not -1.
auto assoc_stream_id = downstream->get_assoc_stream_id();
auto stream_id = downstream->get_stream_id();
auto assoc_stream = nghttp2_session_find_stream(session_, assoc_stream_id);
auto stream = nghttp2_session_find_stream(session_, stream_id);
// By default, downstream depends on assoc_stream. If its
// relationship is changed, then we don't change priority.
if (!assoc_stream || assoc_stream != nghttp2_stream_get_parent(stream)) {
return 0;
}
// We are going to make stream depend on dep_stream which is the
// parent stream of assoc_stream, if the content-type of stream
// indicates javascript or css.
auto dep_stream = nghttp2_stream_get_parent(assoc_stream);
if (!dep_stream) {
return 0;
}
const auto &resp = downstream->response();
auto ct = resp.fs.header(http2::HD_CONTENT_TYPE);
if (!ct) {
return 0;
}
if (!util::istarts_with_l(ct->value, "application/javascript") &&
!util::istarts_with_l(ct->value, "text/css")) {
return 0;
}
auto dep_stream_id = nghttp2_stream_get_stream_id(dep_stream);
auto weight = nghttp2_stream_get_weight(assoc_stream);
nghttp2_priority_spec pri_spec;
nghttp2_priority_spec_init(&pri_spec, dep_stream_id, weight, 0);
rv = nghttp2_session_change_stream_priority(session_, stream_id, &pri_spec);
if (nghttp2_is_fatal(rv)) {
ULOG(FATAL, this) << "nghttp2_session_change_stream_priority() failed: "
<< nghttp2_strerror(rv);
return -1;
}
if (rv == 0) {
if (LOG_ENABLED(INFO)) {
ULOG(INFO, this) << "Changed pushed stream priority: pushed stream("
<< stream_id << ") now depends on stream("
<< dep_stream_id << ") with weight " << weight;
}
}
return 0;
}
// WARNING: Never call directly or indirectly nghttp2_session_send or
// nghttp2_session_recv. These calls may delete downstream.
int Http2Upstream::on_downstream_body_complete(Downstream *downstream) {
......@@ -1953,6 +2021,8 @@ Http2Upstream::on_downstream_push_promise(Downstream *downstream,
auto &promised_req = promised_downstream->request();
promised_downstream->set_downstream_stream_id(promised_stream_id);
// Set associated stream in frontend
promised_downstream->set_assoc_stream_id(downstream->get_stream_id());
promised_downstream->disable_upstream_rtimer();
......
......@@ -125,6 +125,10 @@ public:
void set_pending_data_downstream(Downstream *downstream, size_t n,
size_t padlen);
// Changes stream priority of |downstream|, which is assumed to be a
// pushed stream.
int adjust_pushed_stream_priority(Downstream *downstream);
private:
WriteBuffer wb_;
std::unique_ptr<HttpsUpstream> pre_upstream_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment