From 2cadcb2718c6c26921e5d070f2610196b4d36bc5 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 2 May 2024 14:36:34 +0200 Subject: [PATCH] Don't cast `stale_state` to String I'm looking at reducing the allocations and memory footprint of Active Record models, and based on a small benchmark I saw a lot of numeric strings being created from `stale_state`. I tracked this cast all the way down to 1c07b84df95e932d50376c1d0a13585b2e2ef868 but unfortunately the commit message doesn't explain why it was added. I can't think of a reason why this would be needed. The benchmark is essentially just: `Post.includes(:comments).to_a` with `100` posts and `20` comments each. ``` Total allocated: 4.69 MB (45077 objects) Total retained: 3.84 MB (29623 objects) retained objects by location ----------------------------------- ... 2000 activerecord/lib/active_record/associations/belongs_to_association.rb:152 retained memory by location ----------------------------------- ... 80.00 kB activerecord/lib/active_record/associations/belongs_to_association.rb:152 ``` NB: This break the final assertion of the Rails 6.1 Marshal backward compatibility test, but I think it's an acceptable tradeoff. --- .../lib/active_record/associations/belongs_to_association.rb | 3 +-- .../associations/belongs_to_polymorphic_association.rb | 5 +++-- .../lib/active_record/associations/through_association.rb | 2 +- activerecord/test/cases/marshal_serialization_test.rb | 1 - 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index cf87d308cf4b9..a38d2f8d4807d 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -148,8 +148,7 @@ def invertible_for?(record) end def stale_state - result = owner._read_attribute(reflection.foreign_key) { |n| owner.send(:missing_attribute, n, caller) } - result && result.to_s + owner._read_attribute(reflection.foreign_key) { |n| owner.send(:missing_attribute, n, caller) } end end end diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index 298fa1011e5a7..a6e5ee3b03c10 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -41,8 +41,9 @@ def raise_on_type_mismatch!(record) end def stale_state - foreign_key = super - foreign_key && [foreign_key.to_s, owner[reflection.foreign_type].to_s] + if foreign_key = super + [foreign_key, owner[reflection.foreign_type]] + end end end end diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index e3680bac9390c..7389ecc787a59 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -82,7 +82,7 @@ def construct_join_attributes(*records) def stale_state if through_reflection.belongs_to? Array(through_reflection.foreign_key).filter_map do |foreign_key_column| - owner[foreign_key_column] && owner[foreign_key_column].to_s + owner[foreign_key_column] end.presence end end diff --git a/activerecord/test/cases/marshal_serialization_test.rb b/activerecord/test/cases/marshal_serialization_test.rb index c72f0efb3de7f..70cd9d1456795 100644 --- a/activerecord/test/cases/marshal_serialization_test.rb +++ b/activerecord/test/cases/marshal_serialization_test.rb @@ -33,7 +33,6 @@ def test_deserializing_rails_6_1_marshal_with_loaded_association_cache assert_equal "Have a nice day", topic.content assert_predicate topic.association(:replies), :loaded? assert_predicate topic.replies.first.association(:topic), :loaded? - assert_same topic, topic.replies.first.topic end def test_deserializing_rails_7_1_marshal_basic