diff --git a/Rakefile b/Rakefile index def6fd625..7c629c8a6 100644 --- a/Rakefile +++ b/Rakefile @@ -9,3 +9,10 @@ Rake::TestTask.new do |t| end task default: :test + +desc 'Run benchmarks' +namespace :test do + Rake::TestTask.new(:benchmark) do |t| + t.pattern = 'test/benchmark/*_benchmark.rb' + end +end diff --git a/jsonapi-resources.gemspec b/jsonapi-resources.gemspec index f9b7b05a5..cedf4b174 100644 --- a/jsonapi-resources.gemspec +++ b/jsonapi-resources.gemspec @@ -25,5 +25,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'minitest-spec-rails' spec.add_development_dependency 'simplecov' spec.add_development_dependency 'pry' + spec.add_development_dependency 'concurrent-ruby-ext' spec.add_dependency 'rails', '>= 4.0' + spec.add_dependency 'concurrent-ruby' end diff --git a/lib/jsonapi-resources.rb b/lib/jsonapi-resources.rb index 6bd727a21..741be7b29 100644 --- a/lib/jsonapi-resources.rb +++ b/lib/jsonapi-resources.rb @@ -1,3 +1,4 @@ +require 'jsonapi/naive_cache' require 'jsonapi/resource' require 'jsonapi/response_document' require 'jsonapi/acts_as_resource_controller' diff --git a/lib/jsonapi/acts_as_resource_controller.rb b/lib/jsonapi/acts_as_resource_controller.rb index c609a82a8..42fa2b40e 100644 --- a/lib/jsonapi/acts_as_resource_controller.rb +++ b/lib/jsonapi/acts_as_resource_controller.rb @@ -113,7 +113,7 @@ def serialization_options # JSONAPI.configuration.route = :camelized_route # # Override if you want to set a per controller key format. - # Must return a class derived from KeyFormatter. + # Must return an instance of a class derived from KeyFormatter. def key_formatter JSONAPI.configuration.key_formatter end diff --git a/lib/jsonapi/configuration.rb b/lib/jsonapi/configuration.rb index d54de4109..a5c715d30 100644 --- a/lib/jsonapi/configuration.rb +++ b/lib/jsonapi/configuration.rb @@ -1,14 +1,13 @@ require 'jsonapi/formatter' require 'jsonapi/operations_processor' require 'jsonapi/active_record_operations_processor' +require 'concurrent' module JSONAPI class Configuration attr_reader :json_key_format, :resource_key_type, - :key_formatter, :route_format, - :route_formatter, :raise_if_parameters_not_allowed, :operations_processor, :allow_include, @@ -23,7 +22,8 @@ class Configuration :top_level_meta_record_count_key, :exception_class_whitelist, :always_include_to_one_linkage_data, - :always_include_to_many_linkage_data + :always_include_to_many_linkage_data, + :cache_formatters def initialize #:underscored_key, :camelized_key, :dasherized_key, or custom @@ -74,20 +74,69 @@ def initialize # NOTE: always_include_to_many_linkage_data is not currently implemented self.always_include_to_one_linkage_data = false self.always_include_to_many_linkage_data = false + + # Formatter Caching + # Set to false to disable caching of string operations on keys and links. + self.cache_formatters = true + end + + def cache_formatters=(bool) + @cache_formatters = bool + if bool + @key_formatter_tlv = Concurrent::ThreadLocalVar.new + @route_formatter_tlv = Concurrent::ThreadLocalVar.new + else + @key_formatter_tlv = nil + @route_formatter_tlv = nil + end end def json_key_format=(format) @json_key_format = format - @key_formatter = JSONAPI::Formatter.formatter_for(format) + if @cache_formatters + @key_formatter_tlv = Concurrent::ThreadLocalVar.new + end + end + + def route_format=(format) + @route_format = format + if @cache_formatters + @route_formatter_tlv = Concurrent::ThreadLocalVar.new + end + end + + def key_formatter + if self.cache_formatters + formatter = @key_formatter_tlv.value + return formatter if formatter + end + + formatter = JSONAPI::Formatter.formatter_for(self.json_key_format) + + if self.cache_formatters + formatter = @key_formatter_tlv.value = formatter.cached + end + + return formatter end def resource_key_type=(key_type) @resource_key_type = key_type end - def route_format=(format) - @route_format = format - @route_formatter = JSONAPI::Formatter.formatter_for(format) + def route_formatter + if self.cache_formatters + formatter = @route_formatter_tlv.value + return formatter if formatter + end + + formatter = JSONAPI::Formatter.formatter_for(self.route_format) + + if self.cache_formatters + formatter = @route_formatter_tlv.value = formatter.cached + end + + return formatter end def operations_processor=(operations_processor) diff --git a/lib/jsonapi/formatter.rb b/lib/jsonapi/formatter.rb index 9965a7ac3..9da13cf86 100644 --- a/lib/jsonapi/formatter.rb +++ b/lib/jsonapi/formatter.rb @@ -9,9 +9,12 @@ def unformat(arg) arg end + def cached + return FormatterWrapperCache.new(self) + end + def formatter_for(format) - formatter_class_name = "#{format.to_s.camelize}Formatter" - formatter_class_name.safe_constantize + "#{format.to_s.camelize}Formatter".safe_constantize end end end @@ -51,11 +54,33 @@ def unformat(value) end def value_formatter_for(type) - formatter_name = "#{type.to_s.camelize}Value" - formatter_for(formatter_name) + "#{type.to_s.camelize}ValueFormatter".safe_constantize end end end + + # Warning: Not thread-safe. Wrap in ThreadLocalVar as needed. + class FormatterWrapperCache + attr_reader :formatter_klass + + def initialize(formatter_klass) + @formatter_klass = formatter_klass + @format_cache = NaiveCache.new{|arg| formatter_klass.format(arg) } + @unformat_cache = NaiveCache.new{|arg| formatter_klass.unformat(arg) } + end + + def format(arg) + @format_cache.get(arg) + end + + def unformat(arg) + @unformat_cache.get(arg) + end + + def cached + self + end + end end class UnderscoredKeyFormatter < JSONAPI::KeyFormatter diff --git a/lib/jsonapi/link_builder.rb b/lib/jsonapi/link_builder.rb index 2ca5fef90..5ce94992a 100644 --- a/lib/jsonapi/link_builder.rb +++ b/lib/jsonapi/link_builder.rb @@ -2,21 +2,24 @@ module JSONAPI class LinkBuilder attr_reader :base_url, :primary_resource_klass, - :route_formatter + :route_formatter, + :engine_name def initialize(config = {}) @base_url = config[:base_url] @primary_resource_klass = config[:primary_resource_klass] @route_formatter = config[:route_formatter] - @is_engine = !!engine_name - end + @engine_name = build_engine_name - def engine? - @is_engine + # Warning: These make LinkBuilder non-thread-safe. That's not a problem with the + # request-specific way it's currently used, though. + @resources_path_cache = JSONAPI::NaiveCache.new do |source_klass| + formatted_module_path_from_class(source_klass) + format_route(source_klass._type.to_s) + end end - def engine_name - @engine_name ||= build_engine_name + def engine? + !!@engine_name end def primary_resources_url @@ -96,7 +99,7 @@ def engine_resources_path_name_from_class(klass) end def format_route(route) - route_formatter.format(route.to_s) + route_formatter.format(route) end def formatted_module_path_from_class(klass) @@ -113,11 +116,12 @@ def module_scopes_from_class(klass) klass.name.to_s.split("::")[0...-1] end + def regular_resources_path(source_klass) + @resources_path_cache.get(source_klass) + end + def regular_primary_resources_path - [ - formatted_module_path_from_class(primary_resource_klass), - format_route(primary_resource_klass._type.to_s), - ].join + regular_resources_path(primary_resource_klass) end def regular_primary_resources_url @@ -125,11 +129,7 @@ def regular_primary_resources_url end def regular_resource_path(source) - [ - formatted_module_path_from_class(source.class), - format_route(source.class._type.to_s), - "/#{ source.id }", - ].join + "#{regular_resources_path(source.class)}/#{source.id}" end def regular_resource_url(source) diff --git a/lib/jsonapi/naive_cache.rb b/lib/jsonapi/naive_cache.rb new file mode 100644 index 000000000..53bf6ccb0 --- /dev/null +++ b/lib/jsonapi/naive_cache.rb @@ -0,0 +1,30 @@ +module JSONAPI + + # Cache which memoizes the given block. + # + # It's "naive" because it clears the least-recently-inserted cache entry + # rather than the least-recently-used. This makes lookups faster but cache + # misses more frequent after cleanups. Therefore you the best time to use + # this cache is when you expect only a small number of unique lookup keys, so + # that the cache never has to clear. + # + # Also, it's not thread safe (although jsonapi-resources is careful to only + # use it in a thread safe way). + class NaiveCache + def initialize(cap = 10000, &calculator) + @cap = cap + @data = {} + @calculator = calculator + end + + def get(key) + found = true + value = @data.fetch(key) { found = false } + return value if found + value = @calculator.call(key) + @data[key] = value + @data.shift if @data.length > @cap + return value + end + end +end diff --git a/lib/jsonapi/relationship.rb b/lib/jsonapi/relationship.rb index 3d6492435..d03589e70 100644 --- a/lib/jsonapi/relationship.rb +++ b/lib/jsonapi/relationship.rb @@ -22,7 +22,7 @@ def primary_key end def resource_klass - @resource_klass = @parent_resource.resource_for(@class_name) + @resource_klass ||= @parent_resource.resource_for(@class_name) end def table_name diff --git a/lib/jsonapi/resource.rb b/lib/jsonapi/resource.rb index da89f4950..a4781f7ba 100644 --- a/lib/jsonapi/resource.rb +++ b/lib/jsonapi/resource.rb @@ -601,8 +601,10 @@ def find(filters, options = {}) records = apply_pagination(records, options[:paginator], order_options) resources = [] + resource_classes = {} records.each do |model| - resources.push self.resource_for_model(model).new(model, context) + resource_class = resource_classes[model.class] ||= self.resource_for_model(model) + resources.push resource_class.new(model, context) end resources @@ -665,7 +667,7 @@ def key_type(key_type) end def resource_key_type - @_resource_key_type || JSONAPI.configuration.resource_key_type + @_resource_key_type ||= JSONAPI.configuration.resource_key_type end def verify_key(key, context = nil) diff --git a/lib/jsonapi/resource_serializer.rb b/lib/jsonapi/resource_serializer.rb index 421259b8d..0c82e3b90 100644 --- a/lib/jsonapi/resource_serializer.rb +++ b/lib/jsonapi/resource_serializer.rb @@ -12,7 +12,7 @@ class ResourceSerializer # Purpose: determines which fields are serialized for a resource type. This encompasses both attributes and # relationship ids in the links section for a resource. Fields are global for a resource type. # Example: { people: [:id, :email, :comments], posts: [:id, :title, :author], comments: [:id, :body, :post]} - # key_formatter: KeyFormatter class to override the default configuration + # key_formatter: KeyFormatter instance to override the default configuration # serializer_options: additional options that will be passed to resource meta and links lambdas def initialize(primary_resource_klass, options = {}) @@ -21,12 +21,17 @@ def initialize(primary_resource_klass, options = {}) @include = options.fetch(:include, []) @include_directives = options[:include_directives] @key_formatter = options.fetch(:key_formatter, JSONAPI.configuration.key_formatter) + @id_formatter = ValueFormatter.value_formatter_for(:id) @link_builder = generate_link_builder(primary_resource_klass, options) @always_include_to_one_linkage_data = options.fetch(:always_include_to_one_linkage_data, JSONAPI.configuration.always_include_to_one_linkage_data) @always_include_to_many_linkage_data = options.fetch(:always_include_to_many_linkage_data, JSONAPI.configuration.always_include_to_many_linkage_data) @serialization_options = options.fetch(:serialization_options, {}) + + # Warning: This makes ResourceSerializer non-thread-safe. That's not a problem with the + # request-specific way it's currently used, though. + @value_formatter_type_cache = NaiveCache.new{|arg| ValueFormatter.value_formatter_for(arg) } end # Converts a single resource, or an array of resources to a hash, conforming to the JSONAPI structure @@ -81,8 +86,7 @@ def format_key(key) end def format_value(value, format) - value_formatter = JSONAPI::ValueFormatter.value_formatter_for(format) - value_formatter.format(value) + @value_formatter_type_cache.get(format).format(value) end private @@ -300,18 +304,26 @@ def link_object(source, relationship, include_linkage = false) def foreign_key_value(source, relationship) foreign_key = relationship.foreign_key value = source.public_send(foreign_key) - IdValueFormatter.format(value) + @id_formatter.format(value) end def foreign_key_types_and_values(source, relationship) if relationship.is_a?(JSONAPI::Relationship::ToMany) if relationship.polymorphic? - source._model.public_send(relationship.name).pluck(:type, :id).map do |type, id| - [type.underscore.pluralize, IdValueFormatter.format(id)] + assoc = source._model.public_send(relationship.name) + # Avoid hitting the database again for values already pre-loaded + if assoc.respond_to?(:loaded?) and assoc.loaded? + assoc.map do |obj| + [obj.type.underscore.pluralize, @id_formatter.format(obj.id)] + end + else + assoc.pluck(:type, :id).map do |type, id| + [type.underscore.pluralize, @id_formatter.format(id)] + end end else source.public_send(relationship.foreign_key).map do |value| - [relationship.type, IdValueFormatter.format(value)] + [relationship.type, @id_formatter.format(value)] end end end diff --git a/test/benchmark/request_benchmark.rb b/test/benchmark/request_benchmark.rb new file mode 100644 index 000000000..7db5b25be --- /dev/null +++ b/test/benchmark/request_benchmark.rb @@ -0,0 +1,14 @@ +require File.expand_path('../../test_helper', __FILE__) + +class RequestBenchmark < IntegrationBenchmark + def setup + $test_user = Person.find(1) + end + + def bench_large_index_request + 10.times do + get '/api/v2/books?include=bookComments,bookComments.author' + assert_jsonapi_response 200 + end + end +end diff --git a/test/fixtures/books.yml b/test/fixtures/books.yml index 667dd569b..87265107f 100644 --- a/test/fixtures/books.yml +++ b/test/fixtures/books.yml @@ -4,4 +4,4 @@ book_<%= book_num %>: title: Book <%= book_num %> isbn: 12345-<%= book_num %>-6789 banned: <%= book_num > 600 && book_num < 700 %> -<% end %> \ No newline at end of file +<% end %> diff --git a/test/integration/requests/request_test.rb b/test/integration/requests/request_test.rb index 992144cac..7f43e8323 100644 --- a/test/integration/requests/request_test.rb +++ b/test/integration/requests/request_test.rb @@ -12,11 +12,6 @@ def after_teardown JSONAPI.configuration.route_format = :underscored_route end - def assert_jsonapi_response(expected_status) - assert_equal JSONAPI::MEDIA_TYPE, response.content_type - assert_equal expected_status, status - end - def test_get get '/posts' assert_jsonapi_response 200 diff --git a/test/test_helper.rb b/test/test_helper.rb index 303409d65..44db43553 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -425,6 +425,25 @@ class ActiveSupport::TestCase class ActionDispatch::IntegrationTest self.fixture_path = "#{Rails.root}/fixtures" fixtures :all + + def assert_jsonapi_response(expected_status) + assert_equal JSONAPI::MEDIA_TYPE, response.content_type + assert_equal expected_status, status + end +end + +class IntegrationBenchmark < ActionDispatch::IntegrationTest + def self.runnable_methods + methods_matching(/^bench_/) + end + + def self.run_one_method(klass, method_name, reporter) + Benchmark.bm(method_name.length) do |job| + job.report(method_name) do + super(klass, method_name, reporter) + end + end + end end class UpperCamelizedKeyFormatter < JSONAPI::KeyFormatter