diff --git a/lib/protocol/http2/dependency.rb b/lib/protocol/http2/dependency.rb index 6752a36..3862424 100644 --- a/lib/protocol/http2/dependency.rb +++ b/lib/protocol/http2/dependency.rb @@ -58,6 +58,10 @@ def <=> other @weight <=> other.weight end + def == other + @id == other.id + end + # The connection this stream belongs to. attr :connection @@ -101,13 +105,30 @@ def add_child(dependency) dependency.parent = self - self.clear_cache! + if @ordered_children + # Binary search for insertion point: + index = @ordered_children.bsearch_index do |child| + child.weight >= dependency.weight + end + + if index + @ordered_children.insert(index, dependency) + else + @ordered_children.push(dependency) + end + + @total_weight += dependency.weight + end end def remove_child(dependency) @children&.delete(dependency.id) - self.clear_cache! + if @ordered_children + # Don't do a linear search here, it can be slow. Instead, the child's parent will be set to `nil`, and we check this in {#consume_window} using `delete_if`. + # @ordered_children.delete(dependency) + @total_weight -= dependency.weight + end end # An exclusive flag allows for the insertion of a new level of dependencies. The exclusive flag causes the stream to become the sole dependency of its parent stream, causing other dependencies to become dependent on the exclusive stream. @@ -195,11 +216,17 @@ def consume_window(size) end # Otherwise, allow the dependent children to use up the available window: - self.ordered_children&.each do |child| - # Compute the proportional allocation: - allocated = (child.weight * size) / @total_weight - - child.consume_window(allocated) if allocated > 0 + self.ordered_children&.delete_if do |child| + if child.parent + # Compute the proportional allocation: + allocated = (child.weight * size) / @total_weight + + child.consume_window(allocated) if allocated > 0 + + false + else + true + end end end diff --git a/test/protocol/http2/dependency.rb b/test/protocol/http2/dependency.rb index 9cce48c..0b28da8 100644 --- a/test/protocol/http2/dependency.rb +++ b/test/protocol/http2/dependency.rb @@ -48,16 +48,14 @@ def before # a # / # b - begin - a.priority = a.priority - server.read_frame - - priority = b.priority - priority.stream_dependency = a.id - b.priority = priority - - server.read_frame - end + a.priority = a.priority + server.read_frame + + priority = b.priority + priority.stream_dependency = a.id + b.priority = priority + + server.read_frame expect(a.dependency.children).to be == {b.id => b.dependency} @@ -68,13 +66,11 @@ def before # a # / \ # b c - begin - priority = c.priority - priority.stream_dependency = a.id - c.priority = priority - - server.read_frame - end + priority = c.priority + priority.stream_dependency = a.id + c.priority = priority + + server.read_frame expect(a.dependency.children).to be == {b.id => b.dependency, c.id => c.dependency} expect(server.dependencies[a.id].children).to be == {b.id => server.dependencies[b.id], c.id => server.dependencies[c.id]} @@ -84,14 +80,12 @@ def before # d # / \ # b c - begin - priority = d.priority - priority.stream_dependency = a.id - priority.exclusive = true - d.priority = priority - - server.read_frame - end + priority = d.priority + priority.stream_dependency = a.id + priority.exclusive = true + d.priority = priority + + server.read_frame expect(a.dependency.children).to be == {d.id => d.dependency} expect(d.dependency.children).to be == {b.id => b.dependency, c.id => c.dependency} @@ -163,25 +157,23 @@ def before # b c # | # d - begin - a.priority = a.priority - server.read_frame - - priority = b.priority - priority.stream_dependency = a.id - b.priority = priority - server.read_frame - - priority = c.priority - priority.stream_dependency = a.id - c.priority = priority - server.read_frame - - priority = d.priority - priority.stream_dependency = c.id - d.priority = priority - server.read_frame - end + a.priority = a.priority + server.read_frame + + priority = b.priority + priority.stream_dependency = a.id + b.priority = priority + server.read_frame + + priority = c.priority + priority.stream_dependency = a.id + c.priority = priority + server.read_frame + + priority = d.priority + priority.stream_dependency = c.id + d.priority = priority + server.read_frame expect(server.dependencies[a.id].children).to be == {b.id => server.dependencies[b.id], c.id => server.dependencies[c.id]} expect(server.dependencies[c.id].children).to be == {d.id => server.dependencies[d.id]} @@ -189,10 +181,8 @@ def before # a # / \ # b d - begin - c.send_reset_stream - server.read_frame - end + c.send_reset_stream + server.read_frame expect(server.dependencies[a.id].children).to be == {b.id => server.dependencies[b.id], d.id => server.dependencies[d.id]} expect(server.dependencies[c.id]).to be == nil @@ -205,20 +195,18 @@ def before # a # / \ # b c - begin - a.priority = a.priority - server.read_frame - - priority = b.priority - priority.stream_dependency = a.id - b.priority = priority - server.read_frame - - priority = c.priority - priority.stream_dependency = a.id - c.priority = priority - server.read_frame - end + a.priority = a.priority + server.read_frame + + priority = b.priority + priority.stream_dependency = a.id + b.priority = priority + server.read_frame + + priority = c.priority + priority.stream_dependency = a.id + c.priority = priority + server.read_frame a.dependency.print_hierarchy(buffer) @@ -228,4 +216,37 @@ def before # END end + + it "can insert several children" do + a, b, c, d = 4.times.collect {client.create_stream} + + b.dependency.weight = 10 + c.dependency.weight = 20 + d.dependency.weight = 30 + + # a __ + # / \ \ + # b c d + + a.priority = a.priority + server.read_frame + + priority = d.priority + priority.stream_dependency = a.id + d.priority = priority + server.read_frame + + priority = b.priority + priority.stream_dependency = a.id + b.priority = priority + server.read_frame + + priority = c.priority + priority.stream_dependency = a.id + c.priority = priority + server.read_frame + + ordered_children = server.dependencies[a.id].ordered_children + expect(ordered_children).to be == [b.dependency, c.dependency, d.dependency] + end end