Improve performance of dependency management - avoid linear search.

Author: ioquatix

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
 

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,36 +157,32 @@ 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]}
 
 		#    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
 				#<Protocol::HTTP2::Dependency id=5 parent id=1 weight=16 0 children>
 		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