-
Kevin Dew authored
In https://github.com/rails/rails/pull/43036 an optimisation was applied to ActiveModel#Serialization to speed up the generation of a serialized_hash by avoiding loading the subjects attributes by using an attribute_names method. A fallback method, ActiveModel::Serialization#attribute_names` was added as #attribute_names isn't part of the public API of ActiveModel. Unfortunately, this fallback method happens to override the ActiveRecord method (as ActiveModel::Serialization is a later mixin than ActiveRecord::AttributeMethods), so this change didn't actually provide an optimisation - the full attribute data was loaded as per [1] This change also, in our case, produced some severe performance issues as it introduced an N+1 query in a situation where we had one gem, Globalize [2], which adds in dynamic attributes that are loaded by a query; and another gem, Transitions [3], that checks attribute names at initialization. The combination of these meant that for every model that was initialized an extra query would run - no matter what includes or eager_load steps were in place. This rapidly hindered our applications' performance and meant we had to rollback the Rails 7 upgrade. Following rafaelfranca's suggestion [4] this adds a `attribute_names_for_serialization` method to Serialization modules in ActiveRecord and ActiveModel. This allows the ActiveRecord one to override the ActiveModel fallback and thus be optimised. Some basic benchmarks of this follow - they use code from https://github.com/ollietreend/rails-demo and have some pretty large arrays set as serialized attributes [5] to demonstrate impacts. Loading attribute names: Rails 7.0.2.3 ``` > Benchmark.ms { Widget.all.map(&:attribute_names) } Widget Load (131.1ms) SELECT "widgets".* FROM "widgets" => 20108.852999983355 ``` This patch ``` > Benchmark.ms { Widget.all.map(&:attribute_names) } Widget Load (144.0ms) SELECT "widgets".* FROM "widgets" => 237.96699999365956 ``` Using serializable_hash: Rails 7.0.2.3 ``` > widgets = Widget.all.to_a; Benchmark.ms { widgets.map { |w| w.serializable_hash(only: []) } } Widget Load (133.3ms) SELECT "widgets".* FROM "widgets" => 22071.45000001765 ``` This patch ``` > widgets = Widget.all.to_a; Benchmark.ms { widgets.map { |w| w.serializable_hash(only: []) } } Widget Load (83.5ms) SELECT "widgets".* FROM "widgets" => 67.9039999959059 ``` [1]: https://github.com/rails/rails/blob/eeb2cfb6864169d7b9cb78744d44b7da17b2b288/activemodel/lib/active_model/serialization.rb#L151-L154 [2]: https://github.com/globalize/globalize [3]: https://github.com/troessner/transitions [4]: https://github.com/rails/rails/pull/44770#pullrequestreview-922209612 [5]: https://github.com/ollietreend/rails-demo/blob/525f88887bb1605a72f7b944b59b0d0e204f92aa/db/seeds.rb
Kevin Dew authoredIn https://github.com/rails/rails/pull/43036 an optimisation was applied to ActiveModel#Serialization to speed up the generation of a serialized_hash by avoiding loading the subjects attributes by using an attribute_names method. A fallback method, ActiveModel::Serialization#attribute_names` was added as #attribute_names isn't part of the public API of ActiveModel. Unfortunately, this fallback method happens to override the ActiveRecord method (as ActiveModel::Serialization is a later mixin than ActiveRecord::AttributeMethods), so this change didn't actually provide an optimisation - the full attribute data was loaded as per [1] This change also, in our case, produced some severe performance issues as it introduced an N+1 query in a situation where we had one gem, Globalize [2], which adds in dynamic attributes that are loaded by a query; and another gem, Transitions [3], that checks attribute names at initialization. The combination of these meant that for every model that was initialized an extra query would run - no matter what includes or eager_load steps were in place. This rapidly hindered our applications' performance and meant we had to rollback the Rails 7 upgrade. Following rafaelfranca's suggestion [4] this adds a `attribute_names_for_serialization` method to Serialization modules in ActiveRecord and ActiveModel. This allows the ActiveRecord one to override the ActiveModel fallback and thus be optimised. Some basic benchmarks of this follow - they use code from https://github.com/ollietreend/rails-demo and have some pretty large arrays set as serialized attributes [5] to demonstrate impacts. Loading attribute names: Rails 7.0.2.3 ``` > Benchmark.ms { Widget.all.map(&:attribute_names) } Widget Load (131.1ms) SELECT "widgets".* FROM "widgets" => 20108.852999983355 ``` This patch ``` > Benchmark.ms { Widget.all.map(&:attribute_names) } Widget Load (144.0ms) SELECT "widgets".* FROM "widgets" => 237.96699999365956 ``` Using serializable_hash: Rails 7.0.2.3 ``` > widgets = Widget.all.to_a; Benchmark.ms { widgets.map { |w| w.serializable_hash(only: []) } } Widget Load (133.3ms) SELECT "widgets".* FROM "widgets" => 22071.45000001765 ``` This patch ``` > widgets = Widget.all.to_a; Benchmark.ms { widgets.map { |w| w.serializable_hash(only: []) } } Widget Load (83.5ms) SELECT "widgets".* FROM "widgets" => 67.9039999959059 ``` [1]: https://github.com/rails/rails/blob/eeb2cfb6864169d7b9cb78744d44b7da17b2b288/activemodel/lib/active_model/serialization.rb#L151-L154 [2]: https://github.com/globalize/globalize [3]: https://github.com/troessner/transitions [4]: https://github.com/rails/rails/pull/44770#pullrequestreview-922209612 [5]: https://github.com/ollietreend/rails-demo/blob/525f88887bb1605a72f7b944b59b0d0e204f92aa/db/seeds.rb
Loading