Merge pull request rmosolgo#3499 from rmosolgo/directives-on-fragments

Runtime directives on fragments

Author: Robert Mosolgo

benchmark/run.rb

@@ -3,7 +3,7 @@
 require "graphql"
 require "jazz"
 require "benchmark/ips"
-require "ruby-prof"
+require "stackprof"
 require "memory_profiler"
 require "graphql/batch"
 
@@ -46,15 +46,11 @@ def self.profile
     SCHEMA.execute(document: DOCUMENT)
     # CARD_SCHEMA.validate(ABSTRACT_FRAGMENTS)
     res = nil
-    result = RubyProf.profile do
+    result = StackProf.run(mode: :wall) do
       # CARD_SCHEMA.validate(ABSTRACT_FRAGMENTS)
       res = SCHEMA.execute(document: DOCUMENT)
     end
-    # printer = RubyProf::FlatPrinter.new(result)
-    # printer = RubyProf::GraphHtmlPrinter.new(result)
-    printer = RubyProf::FlatPrinterWithLineNumbers.new(result)
-
-    printer.print(STDOUT, {})
+    StackProf::Report.new(result).print_text
   end
 
   # Adapted from https://github.com/rmosolgo/graphql-ruby/issues/861
@@ -67,13 +63,10 @@ def self.profile_large_result
       }
     end
 
-    result = RubyProf.profile do
+    result = StackProf.run(mode: :wall) do
       schema.execute(document: document)
     end
-    printer = RubyProf::FlatPrinter.new(result)
-    # printer = RubyProf::GraphHtmlPrinter.new(result)
-    # printer = RubyProf::FlatPrinterWithLineNumbers.new(result)
-    printer.print(STDOUT, {})
+    StackProf::Report.new(result).print_text
 
     report = MemoryProfiler.report do
       schema.execute(document: document)

graphql.gemspec

@@ -36,6 +36,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency "racc", "~> 1.4"
   s.add_development_dependency "rake", "~> 12"
   s.add_development_dependency "rubocop", "0.68" # for Ruby 2.2 enforcement
+  s.add_development_dependency "stackprof"
   # required for upgrader
   s.add_development_dependency "parser"
   # website stuff

guides/type_definitions/directives.md

@@ -77,7 +77,7 @@ query {
 Directive classes may implement the following class methods to interact with the runtime:
 
 - `def self.include?(obj, args, ctx)`: If this hook returns `false`, the nodes flagged by this directive will be skipped at runtime.
-- `def self.resolve(obj, args, ctx)`: Wraps the resolution of flagged nodes. Resolution is passed as a __block__, so `yield` will continue resolution. The return value of this method will be treated as the return value of the flagged field.
+- `def self.resolve(obj, args, ctx)`: Wraps the resolution of flagged nodes. Resolution is passed as a __block__, so `yield` will continue resolution.
 
 Looking for a runtime hook that isn't listed here? {% open_an_issue "New directive hook: @something", "<!-- Describe how the directive would be used and then how you might implement it --> " %} to start the conversation!
 

lib/graphql/dataloader.rb

@@ -77,6 +77,21 @@ def append_job(&job)
       nil
     end
 
+    # Use a self-contained queue for the work in the block.
+    def run_isolated
+      prev_queue = @pending_jobs
+      @pending_jobs = []
+      res = nil
+      # Make sure the block is inside a Fiber, so it can `Fiber.yield`
+      append_job {
+        res = yield
+      }
+      run
+      res
+    ensure
+      @pending_jobs = prev_queue
+    end
+
     # @api private Move along, move along
     def run
       # At a high level, the algorithm is:

lib/graphql/dataloader/null_dataloader.rb

@@ -10,6 +10,7 @@ class NullDataloader < Dataloader
       # These are all no-ops because code was
       # executed sychronously.
       def run; end
+      def run_isolated; yield; end
       def yield; end
 
       def append_job

lib/graphql/execution/interpreter/arguments_cache.rb

@@ -28,11 +28,12 @@ def initialize(query)
         end
 
         def fetch(ast_node, argument_owner, parent_object)
-          @storage[ast_node][argument_owner][parent_object]
           # If any jobs were enqueued, run them now,
           # since this might have been called outside of execution.
           # (The jobs are responsible for updating `result` in-place.)
-          @dataloader.run
+          @dataloader.run_isolated do
+            @storage[ast_node][argument_owner][parent_object]
+          end
           # Ack, the _hash_ is updated, but the key is eventually
           # overridden with an immutable arguments instance.
           # The first call queues up the job,