Prevent creation of unnecessary fieldset

[abcang]

Purpose

When I profiled a simple sample with stackprof, I found that ActiveSupport::Inflector#apply_inflections was taking a long time.

class Blog < ActiveModelSerializers::Model
  attributes :id, :name, :authors
end
class Author < ActiveModelSerializers::Model
  attributes :id, :name
end
class AuthorSerializer < ActiveModel::Serializer
  attributes :id, :name
end
class BlogSerializer < ActiveModel::Serializer
  attributes :id
  attribute :name, key: :title
  
  has_many :authors, serializer: ::AuthorSerializer
end

ActiveSupport::Notifications.unsubscribe(ActiveModelSerializers::Logging::RENDER_EVENT)
authors = [Author.new(id: 1, name: 'Blog Author')]
blog = Blog.new(id: 2, name: 'The Blog', authors: authors)

StackProf.run(mode: :cpu, out: Rails.root.join('tmp/stackprof.dump').to_s, raw: true) do
  10000.times do
    ActiveModelSerializers::SerializableResource.new(blog, serializer: BlogSerializer, adapter: :attributes).as_json
  end
end

$ bundle exec stackprof tmp/stackprof.dump --text
==================================
  Mode: cpu(1000)
  Samples: 1193 (0.00% miss rate)
  GC: 110 (9.22%)
==================================
     TOTAL    (pct)     SAMPLES    (pct)     FRAME
       318  (26.7%)         287  (24.1%)     ActiveSupport::Inflector#apply_inflections
        67   (5.6%)          67   (5.6%)     (sweeping)
        67   (5.6%)          67   (5.6%)     ActiveModel::Serializer::LazyAssociation#reflection_options
        69   (5.8%)          53   (4.4%)     Hash#deep_dup
        43   (3.6%)          43   (3.6%)     (marking)
       651  (54.6%)          41   (3.4%)     ActiveModel::Serializer#associations
        39   (3.3%)          37   (3.1%)     Hash#except
       910  (76.3%)          29   (2.4%)     ActiveModel::Serializer#serializable_hash
        26   (2.2%)          26   (2.2%)     ActiveSupport::Inflector::Inflections::Uncountables#uncountable?
        35   (2.9%)          25   (2.1%)     ActiveSupport::Inflector#underscore
        23   (1.9%)          23   (1.9%)     Set#include?
       660  (55.3%)          23   (1.9%)     ActiveModel::Serializer#associations_hash
        42   (3.5%)          22   (1.8%)     #<Module:0x00007fc6e9ad4cd0>.parse_include_args
        49   (4.1%)          21   (1.8%)     ActiveModel::Serializer#attributes
        21   (1.8%)          20   (1.7%)     #<Module:0x00007fc6e9ad4cd0>.parse_hash
        19   (1.6%)          19   (1.6%)     ActiveModel::Serializer::Association#initialize
        18   (1.5%)          18   (1.5%)     String#blank?
        18   (1.5%)          18   (1.5%)     Concurrent::Collection::NonConcurrentMapBackend#[]
        17   (1.4%)          17   (1.4%)     ActiveModel::Serializer#read_attribute_for_serialization
        16   (1.3%)          16   (1.3%)     ActiveSupport::TaggedLogging::Formatter#current_tags

ActiveSupport::Inflector#apply_inflections is a method executed by String#singularize and String#pluralize. And these are executed from ActiveModel::Serializer::Fieldset#fields_for.

active_model_serializers/lib/active_model/serializer/fieldset.rb

Lines 14 to 16 in 2581fe0

	def fields_for(type)
	fields[type.to_s.singularize.to_sym] || fields[type.to_s.pluralize.to_sym]
	end

Currently, even if there are no options for fields or fieldset, Fieldset is created and fields_for is executed. Therefore, if there is no option, speed up by changing to not generate Fieldset and not execute fields_for.

Additional helpful information

difference of bench result

 ./bin/bench -p bm_adapter,bm_active_record
-attributes 145.66285947392612/ips; 5672 objects
-json_api 198.91949534942208/ips; 4035 objects
-json 130.68608467497918/ips; 5729 objects
+attributes 719.0152573082975/ips; 2920 objects
+json_api 259.23329654303933/ips; 3995 objects
+json 588.9270769106553/ips; 2977 objects
 Benchmark results:
 {
   "commit_hash": "64c7fee7",
   "version": "0.10.10",
   "rails_version": "4.2.11.1",
   "benchmark_run[environment]": "2.5.7p206",
   "runs": [
     {
       "benchmark_type[category]": "attributes",
-      "benchmark_run[result][iterations_per_second]": 145.663,
-      "benchmark_run[result][total_allocated_objects_per_iteration]": 5672
+      "benchmark_run[result][iterations_per_second]": 719.015,
+      "benchmark_run[result][total_allocated_objects_per_iteration]": 2920
     },
     {
       "benchmark_type[category]": "json_api",
-      "benchmark_run[result][iterations_per_second]": 198.919,
-      "benchmark_run[result][total_allocated_objects_per_iteration]": 4035
+      "benchmark_run[result][iterations_per_second]": 259.233,
+      "benchmark_run[result][total_allocated_objects_per_iteration]": 3995
     },
     {
       "benchmark_type[category]": "json",
-      "benchmark_run[result][iterations_per_second]": 130.686,
-      "benchmark_run[result][total_allocated_objects_per_iteration]": 5729
+      "benchmark_run[result][iterations_per_second]": 588.927,
+      "benchmark_run[result][total_allocated_objects_per_iteration]": 2977
     }
   ]
 }
-AR: attributes 12407.733984605495/ips; 95 objects
-AR: json 12371.410834512175/ips; 95 objects
-AR: JSON API 12044.448251130165/ips; 95 objects
+AR: attributes 13657.023038033145/ips; 95 objects
+AR: json 13658.777954548972/ips; 95 objects
+AR: JSON API 13409.89490021574/ips; 95 objects
 Benchmark results:
 {
   "commit_hash": "64c7fee7",
   "version": "0.10.10",
   "rails_version": "4.2.11.1",
   "benchmark_run[environment]": "2.5.7p206",
   "runs": [
     {
       "benchmark_type[category]": "AR: attributes",
-      "benchmark_run[result][iterations_per_second]": 12407.734,
+      "benchmark_run[result][iterations_per_second]": 13657.023,
       "benchmark_run[result][total_allocated_objects_per_iteration]": 95
     },
     {
       "benchmark_type[category]": "AR: json",
-      "benchmark_run[result][iterations_per_second]": 12371.411,
+      "benchmark_run[result][iterations_per_second]": 13658.778,
       "benchmark_run[result][total_allocated_objects_per_iteration]": 95
     },
     {
       "benchmark_type[category]": "AR: JSON API",
-      "benchmark_run[result][iterations_per_second]": 12044.448,
+      "benchmark_run[result][iterations_per_second]": 13409.895,
       "benchmark_run[result][total_allocated_objects_per_iteration]": 95
     }
   ]
 }

[bf4]

Great analysis and reporting! Reviewing code and CI

[abcang]

The failure of the test with the combination of ruby v2.2 and rails v5.2 seems to be a bug of rails v5.2.4.1. It seems to have been fixed in the 5-2-stable branch.
rails/rails@v5.2.4.1...5-2-stable

[say8425]

@abcang Hey Rails 5.2.4.2 was released! Can you push this AWESOME PR again?

[wasifhossain]

i fear there are some issues with the travis config on 0-10-stable. the probable fix is under WIP in #2371

once that's merged, hopefully this PR can be pushed again with success

[liijunwei]

hi everyone, just would like to bring this up again as we spot performance issue in lib/active_model/serializer/fieldset.rb as well

our temp solution is patch lib/active_model/serializer/fieldset.rb@fields_for

and it saves about 50 ms response time!! (it was a slow api, but the improvement is significant)
it only works for limited cases, still we think it'll be useful for other users as well

we also found that this PR(#2370) is actually better --- we don't need to initialize Fieldset when fields is empty

Really appreciate you attention on this ~
I'd like to contribute or take over this PR if you're busy🙏