Improve GraphQL perf up to 3x times by syncing with lazy_resolve only

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"

lib/batch-loader.rb

@@ -1 +1 @@
-require "batch_loader"
+require_relative "./batch_loader"

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
     "#<BatchLoader:0x#{(object_id << 1)}>"
   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

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

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) }

spec/batch_loader_spec.rb

@@ -62,6 +62,29 @@
 
       expect { post.user_lazy.foo }.to raise_error(NoMethodError, /undefined method `foo' for #<User/)
     end
+
+    it 'works with nested BatchLoaders' do
+      user1 = User.save(id: 1)
+      Post.new(user_id: user1.id)
+      user2 = User.save(id: 2)
+      Post.new(user_id: user2.id)
+      nested_batch_loader = ->(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)

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) }

spec/benchmarks/profiling.rb

@@ -1,13 +1,14 @@
 # frozen_string_literal: true
 
-# Usage: ruby spec/benchmarks/profiling.rb
+# Usage: ruby spec/benchmarks/profiling.rb && open tmp/stack.html
 
 require 'ruby-prof'
 
 require_relative "../../lib/batch_loader"
 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)

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