Optimize array! DSL (#14)

* Optimize array! DSL

* Better array! optimization

* Default collections args to nil

* Stop allocating memory for empty collections

* Save memory allocations for internal array calls

* Make constants private

* Add support for present?

* More frozen empty arrays

* Add benchmark code

Author: moberegger

benchmarks/jbuilder/set_dsl_array_block.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+require 'benchmark/ips'
+require 'benchmark/memory'
+require_relative '../../lib/jbuilder'
+
+Post = Struct.new(:id, :body)
+json = Jbuilder.new
+posts = 3.times.map { Post.new(it, "Post ##{it}") }
+
+Benchmark.ips do |x|
+  x.report('before') do
+    json.set! :posts, posts do |post|
+      json.extract! post, :id, :body
+    end
+  end
+  x.report('after') do
+    json.set! :posts, posts do |post|
+      json.extract! post, :id, :body
+    end
+  end
+
+  x.hold! 'temp_array_ips'
+  x.compare!
+end
+
+json = Jbuilder.new
+
+Benchmark.memory do |x|
+  x.report('before') do
+    json.set! :posts, posts do |post|
+      json.extract! post, :id, :body
+    end
+  end
+  x.report('after') do
+    json.set! :posts, posts do |post|
+      json.extract! post, :id, :body
+    end
+  end
+
+  x.hold! 'temp_array_memory'
+  x.compare!
+end

benchmarks/jbuilder/set_dsl_extract_array.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'benchmark/ips'
+require 'benchmark/memory'
+require_relative '../../lib/jbuilder'
+
+Post = Struct.new(:id, :body)
+json = Jbuilder.new
+posts = 3.times.map { Post.new(it, "Post ##{it}") }
+
+Benchmark.ips do |x|
+  x.report('before') do
+    json.set! :posts, posts, :id, :body
+  end
+  x.report('after') do
+    json.set! :posts, posts, :id, :body
+  end
+
+  x.hold! 'temp_array_ips'
+  x.compare!
+end
+
+json = Jbuilder.new
+
+Benchmark.memory do |x|
+  x.report('before') do
+    json.set! :posts, posts, :id, :body
+  end
+  x.report('after') do
+    json.set! :posts, posts, :id, :body
+  end
+
+  x.hold! 'temp_array_memory'
+  x.compare!
+end

benchmarks/jbuilder_template/array_dsl.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+require 'benchmark/ips'
+require 'benchmark/memory'
+require_relative '../../lib/jbuilder'
+require_relative '../../lib/jbuilder/jbuilder_template'
+
+Post = Struct.new(:id, :body)
+json = JbuilderTemplate.new nil
+posts = 3.times.map { Post.new(it, "Post ##{it}") }
+
+Benchmark.ips do |x|
+  x.report('before') do
+    json.array!(nil)
+  end
+  x.report('after') do
+    json.array!(nil)
+  end
+
+  x.hold! 'temp_array_ips'
+  x.compare!
+end
+
+json = JbuilderTemplate.new nil
+
+Benchmark.memory do |x|
+  x.report('before') { json.array! posts, :id, :body }
+  x.report('after') { json.array! posts, :id, :body }
+
+  x.hold! 'temp_array_memory'
+  x.compare!
+end

lib/jbuilder.rb

@@ -8,6 +8,7 @@
 require 'jbuilder/version'
 require 'json'
 require 'active_support/core_ext/hash/deep_merge'
+require 'active_support/core_ext/object/blank'
 
 class Jbuilder
   @@key_formatter = nil
@@ -33,14 +34,16 @@ def self.encode(*args, &block)
     new(*args, &block).target!
   end
 
-  BLANK = Blank.new
+  BLANK = Blank.new.freeze
+  EMPTY_ARRAY = [].freeze
+  private_constant :BLANK, :EMPTY_ARRAY
 
   def set!(key, value = BLANK, *args, &block)
     result = if ::Kernel.block_given?
       if !_blank?(value)
         # json.comments @post.comments { |comment| ... }
         # { "comments": [ { ... }, { ... } ] }
-        _scope{ array! value, &block }
+        _scope{ _array value, &block }
       else
         # json.comments { ... }
         # { "comments": ... }
@@ -60,7 +63,7 @@ def set!(key, value = BLANK, *args, &block)
     elsif _is_collection?(value)
       # json.comments @post.comments, :content, :created_at
       # { "comments": [ { "content": "hello", "created_at": "..." }, { "content": "world", "created_at": "..." } ] }
-      _scope{ array! value, *args }
+      _scope{ _array value, args }
     else
       # json.author @post.creator, :name, :email_address
       # { "author": { "name": "David", "email_address": "[email protected]" } }
@@ -207,18 +210,8 @@ def child!
   #   json.array! [1, 2, 3]
   #
   #   [1,2,3]
-  def array!(collection = [], *attributes, &block)
-    array = if collection.nil?
-      []
-    elsif ::Kernel.block_given?
-      _map_collection(collection, &block)
-    elsif attributes.any?
-      _map_collection(collection) { |element| _extract element, attributes }
-    else
-      _format_keys(collection.to_a)
-    end
-
-    @attributes = _merge_values(@attributes, array)
+  def array!(collection = EMPTY_ARRAY, *attributes, &block)
+    _array(collection, attributes, &block)
   end
 
   # Extracts the mentioned attributes or hash elements from the passed object and turns them into attributes of the JSON.
@@ -244,7 +237,7 @@ def extract!(object, *attributes)
 
   def call(object, *attributes, &block)
     if ::Kernel.block_given?
-      array! object, &block
+      _array object, &block
     else
       _extract object, attributes
     end
@@ -277,6 +270,20 @@ def target!
 
   alias_method :method_missing, :set!
 
+  def _array(collection = EMPTY_ARRAY, attributes = nil, &block)
+    array = if collection.nil?
+      EMPTY_ARRAY
+    elsif block
+      _map_collection(collection, &block)
+    elsif attributes.present?
+      _map_collection(collection) { |element| _extract element, attributes }
+    else
+      _format_keys(collection.to_a)
+    end
+
+    @attributes = _merge_values(@attributes, array)
+  end
+
   def _extract(object, attributes)
     if ::Hash === object
       _extract_hash_values(object, attributes)

lib/jbuilder/jbuilder_template.rb

@@ -118,14 +118,14 @@ def target!
     @cached_root || super
   end
 
-  def array!(collection = [], *args)
+  def array!(collection = EMPTY_ARRAY, *args, &block)
     options = args.first
 
-    if args.one? && _partial_options?(options)
+    if _partial_options?(options)
       options[:collection] = collection
       _render_partial_with_options options
     else
-      super
+      _array collection, args, &block
     end
   end
 
@@ -149,7 +149,7 @@ def _render_partial_with_options(options)
     as = options[:as]
 
     if as && options.key?(:collection)
-      collection = options.delete(:collection) || []
+      collection = options.delete(:collection) || EMPTY_ARRAY
       partial = options.delete(:partial)
       options[:locals][:json] = self
       collection = EnumerableCompat.new(collection) if collection.respond_to?(:count) && !collection.respond_to?(:size)
@@ -167,9 +167,9 @@ def _render_partial_with_options(options)
           .new(@context.lookup_context, options) { |&block| _scope(&block) }
           .render_collection_with_partial(collection, partial, @context, nil)
 
-        array! if results.respond_to?(:body) && results.body.nil?
+        _array if results.respond_to?(:body) && results.body.nil?
       else
-        array!
+        _array
       end
     else
       _render_partial options
@@ -232,7 +232,7 @@ def _is_active_model?(object)
 
   def _set_inline_partial(name, object, options)
     value = if object.nil?
-      []
+      EMPTY_ARRAY
     elsif _is_collection?(object)
       _scope do
         options[:collection] = object