Skip to content

Commit

Permalink
Improve performance of dependency management - avoid linear search.
Browse files Browse the repository at this point in the history
  • Loading branch information
ioquatix committed Nov 9, 2024
1 parent 191c31f commit 0b3bdc4
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 69 deletions.
41 changes: 34 additions & 7 deletions lib/protocol/http2/dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ def <=> other
@weight <=> other.weight
end

def == other
@id == other.id
end

# The connection this stream belongs to.
attr :connection

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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

Expand Down
145 changes: 83 additions & 62 deletions test/protocol/http2/dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand All @@ -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]}
Expand All @@ -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}
Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -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

0 comments on commit 0b3bdc4

Please sign in to comment.