From 37feccd044b5528b7e05da167a9f62959855f7ee Mon Sep 17 00:00:00 2001 From: Saiqul Haq Date: Sat, 29 Jun 2024 17:14:48 +0700 Subject: [PATCH 1/3] Improve performance of fields_for method with memoization --- lib/active_model/serializer/fieldset.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/active_model/serializer/fieldset.rb b/lib/active_model/serializer/fieldset.rb index 78609a4e5..a63d083fd 100644 --- a/lib/active_model/serializer/fieldset.rb +++ b/lib/active_model/serializer/fieldset.rb @@ -12,7 +12,14 @@ def fields end def fields_for(type) - fields[type.to_s.singularize.to_sym] || fields[type.to_s.pluralize.to_sym] + @fields_for_cache ||= {} + return @fields_for_cache[type] if @fields_for_cache.key?(type) + + singular_type = type.to_s.singularize.to_sym + plural_type = type.to_s.pluralize.to_sym + result = fields[singular_type] || fields[plural_type] + + @fields_for_cache[type] = result end protected From f76d055bf7969461443b8c599c20816d51ffe850 Mon Sep 17 00:00:00 2001 From: Saiqul Haq Date: Sat, 29 Jun 2024 23:21:23 +0700 Subject: [PATCH 2/3] fix: use Concurrent::Map if exist to make it faster --- lib/active_model/serializer/fieldset.rb | 29 ++++++++++++++++++------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/active_model/serializer/fieldset.rb b/lib/active_model/serializer/fieldset.rb index a63d083fd..7db0232d6 100644 --- a/lib/active_model/serializer/fieldset.rb +++ b/lib/active_model/serializer/fieldset.rb @@ -3,8 +3,16 @@ module ActiveModel class Serializer class Fieldset + begin + require 'concurrent' + CONCURRENT_MAP_AVAILABLE = true + rescue LoadError + CONCURRENT_MAP_AVAILABLE = false + end + def initialize(fields) @raw_fields = fields || {} + @fields_for_cache = CONCURRENT_MAP_AVAILABLE ? Concurrent::Map.new : {} end def fields @@ -12,14 +20,13 @@ def fields end def fields_for(type) - @fields_for_cache ||= {} - return @fields_for_cache[type] if @fields_for_cache.key?(type) - - singular_type = type.to_s.singularize.to_sym - plural_type = type.to_s.pluralize.to_sym - result = fields[singular_type] || fields[plural_type] - - @fields_for_cache[type] = result + if CONCURRENT_MAP_AVAILABLE + @fields_for_cache.fetch_or_store(type) do + compute_fields_for(type) + end + else + @fields_for_cache[type] ||= compute_fields_for(type) + end end protected @@ -35,6 +42,12 @@ def parsed_fields {} end end + + def compute_fields_for(type) + singular_type = type.to_s.singularize.to_sym + plural_type = type.to_s.pluralize.to_sym + fields[singular_type] || fields[plural_type] + end end end end From f126f9ea5526c6761ea344620764d307c5a93f4f Mon Sep 17 00:00:00 2001 From: Saiqul Haq Date: Sat, 29 Jun 2024 23:29:08 +0700 Subject: [PATCH 3/3] fix: no need to use Memoization if there is no Concurrent Ruby because it's not a threadsafe --- lib/active_model/serializer/fieldset.rb | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/active_model/serializer/fieldset.rb b/lib/active_model/serializer/fieldset.rb index 7db0232d6..4e98f3274 100644 --- a/lib/active_model/serializer/fieldset.rb +++ b/lib/active_model/serializer/fieldset.rb @@ -3,16 +3,11 @@ module ActiveModel class Serializer class Fieldset - begin - require 'concurrent' - CONCURRENT_MAP_AVAILABLE = true - rescue LoadError - CONCURRENT_MAP_AVAILABLE = false - end + CONCURRENT_MAP_AVAILABLE = defined?(Concurrent::Map) def initialize(fields) @raw_fields = fields || {} - @fields_for_cache = CONCURRENT_MAP_AVAILABLE ? Concurrent::Map.new : {} + @fields_for_cache = Concurrent::Map.new if CONCURRENT_MAP_AVAILABLE end def fields @@ -25,7 +20,7 @@ def fields_for(type) compute_fields_for(type) end else - @fields_for_cache[type] ||= compute_fields_for(type) + compute_fields_for(type) end end