Push stream to queue after effective weight was calculated

lib/nghttp2_stream.c

@@ -216,13 +216,22 @@ static void stream_update_dep_effective_weight(nghttp2_stream *stream)
       si->effective_weight = stream_dep_distributed_top_effective_weight
         (stream, si->weight);
 
+      DEBUGF(fprintf(stderr, "stream: stream=%d top eweight=%d\n",
+                     si->stream_id, si->effective_weight));
+
       continue;
     }
 
     if(si->dpri == NGHTTP2_STREAM_DPRI_NO_DATA) {
+      DEBUGF(fprintf(stderr, "stream: stream=%d no_data, ignored\n",
+                     si->stream_id));
+
       /* 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);
+    } else {
+      DEBUGF(fprintf(stderr, "stream: stream=%d rest, ignored\n",
+                     si->stream_id));
     }
   }
 }
@@ -233,6 +242,8 @@ static void stream_update_dep_set_rest(nghttp2_stream *stream)
     return;
   }
 
+  DEBUGF(fprintf(stderr, "stream: stream=%d is rest\n", stream->stream_id));
+
   if(stream->dpri == NGHTTP2_STREAM_DPRI_REST) {
     return;
   }
@@ -251,7 +262,33 @@ static void stream_update_dep_set_rest(nghttp2_stream *stream)
 
 /*
  * Performs dfs starting |stream|, search stream which can become
- * NGHTTP2_STREAM_DPRI_TOP and queues its data_item.
+ * NGHTTP2_STREAM_DPRI_TOP and set its dpri.
+ */
+static void stream_update_dep_set_top(nghttp2_stream *stream)
+{
+  nghttp2_stream *si;
+
+  if(stream->dpri == NGHTTP2_STREAM_DPRI_TOP) {
+    return;
+  }
+
+  if(stream->dpri == NGHTTP2_STREAM_DPRI_REST) {
+    DEBUGF(fprintf(stderr, "stream: stream=%d data is top\n",
+                   stream->stream_id));
+
+    stream->dpri = NGHTTP2_STREAM_DPRI_TOP;
+
+    return;
+  }
+
+  for(si = stream->dep_next; si; si = si->sib_next) {
+    stream_update_dep_set_top(si);
+  }
+}
+
+/*
+ * Performs dfs starting |stream|, and dueue stream whose dpri is
+ * NGHTTP2_STREAM_DPRI_TOP and has not been queued yet.
  *
  * This function returns 0 if it succeeds, or one of the following
  * negative error codes:
@@ -259,21 +296,20 @@ static void stream_update_dep_set_rest(nghttp2_stream *stream)
  * NGHTTP2_ERR_NOMEM
  *     Out of memory.
  */
-static int stream_update_dep_set_top(nghttp2_stream *stream, nghttp2_pq *pq,
+static int stream_update_dep_queue_top(nghttp2_stream *stream, nghttp2_pq *pq,
-                                     uint64_t cycle)
+                                       uint64_t cycle)
 {
   int rv;
   nghttp2_stream *si;
 
-  if(stream->dpri == NGHTTP2_STREAM_DPRI_TOP) {
+  if(stream->dpri == NGHTTP2_STREAM_DPRI_REST) {
     return 0;
   }
 
-  if(stream->dpri == NGHTTP2_STREAM_DPRI_REST) {
+  if(stream->dpri == NGHTTP2_STREAM_DPRI_TOP) {
-    DEBUGF(fprintf(stderr, "stream: stream=%d data is top\n",
-                   stream->stream_id));
-
     if(!stream->data_item->queued) {
+      DEBUGF(fprintf(stderr, "stream: stream=%d enqueue\n",
+                     stream->stream_id));
       rv = stream_push_data(stream, pq, cycle);
 
       if(rv != 0) {
@@ -281,13 +317,11 @@ static int stream_update_dep_set_top(nghttp2_stream *stream, nghttp2_pq *pq,
       }
     }
 
-    stream->dpri = NGHTTP2_STREAM_DPRI_TOP;
-
     return 0;
   }
 
   for(si = stream->dep_next; si; si = si->sib_next) {
-    rv = stream_update_dep_set_top(si, pq, cycle);
+    rv = stream_update_dep_queue_top(si, pq, cycle);
 
     if(rv != 0) {
       return rv;
@@ -347,7 +381,6 @@ static int stream_update_dep_sum_norest_weight(nghttp2_stream *stream)
 static int stream_update_dep_on_attach_data(nghttp2_stream *stream,
                                             nghttp2_pq *pq, uint64_t cycle)
 {
-  int rv;
   nghttp2_stream *root_stream;
 
   stream->dpri = NGHTTP2_STREAM_DPRI_REST;
@@ -358,38 +391,29 @@ static int stream_update_dep_on_attach_data(nghttp2_stream *stream,
 
   DEBUGF(fprintf(stderr, "root=%p, stream=%p\n", root_stream, stream));
 
-  rv = stream_update_dep_set_top(root_stream, pq, cycle);
+  stream_update_dep_set_top(root_stream);
-
-  if(rv != 0) {
-    return rv;
-  }
 
   stream_update_dep_sum_norest_weight(root_stream);
   stream_update_dep_effective_weight(root_stream);
 
-  return 0;
+  return stream_update_dep_queue_top(root_stream, pq, cycle);
 }
 
 static int stream_update_dep_on_detach_data(nghttp2_stream *stream,
                                             nghttp2_pq *pq, uint64_t cycle)
 {
-  int rv;
   nghttp2_stream *root_stream;
 
   stream->dpri = NGHTTP2_STREAM_DPRI_NO_DATA;
 
   root_stream = nghttp2_stream_get_dep_root(stream);
 
-  rv = stream_update_dep_set_top(root_stream, pq, cycle);
+  stream_update_dep_set_top(root_stream);
-
-  if(rv != 0) {
-    return rv;
-  }
 
   stream_update_dep_sum_norest_weight(root_stream);
   stream_update_dep_effective_weight(root_stream);
 
-  return 0;
+  return stream_update_dep_queue_top(root_stream, pq, cycle);
 }
 
 int nghttp2_stream_attach_data(nghttp2_stream *stream,
@@ -731,7 +755,6 @@ int nghttp2_stream_dep_insert_subtree(nghttp2_stream *dep_stream,
   nghttp2_stream *dep_next;
   nghttp2_stream *root_stream;
   size_t delta_substreams;
-  int rv;
 
   DEBUGF(fprintf(stderr, "stream: dep_insert_subtree dep_stream(%p)=%d "
                  "stream(%p)=%d\n",
@@ -777,16 +800,12 @@ int nghttp2_stream_dep_insert_subtree(nghttp2_stream *dep_stream,
 
   root_stream = stream_update_dep_length(dep_stream, delta_substreams);
 
-  rv = stream_update_dep_set_top(root_stream, pq, cycle);
+  stream_update_dep_set_top(root_stream);
-
-  if(rv != 0) {
-    return rv;
-  }
 
   stream_update_dep_sum_norest_weight(root_stream);
   stream_update_dep_effective_weight(root_stream);
 
-  return 0;
+  return stream_update_dep_queue_top(root_stream, pq, cycle);
 }
 
 int nghttp2_stream_dep_add_subtree(nghttp2_stream *dep_stream,
@@ -796,7 +815,6 @@ int nghttp2_stream_dep_add_subtree(nghttp2_stream *dep_stream,
 {
   nghttp2_stream *last_sib;
   nghttp2_stream *root_stream;
-  int rv;
 
   DEBUGF(fprintf(stderr, "stream: dep_add_subtree dep_stream(%p)=%d "
                  "stream(%p)=%d\n",
@@ -822,16 +840,12 @@ int nghttp2_stream_dep_add_subtree(nghttp2_stream *dep_stream,
 
   root_stream = stream_update_dep_length(dep_stream, stream->num_substreams);
 
-  rv = stream_update_dep_set_top(root_stream, pq, cycle);
+  stream_update_dep_set_top(root_stream);
-
-  if(rv != 0) {
-    return rv;
-  }
 
   stream_update_dep_sum_norest_weight(root_stream);
   stream_update_dep_effective_weight(root_stream);
 
-  return 0;
+  return stream_update_dep_queue_top(root_stream, pq, cycle);
 }
 
 void nghttp2_stream_dep_remove_subtree(nghttp2_stream *stream)
@@ -888,8 +902,6 @@ void nghttp2_stream_dep_remove_subtree(nghttp2_stream *stream)
 int nghttp2_stream_dep_make_root(nghttp2_stream *stream, nghttp2_pq *pq,
                                  uint64_t cycle)
 {
-  int rv;
-
   DEBUGF(fprintf(stderr, "stream: dep_make_root stream(%p)=%d\n",
                  stream, stream->stream_id));
 
@@ -899,16 +911,12 @@ int nghttp2_stream_dep_make_root(nghttp2_stream *stream, nghttp2_pq *pq,
 
   stream->effective_weight = stream->weight;
 
-  rv = stream_update_dep_set_top(stream, pq, cycle);
+  stream_update_dep_set_top(stream);
-
-  if(rv != 0) {
-    return rv;
-  }
 
   stream_update_dep_sum_norest_weight(stream);
   stream_update_dep_effective_weight(stream);
 
-  return 0;
+  return stream_update_dep_queue_top(stream, pq, cycle);
 }
 
 int nghttp2_stream_dep_all_your_stream_are_belong_to_us

tests/nghttp2_session_test.c

@@ -6084,6 +6084,17 @@ void test_nghttp2_session_stream_attach_data(void)
 
   CU_ASSERT(16 * 16 / 16 == b->effective_weight);
 
+  CU_ASSERT(0 == dd->queued);
+
+  nghttp2_stream_detach_data(b, &session->ob_da_pq, session->last_cycle);
+
+  CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == a->dpri);
+  CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == b->dpri);
+  CU_ASSERT(NGHTTP2_STREAM_DPRI_NO_DATA == c->dpri);
+  CU_ASSERT(NGHTTP2_STREAM_DPRI_TOP == d->dpri);
+
+  CU_ASSERT(16 * 16 / 16 == d->effective_weight);
+
   CU_ASSERT(1 == dd->queued);
 
   nghttp2_session_del(session);