diff --git a/batch-loader.gemspec b/batch-loader.gemspec index f97a68d..b7dfe38 100644 --- a/batch-loader.gemspec +++ b/batch-loader.gemspec @@ -1,7 +1,5 @@ # coding: utf-8 -lib = File.expand_path("../lib", __FILE__) -$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) -require "batch_loader/version" +require_relative "./lib/batch_loader/version" Gem::Specification.new do |spec| spec.name = "batch-loader" diff --git a/lib/batch-loader.rb b/lib/batch-loader.rb index f8b33bb..6145e5d 100644 --- a/lib/batch-loader.rb +++ b/lib/batch-loader.rb @@ -1 +1 @@ -require "batch_loader" +require_relative "./batch_loader" diff --git a/lib/batch_loader.rb b/lib/batch_loader.rb index 08c54b3..d97fa1b 100644 --- a/lib/batch_loader.rb +++ b/lib/batch_loader.rb @@ -2,13 +2,13 @@ require "set" -require "batch_loader/version" -require "batch_loader/executor_proxy" -require "batch_loader/middleware" -require "batch_loader/graphql" +require_relative "./batch_loader/version" +require_relative "./batch_loader/executor_proxy" +require_relative "./batch_loader/middleware" +require_relative "./batch_loader/graphql" class BatchLoader - IMPLEMENTED_INSTANCE_METHODS = %i[object_id __id__ __send__ singleton_method_added batch_loader? respond_to? batch inspect].freeze + IMPLEMENTED_INSTANCE_METHODS = %i[object_id __id__ __send__ singleton_method_added __sync respond_to? batch inspect].freeze REPLACABLE_INSTANCE_METHODS = %i[batch inspect].freeze LEFT_INSTANCE_METHODS = (IMPLEMENTED_INSTANCE_METHODS - REPLACABLE_INSTANCE_METHODS).freeze @@ -25,17 +25,13 @@ def initialize(item:) def batch(cache: true, &batch_block) @cache = cache @batch_block = batch_block - executor_proxy.add(item: @item) + __executor_proxy.add(item: @item) - singleton_class.class_eval { undef_method(:batch) } + __singleton_class.class_eval { undef_method(:batch) } self end - def batch_loader? - true - end - def respond_to?(method_name) LEFT_INSTANCE_METHODS.include?(method_name) || method_missing(:respond_to?, method_name) end @@ -44,63 +40,74 @@ def inspect "#" end + def __sync + return @loaded_value if @synced + + __ensure_batched + @loaded_value = __executor_proxy.loaded_value(item: @item) + + if @cache + @synced = true + else + __purge_cache + end + + @loaded_value + end + private def method_missing(method_name, *args, &block) - sync!.public_send(method_name, *args, &block) + __sync!.public_send(method_name, *args, &block) end - def sync! - return self if @synced - - ensure_batched - loaded_value = executor_proxy.loaded_value(item: @item) + def __sync! + loaded_value = __sync if @cache - replace_with!(loaded_value) - @synced = true - self + __replace_with!(loaded_value) else - purge_cache loaded_value end end - def ensure_batched - return if executor_proxy.value_loaded?(item: @item) + def __ensure_batched + return if __executor_proxy.value_loaded?(item: @item) - items = executor_proxy.list_items - loader = ->(item, value) { executor_proxy.load(item: item, value: value) } + items = __executor_proxy.list_items + loader = ->(item, value) { __executor_proxy.load(item: item, value: value) } @batch_block.call(items, loader) items.each do |item| - next if executor_proxy.value_loaded?(item: item) + next if __executor_proxy.value_loaded?(item: item) loader.call(item, nil) # use "nil" for not loaded item after succesfull batching end - executor_proxy.delete(items: items) + __executor_proxy.delete(items: items) end - def singleton_class + def __singleton_class class << self ; self ; end end - def replace_with!(value) - singleton_class.class_eval do + def __replace_with!(value) + __singleton_class.class_eval do (value.methods - LEFT_INSTANCE_METHODS).each do |method_name| define_method(method_name) do |*args, &block| value.public_send(method_name, *args, &block) end end end + + self end - def purge_cache - executor_proxy.unload_value(item: @item) - executor_proxy.add(item: @item) + def __purge_cache + __executor_proxy.unload_value(item: @item) + __executor_proxy.add(item: @item) end - def executor_proxy - @executor_proxy ||= begin + def __executor_proxy + @__executor_proxy ||= begin raise NoBatchError.new("Please provide a batch block first") unless @batch_block BatchLoader::ExecutorProxy.new(&@batch_block) end diff --git a/lib/batch_loader/executor_proxy.rb b/lib/batch_loader/executor_proxy.rb index 09ddcc6..91e2405 100644 --- a/lib/batch_loader/executor_proxy.rb +++ b/lib/batch_loader/executor_proxy.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "batch_loader/executor" +require_relative "./executor" class BatchLoader class ExecutorProxy diff --git a/lib/batch_loader/graphql.rb b/lib/batch_loader/graphql.rb index bdf4f5d..5cba1df 100644 --- a/lib/batch_loader/graphql.rb +++ b/lib/batch_loader/graphql.rb @@ -8,7 +8,7 @@ def initialize(batch_loader) end def sync - @batch_loader + @batch_loader.__sync end end @@ -21,7 +21,7 @@ def self.instrument(type, field) old_resolve_proc = field.resolve_proc new_resolve_proc = ->(object, arguments, context) do result = old_resolve_proc.call(object, arguments, context) - result.respond_to?(:batch_loader?) ? BatchLoader::GraphQL::Wrapper.new(result) : result + result.respond_to?(:__sync) ? BatchLoader::GraphQL::Wrapper.new(result) : result end field.redefine { resolve(new_resolve_proc) } diff --git a/spec/batch_loader_spec.rb b/spec/batch_loader_spec.rb index 601a484..8905fe5 100644 --- a/spec/batch_loader_spec.rb +++ b/spec/batch_loader_spec.rb @@ -62,6 +62,29 @@ expect { post.user_lazy.foo }.to raise_error(NoMethodError, /undefined method `foo' for #(id) do + BatchLoader.for(id).batch do |user_ids, loader| + User.where(id: user_ids).each { |u| loader.call(u.id, u.id) } + end + end + batch_loader = ->(id) do + BatchLoader.for(id).batch do |user_ids, loader| + user_ids.each { |user_id| loader.call(user_id, nested_batch_loader.call(user_id)) } + end + end + + expect(User).to receive(:where).with(id: [1, 2]).once.and_call_original + + result = [batch_loader.call(1), batch_loader.call(2)] + + expect(result).to eq([1, 2]) + end end context 'loader' do @@ -77,15 +100,6 @@ end end - describe '#batch_loader?' do - it 'always returns true' do - user = User.save(id: 1) - post = Post.new(user_id: user.id) - - expect(post.user_lazy.batch_loader?).to eq(true) - end - end - describe '#inspect' do it 'returns BatchLoader without syncing and delegates #inspect after' do user = User.save(id: 1) diff --git a/spec/benchmarks/graphql.rb b/spec/benchmarks/graphql.rb new file mode 100644 index 0000000..8a6ddb5 --- /dev/null +++ b/spec/benchmarks/graphql.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Usage: ruby spec/benchmarks/graphql.rb && open tmp/stack.html + +require 'ruby-prof' +require "graphql" + +require_relative "../../lib/batch_loader" +require_relative "../fixtures/models" +require_relative "../fixtures/graphql_schema" + +iterations = Array.new(2_000) + +iterations.each_with_index do |_, i| + user = User.save(id: i) + Post.save(user_id: user.id) +end + +query = "{ posts { user { id } } }" + +RubyProf.measure_mode = RubyProf::WALL_TIME +RubyProf.start + +GraphqlSchema.execute(query) # 0.45, 0.52, 0.47 sec + +result = RubyProf.stop +stack_printer = RubyProf::CallStackPrinter.new(result) +File.open("tmp/stack.html", 'w') { |file| stack_printer.print(file) } diff --git a/spec/benchmarks/profiling.rb b/spec/benchmarks/profiling.rb index efec91c..a8161ea 100644 --- a/spec/benchmarks/profiling.rb +++ b/spec/benchmarks/profiling.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Usage: ruby spec/benchmarks/profiling.rb +# Usage: ruby spec/benchmarks/profiling.rb && open tmp/stack.html require 'ruby-prof' @@ -8,6 +8,7 @@ require_relative "../fixtures/models" User.save(id: 1) +iterations = Array.new(5_000) def batch_loader BatchLoader.for(1).batch do |ids, loader| @@ -18,7 +19,7 @@ def batch_loader RubyProf.measure_mode = RubyProf::WALL_TIME RubyProf.start -1_000.times { batch_loader.id } +iterations.each { batch_loader.id } # 2.46, 2.87, 2.56 sec result = RubyProf.stop stack_printer = RubyProf::CallStackPrinter.new(result) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8f2943a..7b75edd 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,11 +5,11 @@ Coveralls.wear! end -require "batch_loader" +require_relative "../lib/batch_loader" require "graphql" -require "fixtures/models" -require "fixtures/graphql_schema" +require_relative "./fixtures/models" +require_relative "./fixtures/graphql_schema" RSpec.configure do |config| # Enable flags like --only-failures and --next-failure