Skip to content

Commit 9487482

Browse files
AndrewOrichmolj
authored andcommitted
Index children arrays to remove quadratic time complexity (#135)
After profiling a call, I saw that more than half of the time was spent in a single `has_one` sideload. The existing code has a couple of cases of nested loops which were at the root of this. Replacing the inner loops with Hash lookups caused my request time to go from >3.5s to ~1s (at which point, a lot of that is dependent on latency from upstream resources and Rails itself).
1 parent 988e616 commit 9487482

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

lib/jsonapi_compliable/adapters/active_record_sideloading.rb

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ def has_many(association_name, scope: nil, resource:, foreign_key:, primary_key:
1111
end
1212

1313
assign do |parents, children|
14+
children_hash = children.group_by(&foreign_key)
1415
parents.each do |parent|
1516
parent.association(association_name).loaded!
16-
relevant_children = children.select { |c| c.send(foreign_key) == parent.send(primary_key) }
17+
relevant_children = children_hash[parent.send(primary_key)] || []
1718
relevant_children.each do |c|
1819
parent.association(association_name).add_to_target(c, :skip_callbacks)
1920
end
@@ -34,8 +35,9 @@ def belongs_to(association_name, scope: nil, resource:, foreign_key:, primary_ke
3435
end
3536

3637
assign do |parents, children|
38+
children_hash = children.index_by(&primary_key)
3739
parents.each do |parent|
38-
relevant_child = children.find { |c| parent.send(foreign_key) == c.send(primary_key) }
40+
relevant_child = children_hash[parent.send(foreign_key)]
3941
parent.send(:"#{association_name}=", relevant_child)
4042
end
4143
end
@@ -62,10 +64,12 @@ def has_one(association_name, scope: nil, resource:, foreign_key:, primary_key:
6264
# remove anything else. This is more or less what AR does.
6365
assign do |parents, children|
6466
assigned = []
67+
children_hash = children.group_by(&foreign_key)
6568
parents.each do |parent|
6669
parent.association(association_name).loaded!
67-
relevant_child = children.find { |c| c.send(foreign_key) == parent.send(primary_key) }
68-
next unless relevant_child
70+
relevant_children = children_hash[parent.send(primary_key)]
71+
next unless relevant_children
72+
relevant_child = relevant_children.first
6973

7074
# Use private methods because of Rails bug
7175
# https://github.com/rails/rails/issues/32886
@@ -75,9 +79,7 @@ def has_one(association_name, scope: nil, resource:, foreign_key:, primary_key:
7579
association.send(:target=, relevant_child)
7680
assigned << relevant_child
7781
end
78-
(children - assigned).each do |unassigned|
79-
children.delete(unassigned)
80-
end
82+
children.replace(assigned)
8183
end
8284

8385
instance_eval(&blk) if blk

0 commit comments

Comments
 (0)