Distribute closed or blocked stream's weight to its siblings

This also means that at least one stream whose dpri is
NGHTTP2_STREAM_DPRI_TOP exists, its siblings descendants have no
chance to send streams, even if their parent stream has
NGHTTP2_STREAM_DPRI_NODATA.

lib/nghttp2_stream.c

@@ -65,6 +65,7 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id,
   stream->effective_weight = stream->weight;
   stream->sum_dep_weight = 0;
   stream->sum_norest_weight = 0;
+  stream->sum_top_weight = 0;
 
   stream->roots = roots;
   stream->root_prev = NULL;
@@ -157,6 +158,20 @@ int32_t nghttp2_stream_dep_distributed_effective_weight
   return nghttp2_max(1, weight);
 }
 
+static int32_t stream_dep_distributed_top_effective_weight
+(nghttp2_stream *stream, int32_t weight)
+{
+  if(stream->sum_top_weight == 0) {
+    return stream->effective_weight;
+  }
+
+  weight = stream->effective_weight * weight / stream->sum_top_weight;
+
+  return nghttp2_max(1, weight);
+}
+
+static void stream_update_dep_set_rest(nghttp2_stream *stream);
+
 /* Updates effective_weight of descendant streams in subtree of
    |stream|.  We assume that stream->effective_weight is already set
    right. */
@@ -165,9 +180,10 @@ static void stream_update_dep_effective_weight(nghttp2_stream *stream)
   nghttp2_stream *si;
 
   DEBUGF(fprintf(stderr, "stream: update_dep_effective_weight "
-                 "stream(%p)=%d, weight=%d, sum_norest_weight=%d\n",
+                 "stream(%p)=%d, weight=%d, sum_norest_weight=%d, "
+                 "sum_top_weight=%d\n",
                  stream, stream->stream_id, stream->weight,
-                 stream->sum_norest_weight));
+                 stream->sum_norest_weight, stream->sum_top_weight));
 
   /* stream->sum_norest_weight == 0 means there is no
      NGHTTP2_STREAM_DPRI_TOP under stream */
@@ -176,13 +192,38 @@ static void stream_update_dep_effective_weight(nghttp2_stream *stream)
     return;
   }
 
+  /* If there is no direct descendant whose dpri is
+     NGHTTP2_STREAM_DPRI_TOP, indirect descendants have the chance to
+     send data, so recursively set weight for descendants. */
+  if(stream->sum_top_weight == 0) {
+    for(si = stream->dep_next; si; si = si->sib_next) {
+      if(si->dpri != NGHTTP2_STREAM_DPRI_REST) {
+        si->effective_weight = nghttp2_stream_dep_distributed_effective_weight
+          (stream, si->weight);
+      }
+
+      stream_update_dep_effective_weight(si);
+    }
+    return;
+  }
+
+  /* If there is at least one direct descendant whose dpri is
+     NGHTTP2_STREAM_DPRI_TOP, we won't give a chance to indirect
+     descendants, since closed or blocked stream's weight is
+     distributed among its siblings */
   for(si = stream->dep_next; si; si = si->sib_next) {
-    if(si->dpri != NGHTTP2_STREAM_DPRI_REST) {
+    if(si->dpri == NGHTTP2_STREAM_DPRI_TOP) {
-      si->effective_weight = nghttp2_stream_dep_distributed_effective_weight
+      si->effective_weight = stream_dep_distributed_top_effective_weight
         (stream, si->weight);
+
+      continue;
     }
 
-    stream_update_dep_effective_weight(si);
+    if(si->dpri == NGHTTP2_STREAM_DPRI_NO_DATA) {
+      /* Since we marked NGHTTP2_STREAM_DPRI_TOP under si, we make
+         them NGHTTP2_STREAM_DPRI_REST again. */
+      stream_update_dep_set_rest(si->dep_next);
+    }
   }
 }
 
@@ -257,14 +298,18 @@ static int stream_update_dep_set_top(nghttp2_stream *stream, nghttp2_pq *pq,
 }
 
 /*
- * Updates stream->sum_norest_weight recursively.  We have to gather
+ * Updates stream->sum_norest_weight and stream->sum_top_weight
- * effective sum of weight of descendants.  If stream->dpri ==
+ * recursively.  We have to gather effective sum of weight of
- * NGHTTP2_STREAM_DPRI_NO_DATA, we have to go deeper and check that
+ * descendants.  If stream->dpri == NGHTTP2_STREAM_DPRI_NO_DATA, we
- * any of its descendants has dpri value of NGHTTP2_STREAM_DPRI_TOP.
+ * have to go deeper and check that any of its descendants has dpri
- * If so, we have to add weight of its direct descendants to
+ * value of NGHTTP2_STREAM_DPRI_TOP.  If so, we have to add weight of
- * stream->sum_norest_weight.  To make this work, this function
+ * its direct descendants to stream->sum_norest_weight.  To make this
- * returns 1 if any of its descendants has dpri value of
+ * work, this function returns 1 if any of its descendants has dpri
- * NGHTTP2_STREAM_DPRI_TOP, otherwise 0.
+ * value of NGHTTP2_STREAM_DPRI_TOP, otherwise 0.
+ *
+ * Calculating stream->sum_top-weight is very simple compared to
+ * stream->sum_norest_weight.  It just adds up the weight of direct
+ * descendants whose dpri is NGHTTP2_STREAM_DPRI_TOP.
  */
 static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream)
 {
@@ -272,6 +317,7 @@ static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream)
   int rv;
 
   stream->sum_norest_weight = 0;
+  stream->sum_top_weight = 0;
 
   if(stream->dpri == NGHTTP2_STREAM_DPRI_TOP) {
     return 1;
@@ -289,6 +335,10 @@ static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream)
       rv = 1;
       stream->sum_norest_weight += si->weight;
     }
+
+    if(si->dpri == NGHTTP2_STREAM_DPRI_TOP) {
+      stream->sum_top_weight += si->weight;
+    }
   }
 
   return rv;

lib/nghttp2_stream.h

@@ -172,6 +172,9 @@ struct nghttp2_stream {
      descendant with dpri == NGHTTP2_STREAM_DPRI_TOP.  We use this
      value to calculate effective weight. */
   int32_t sum_norest_weight;
+  /* sum of weight of direct descendants whose dpri value is
+     NGHTTP2_STREAM_DPRI_TOP */
+  int32_t sum_top_weight;
   nghttp2_stream_state state;
   /* This is bitwise-OR of 0 or more of nghttp2_stream_flag. */
   uint8_t flags;

tests/nghttp2_session_test.c

@@ -6080,10 +6080,9 @@ void test_nghttp2_session_stream_attach_data(void)
   CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == a->dpri);
   CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == b->dpri);
   CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == c->dpri);
-  CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == d->dpri);
+  CU_ASSERT(NGHTTP2_STREAM_DPRI_REST == d->dpri);
 
-  CU_ASSERT(16 * 16 / 32 == b->effective_weight);
+  CU_ASSERT(16 * 16 / 16 == b->effective_weight);
-  CU_ASSERT(16 * 16 / 32 == d->effective_weight);
 
   CU_ASSERT(1 == dd->queued);
 
@@ -6240,13 +6239,12 @@ void test_nghttp2_session_stream_attach_data_subtree(void)
   CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == a->dpri);
   CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == b->dpri);
   CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == c->dpri);
-  CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == d->dpri);
+  CU_ASSERT(NGHTTP2_STREAM_DPRI_REST == d->dpri);
   CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == e->dpri);
   CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == f->dpri);
 
   CU_ASSERT(16 == b->effective_weight);
-  CU_ASSERT(16 * 16 / 32 == e->effective_weight);
+  CU_ASSERT(16 * 16 / 16 == e->effective_weight);
-  CU_ASSERT(16 * 16 / 32 == d->effective_weight);
 
   CU_ASSERT(32 == a->sum_norest_weight);
   CU_ASSERT(16 == c->sum_norest_weight);