Distribute effective weight among only streams with marked as top

If stream with dpri value of no_data, we check any its descendant has
stream with dpri value of top.  If so, we have to distribute of its
portion of weight to its descendants.

lib/nghttp2_stream.c

@@ -62,6 +62,7 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id,
   stream->weight = weight;
   stream->effective_weight = stream->weight;
   stream->sum_dep_weight = 0;
+  stream->sum_norest_weight = 0;
 
   stream->roots = roots;
   stream->root_prev = NULL;
@@ -144,7 +145,7 @@ int32_t nghttp2_stream_dep_distributed_weight(nghttp2_stream *stream,
 int32_t nghttp2_stream_dep_distributed_effective_weight
 (nghttp2_stream *stream, int32_t weight)
 {
-  weight = stream->effective_weight * weight / stream->sum_dep_weight;
+  weight = stream->effective_weight * weight / stream->sum_norest_weight;
 
   return nghttp2_max(1, weight);
 }
@@ -157,12 +158,22 @@ 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\n",
-                 stream, stream->stream_id, stream->weight));
+                 "stream(%p)=%d, weight=%d, sum_norest_weight=%d\n",
+                 stream, stream->stream_id, stream->weight,
+                 stream->sum_norest_weight));
+
+  /* stream->sum_norest_weight == 0 means there is no
+     NGHTTP2_STREAM_DPRI_TOP under stream */
+  if(stream->dpri != NGHTTP2_STREAM_DPRI_NO_DATA ||
+     stream->sum_norest_weight == 0) {
+    return;
+  }
 
   for(si = stream->dep_next; si; si = si->sib_next) {
-    si->effective_weight = nghttp2_stream_dep_distributed_effective_weight
-      (stream, si->weight);
+    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);
   }
@@ -203,23 +214,13 @@ static void stream_update_dep_set_rest(nghttp2_stream *stream)
 static int stream_update_dep_set_top(nghttp2_stream *stream, nghttp2_pq *pq)
 {
   int rv;
-
-  if(stream == NULL) {
-    return 0;
-  }
+  nghttp2_stream *si;
 
   if(stream->dpri == NGHTTP2_STREAM_DPRI_TOP) {
-    rv = stream_update_dep_set_top(stream->sib_next, pq);
-
-    if(rv != 0) {
-      return rv;
-    }
-
     return 0;
   }
 
   if(stream->dpri == NGHTTP2_STREAM_DPRI_REST) {
-
     DEBUGF(fprintf(stderr, "stream: stream=%d data is top\n",
                    stream->stream_id));
 
@@ -233,30 +234,56 @@ static int stream_update_dep_set_top(nghttp2_stream *stream, nghttp2_pq *pq)
 
     stream->dpri = NGHTTP2_STREAM_DPRI_TOP;
 
-    rv = stream_update_dep_set_top(stream->sib_next, pq);
+    return 0;
+  }
+
+  for(si = stream->dep_next; si; si = si->sib_next) {
+    rv = stream_update_dep_set_top(si, pq);
 
     if(rv != 0) {
       return rv;
     }
-
-    return 0;
   }
 
-  assert(stream->dpri == NGHTTP2_STREAM_DPRI_NO_DATA);
+  return 0;
+}
 
-  rv = stream_update_dep_set_top(stream->sib_next, pq);
+/*
+ * Updates stream->sum_norest_weight recursively.  We have to gather
+ * effective sum of weight of descendants.  If stream->dpri ==
+ * NGHTTP2_STREAM_DPRI_NO_DATA, we have to go deeper and check that
+ * any of its descendants has dpri value of NGHTTP2_STREAM_DPRI_TOP.
+ * If so, we have to add weight of its direct descendants to
+ * stream->sum_norest_weight.  To make this work, this function
+ * returns 1 if any of its descendants has dpri value of
+ * NGHTTP2_STREAM_DPRI_TOP, otherwise 0.
+ */
+static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream)
+{
+  nghttp2_stream *si;
+  int rv;
 
-  if(rv != 0) {
-    return rv;
+  stream->sum_norest_weight = 0;
+
+  if(stream->dpri == NGHTTP2_STREAM_DPRI_TOP) {
+    return 1;
+  }
+
+  if(stream->dpri == NGHTTP2_STREAM_DPRI_REST) {
+    return 0;
   }
 
-  rv = stream_update_dep_set_top(stream->dep_next, pq);
+  rv = 0;
 
-  if(rv != 0) {
-    return rv;
+  for(si = stream->dep_next; si; si = si->sib_next) {
+
+    if(stream_update_dep_sum_norest_weight(si)) {
+      rv = 1;
+      stream->sum_norest_weight += si->weight;
+    }
   }
 
-  return 0;
+  return rv;
 }
 
 static int stream_update_dep_on_attach_data(nghttp2_stream *stream,
@@ -279,6 +306,9 @@ static int stream_update_dep_on_attach_data(nghttp2_stream *stream,
     return rv;
   }
 
+  stream_update_dep_sum_norest_weight(root_stream);
+  stream_update_dep_effective_weight(root_stream);
+
   return 0;
 }
 
@@ -288,12 +318,6 @@ static int stream_update_dep_on_detach_data(nghttp2_stream *stream,
   int rv;
   nghttp2_stream *root_stream;
 
-  if(stream->dpri != NGHTTP2_STREAM_DPRI_TOP) {
-    stream->dpri = NGHTTP2_STREAM_DPRI_NO_DATA;
-
-    return 0;
-  }
-
   stream->dpri = NGHTTP2_STREAM_DPRI_NO_DATA;
 
   root_stream = nghttp2_stream_get_dep_root(stream);
@@ -304,6 +328,9 @@ static int stream_update_dep_on_detach_data(nghttp2_stream *stream,
     return rv;
   }
 
+  stream_update_dep_sum_norest_weight(root_stream);
+  stream_update_dep_effective_weight(root_stream);
+
   return 0;
 }
 
@@ -454,6 +481,7 @@ void nghttp2_stream_dep_insert(nghttp2_stream *dep_stream,
                                nghttp2_stream *stream)
 {
   nghttp2_stream *si;
+  nghttp2_stream *root_stream;
 
   assert(stream->data_item == NULL);
 
@@ -477,8 +505,10 @@ void nghttp2_stream_dep_insert(nghttp2_stream *dep_stream,
   dep_stream->dep_next = stream;
   stream->dep_prev = dep_stream;
 
-  stream_update_dep_length(dep_stream, 1);
-  stream_update_dep_effective_weight(dep_stream);
+  root_stream = stream_update_dep_length(dep_stream, 1);
+
+  stream_update_dep_sum_norest_weight(root_stream);
+  stream_update_dep_effective_weight(root_stream);
 
   ++stream->roots->num_streams;
 }
@@ -487,6 +517,7 @@ void nghttp2_stream_dep_add(nghttp2_stream *dep_stream,
                             nghttp2_stream *stream)
 {
   nghttp2_stream *last_sib;
+  nghttp2_stream *root_stream;
 
   assert(stream->data_item == NULL);
 
@@ -495,7 +526,7 @@ void nghttp2_stream_dep_add(nghttp2_stream *dep_stream,
                  dep_stream, dep_stream->stream_id,
                  stream, stream->stream_id));
 
-  stream_update_dep_length(dep_stream, 1);
+  root_stream = stream_update_dep_length(dep_stream, 1);
 
   dep_stream->sum_dep_weight += stream->weight;
 
@@ -508,16 +539,19 @@ void nghttp2_stream_dep_add(nghttp2_stream *dep_stream,
     stream->sib_prev = last_sib;
   }
 
-  stream_update_dep_effective_weight(dep_stream);
+  stream_update_dep_sum_norest_weight(root_stream);
+  stream_update_dep_effective_weight(root_stream);
 
   ++stream->roots->num_streams;
 }
 
 void nghttp2_stream_dep_remove(nghttp2_stream *stream)
 {
-  nghttp2_stream *prev, *next, *dep_next, *dep_prev, *si;
+  nghttp2_stream *prev, *next, *dep_next, *dep_prev, *si, *root_stream;
   int32_t sum_dep_weight_delta;
 
+  root_stream = NULL;
+
   DEBUGF(fprintf(stderr, "stream: dep_remove stream(%p)=%d\n",
                  stream, stream->stream_id));
 
@@ -535,7 +569,7 @@ void nghttp2_stream_dep_remove(nghttp2_stream *stream)
   dep_prev = prev->dep_prev;
 
   if(dep_prev) {
-    stream_update_dep_length(dep_prev, -1);
+    root_stream = stream_update_dep_length(dep_prev, -1);
 
     dep_prev->sum_dep_weight += sum_dep_weight_delta;
   }
@@ -608,8 +642,9 @@ void nghttp2_stream_dep_remove(nghttp2_stream *stream)
     next->sib_prev = prev;
   }
 
-  if(dep_prev) {
-    stream_update_dep_effective_weight(dep_prev);
+  if(root_stream) {
+    stream_update_dep_sum_norest_weight(root_stream);
+    stream_update_dep_effective_weight(root_stream);
   }
 
   stream->num_substreams = 1;
@@ -675,8 +710,6 @@ int nghttp2_stream_dep_insert_subtree(nghttp2_stream *dep_stream,
     dep_stream->sum_dep_weight = stream->weight;
   }
 
-  stream_update_dep_effective_weight(dep_stream);
-
   root_stream = stream_update_dep_length(dep_stream, delta_substreams);
 
   rv = stream_update_dep_set_top(root_stream, pq);
@@ -685,6 +718,9 @@ int nghttp2_stream_dep_insert_subtree(nghttp2_stream *dep_stream,
     return rv;
   }
 
+  stream_update_dep_sum_norest_weight(root_stream);
+  stream_update_dep_effective_weight(root_stream);
+
   return 0;
 }
 
@@ -718,8 +754,6 @@ int nghttp2_stream_dep_add_subtree(nghttp2_stream *dep_stream,
     dep_stream->sum_dep_weight = stream->weight;
   }
 
-  stream_update_dep_effective_weight(dep_stream);
-
   root_stream = stream_update_dep_length(dep_stream, stream->num_substreams);
 
   rv = stream_update_dep_set_top(root_stream, pq);
@@ -728,12 +762,15 @@ int nghttp2_stream_dep_add_subtree(nghttp2_stream *dep_stream,
     return rv;
   }
 
+  stream_update_dep_sum_norest_weight(root_stream);
+  stream_update_dep_effective_weight(root_stream);
+
   return 0;
 }
 
 void nghttp2_stream_dep_remove_subtree(nghttp2_stream *stream)
 {
-  nghttp2_stream *prev, *next, *dep_prev;
+  nghttp2_stream *prev, *next, *dep_prev, *root_stream;
 
   DEBUGF(fprintf(stderr, "stream: dep_remove_subtree stream(%p)=%d\n",
                  stream, stream->stream_id));
@@ -771,9 +808,10 @@ void nghttp2_stream_dep_remove_subtree(nghttp2_stream *stream)
   if(dep_prev) {
     dep_prev->sum_dep_weight -= stream->weight;
 
-    stream_update_dep_effective_weight(dep_prev);
+    root_stream = stream_update_dep_length(dep_prev, -stream->num_substreams);
 
-    stream_update_dep_length(dep_prev, -stream->num_substreams);
+    stream_update_dep_sum_norest_weight(root_stream);
+    stream_update_dep_effective_weight(root_stream);
   }
 
   stream->sib_prev = NULL;
@@ -794,14 +832,15 @@ int nghttp2_stream_dep_make_root(nghttp2_stream *stream, nghttp2_pq *pq)
 
   stream->effective_weight = stream->weight;
 
-  stream_update_dep_effective_weight(stream);
-
   rv = stream_update_dep_set_top(stream, pq);
 
   if(rv != 0) {
     return rv;
   }
 
+  stream_update_dep_sum_norest_weight(stream);
+  stream_update_dep_effective_weight(stream);
+
   return 0;
 }
 

lib/nghttp2_stream.h

@@ -164,6 +164,10 @@ struct nghttp2_stream {
   int32_t effective_weight;
   /* sum of weight (not effective_weight) of direct descendants */
   int32_t sum_dep_weight;
+  /* sum of weight of direct descendants which have at least one
+     descendant with dpri == NGHTTP2_STREAM_DPRI_TOP.  We use this
+     value to calculate effective weight. */
+  int32_t sum_norest_weight;
   nghttp2_stream_state state;
   /* This is bitwise-OR of 0 or more of nghttp2_stream_flag. */
   uint8_t flags;

tests/main.c

@@ -228,8 +228,6 @@ int main(int argc, char* argv[])
                    test_nghttp2_session_stream_attach_data) ||
       !CU_add_test(pSuite, "session_stream_attach_data_subtree",
                    test_nghttp2_session_stream_attach_data_subtree) ||
-      !CU_add_test(pSuite, "session_stream_dep_effective_weight",
-                   test_nghttp2_session_stream_dep_effective_weight) ||
       !CU_add_test(pSuite, "session_stream_keep_closed_stream",
                    test_nghttp2_session_keep_closed_stream) ||
       !CU_add_test(pSuite, "frame_pack_headers",

tests/nghttp2_session_test.h

@@ -106,7 +106,6 @@ void test_nghttp2_session_stream_dep_remove_subtree(void);
 void test_nghttp2_session_stream_dep_all_your_stream_are_belong_to_us(void);
 void test_nghttp2_session_stream_attach_data(void);
 void test_nghttp2_session_stream_attach_data_subtree(void);
-void test_nghttp2_session_stream_dep_effective_weight(void);
 void test_nghttp2_session_keep_closed_stream(void);
 
 #endif /* NGHTTP2_SESSION_TEST_H */

tests/nghttp2_test_helper.c

@@ -232,11 +232,22 @@ void bufs_large_init(nghttp2_bufs *bufs, size_t chunk_size)
   nghttp2_bufs_init2(bufs, chunk_size, 16, NGHTTP2_FRAME_HDLEN + 2);
 }
 
-nghttp2_stream* open_stream(nghttp2_session *session, int32_t stream_id)
+static nghttp2_stream* open_stream_with_all(nghttp2_session *session,
+                                            int32_t stream_id,
+                                            int32_t weight,
+                                            uint8_t exclusive,
+                                            nghttp2_stream *dep_stream)
 {
   nghttp2_priority_spec pri_spec;
+  int32_t dep_stream_id;
 
-  nghttp2_priority_spec_default_init(&pri_spec);
+  if(dep_stream) {
+    dep_stream_id = dep_stream->stream_id;
+  } else {
+    dep_stream_id = 0;
+  }
+
+  nghttp2_priority_spec_init(&pri_spec, dep_stream_id, weight, exclusive);
 
   return nghttp2_session_open_stream(session, stream_id,
                                      NGHTTP2_STREAM_FLAG_NONE,
@@ -245,12 +256,19 @@ nghttp2_stream* open_stream(nghttp2_session *session, int32_t stream_id)
                                      NULL);
 }
 
+
+nghttp2_stream* open_stream(nghttp2_session *session, int32_t stream_id)
+{
+  return open_stream_with_all(session, stream_id, NGHTTP2_DEFAULT_WEIGHT, 0,
+                              NULL);
+}
+
 nghttp2_stream* open_stream_with_dep(nghttp2_session *session,
                                      int32_t stream_id,
                                      nghttp2_stream *dep_stream)
 {
-  return open_stream_with_dep_weight(session, stream_id,
-                                     NGHTTP2_DEFAULT_WEIGHT, dep_stream);
+  return open_stream_with_all(session, stream_id, NGHTTP2_DEFAULT_WEIGHT, 0,
+                              dep_stream);
 }
 
 nghttp2_stream* open_stream_with_dep_weight(nghttp2_session *session,
@@ -258,29 +276,25 @@ nghttp2_stream* open_stream_with_dep_weight(nghttp2_session *session,
                                             int32_t weight,
                                             nghttp2_stream *dep_stream)
 {
-  nghttp2_priority_spec pri_spec;
-
-  nghttp2_priority_spec_init(&pri_spec, dep_stream->stream_id, weight, 0);
-
-  return nghttp2_session_open_stream(session, stream_id,
-                                     NGHTTP2_STREAM_FLAG_NONE,
-                                     &pri_spec,
-                                     NGHTTP2_STREAM_OPENED,
-                                     NULL);
+  return open_stream_with_all(session, stream_id, weight, 0, dep_stream);
 }
 
 nghttp2_stream* open_stream_with_dep_excl(nghttp2_session *session,
                                           int32_t stream_id,
                                           nghttp2_stream *dep_stream)
 {
-  nghttp2_priority_spec pri_spec;
+  return open_stream_with_all(session, stream_id, NGHTTP2_DEFAULT_WEIGHT, 1,
+                              dep_stream);
+}
 
-  nghttp2_priority_spec_init(&pri_spec, dep_stream->stream_id,
-                             NGHTTP2_DEFAULT_WEIGHT, 1);
+nghttp2_outbound_item* create_data_ob_item(void)
+{
+  nghttp2_outbound_item *item;
 
-  return nghttp2_session_open_stream(session, stream_id,
-                                     NGHTTP2_STREAM_FLAG_NONE,
-                                     &pri_spec,
-                                     NGHTTP2_STREAM_OPENED,
-                                     NULL);
+  item = malloc(sizeof(nghttp2_outbound_item));
+  memset(item, 0, sizeof(nghttp2_outbound_item));
+  item->frame_cat = NGHTTP2_CAT_DATA;
+
+  return item;
 }
+

tests/nghttp2_test_helper.h

@@ -98,4 +98,6 @@ nghttp2_stream* open_stream_with_dep_excl(nghttp2_session *session,
                                           int32_t stream_id,
                                           nghttp2_stream *dep_stream);
 
+nghttp2_outbound_item* create_data_ob_item(void);
+
 #endif /* NGHTTP2_TEST_HELPER_H */