Skip to content

Commit

Permalink
Reduce allocations in BelongsToAssociation#replace_keys
Browse files Browse the repository at this point in the history
Using the same benchmark as in #51726

`replace_keys` always allocate a multiple arrays to support composite
primary keys, even though most primary keys likely aren't composite.

And even when they are, we can avoid needless allocations by not using
`Array#zip`.

This reduce allocations by 18% (8k) on that benchmark.

Before:

```
Total allocated: 4.88 MB (44495 objects)
Total retained: 4.16 MB (32043 objects)

allocated memory by file
-----------------------------------
...
 320.00 kB  activerecord/lib/active_record/associations/belongs_to_association.rb

allocated objects by file
-----------------------------------
...
      8000  activerecord/lib/active_record/associations/belongs_to_association.rb
```

After:

```
Total allocated: 4.56 MB (36495 objects)
Total retained: 4.16 MB (32041 objects)
```

NB: `belongs_to_association` doesn't show up in top files anymore
  • Loading branch information
byroot committed May 7, 2024
1 parent dcbd6b0 commit 06968f8
Showing 1 changed file with 14 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,21 @@ def require_counter_update?
end

def replace_keys(record, force: false)
target_key_values = record ? Array(primary_key(record.class)).map { |key| record._read_attribute(key) } : []
reflection_fk = Array(reflection.foreign_key)
reflection_fk = reflection.foreign_key
if reflection_fk.is_a?(Array)
target_key_values = record ? Array(primary_key(record.class)).map { |key| record._read_attribute(key) } : []
reflection_fk = Array(reflection.foreign_key)

if force || reflection_fk.map { |fk| owner._read_attribute(fk) } != target_key_values
reflection_fk.each_with_index do |key, index|
owner[key] = target_key_values[index]
end
end
else
target_key_value = record ? record._read_attribute(primary_key(record.class)) : nil

if force || reflection_fk.map { |fk| owner._read_attribute(fk) } != target_key_values
reflection_fk.zip(target_key_values).each do |key, value|
owner[key] = value
if force || owner._read_attribute(reflection_fk) != target_key_value
owner[reflection_fk] = target_key_value
end
end
end
Expand Down

0 comments on commit 06968f8

Please sign in to comment.