Update benchmark to be more representative, update docs on performance, remove object shapes optimization (#2329)

* update performance docs to reference more representative benchmark

* Revert "Pre-allocate instance variables for better compatibility with Object Shapes (#2282)"

This reverts commit afbac9b.

* lints

* more lints

Author: joelhawksley

docs/CHANGELOG.md

@@ -30,6 +30,18 @@ nav_order: 6
 
     *Stephen Nelson*
 
+* Update documentation on performance to reflect more representative benchmark showing 2-3x speed increase over partials.
+
+    *Joel Hawksley*
+
+* Add documentation note about instrumentation negatively affecting performance.
+
+    *Joel Hawksley*
+
+* Revert object shapes optimization due to lack of evidence of improvement.
+
+    *Joel Hawksley*
+
 ## 4.0.0.alpha5
 
 * BREAKING: `config.view_component_path` is now `config.generate.path`, as components have long since been able to exist in any directory.
@@ -40,10 +52,6 @@ nav_order: 6
 
     *Joel Hawksley*
 
-* Add internal optimization for Ruby object shapes.
-
-    *Adam Hess*, *Joel Hawksley*
-
 ## 4.0.0.alpha4
 
 * BREAKING: Remove default initializer from `ViewComponent::Base`. Previously, `ViewComponent::Base` defined a catch-all initializer that allowed components without an initializer defined to be passed arbitrary arguments.

docs/guide/instrumentation.md

@@ -26,6 +26,8 @@ ActiveSupport::Notifications.subscribe("render.view_component") do |event| # or
 end
 ```
 
+_Note: Enabling instrumentation negatively impacts the performance of ViewComponent._
+
 ## Viewing instrumentation sums in the browser developer tools
 
 When using `render.view_component` with `config.server_timing = true` (default in development) in Rails 7, the browser developer tools display the sum total timing information in Network > Timing under the key `render.view_component`.

docs/index.md

@@ -85,26 +85,12 @@ ViewComponents use a standard Ruby initializer that clearly defines what's neede
 
 ### Performance
 
-Based on several [benchmarks](https://github.com/viewcomponent/view_component/blob/main/performance/partial_benchmark.rb), ViewComponents are ~10x faster than partials in real-world use-cases.
+Based on several [benchmarks](https://github.com/viewcomponent/view_component/blob/main/performance/partial_benchmark.rb), ViewComponents are ~2.5x faster than partials:
 
-The primary optimization is pre-compiling all ViewComponent templates at application boot, instead of at runtime like traditional Rails views.
-
-For example, the `MessageComponent` template is compiled onto the Ruby object:
-
-```ruby
-# app/components/message_component.rb
-class MessageComponent < ViewComponent::Base
-  def initialize(name:)
-    @name = name
-  end
-
-  def call
-    @output_buffer.safe_append = "<h1>Hello, ".freeze
-    @output_buffer.append = (@name)
-    @output_buffer.safe_append = "!</h1>".freeze
-    @output_buffer.to_s
-  end
-end
+```console
+Comparison:
+  component:     6498.1 i/s
+    partial:     2676.5 i/s - 2.50x  slower
 ```
 
 ### Code quality

lib/view_component/base.rb

@@ -34,15 +34,6 @@ class Base
     class << self
       delegate(*ViewComponent::Config.defaults.keys, to: :config)
 
-      # Redefine `new` so we can pre-allocate instance variables to optimize
-      # for Ruby object shapes.
-      def new(...)
-        instance = allocate
-        instance.__vc_pre_allocate_instance_variables
-        instance.send(:initialize, ...)
-        instance
-      end
-
       # Returns the current config.
       #
       # @return [ActiveSupport::OrderedOptions]
@@ -56,29 +47,6 @@ def config
       end
     end
 
-    def __vc_pre_allocate_instance_variables
-      @__vc_parent_render_level = 0
-      @__vc_set_slots = {}
-      @__vc_content_evaluated = false
-      @current_template = nil
-      @output_buffer = nil
-      @lookup_context = nil
-      @view_flow = nil
-      @view_context = nil
-      @virtual_path = nil
-      @__vc_ancestor_calls = nil
-      @__vc_controller = nil
-      @__vc_content = :unset # some behaviors depend on checking for nil
-      @__vc_content_set_by_with_content = nil
-      @__vc_helpers = nil
-      @__vc_inline_template = nil
-      @__vc_inline_template_defined = nil
-      @__vc_render_in_block = nil
-      @__vc_request = nil
-      @__vc_requested_details = nil
-      @__vc_original_view_context = nil
-    end
-
     include ActionView::Helpers
     include ERB::Escape
     include ActiveSupport::CoreExt::ERBUtil
@@ -151,12 +119,14 @@ def render_in(view_context, &block)
       @__vc_requested_details ||= @lookup_context.vc_requested_details
 
       # For caching, such as #cache_if
+      @current_template = nil unless defined?(@current_template)
       old_current_template = @current_template
 
-      if block && __vc_content_set_by_with_content?
+      if block && defined?(@__vc_content_set_by_with_content)
         raise DuplicateContentError.new(self.class.name)
       end
 
+      @__vc_content_evaluated = false
       @__vc_render_in_block = block
 
       before_render
@@ -215,12 +185,16 @@ def render_parent
     #
     # When rendering the parent inside an .erb template, use `#render_parent` instead.
     def render_parent_to_string
-      target_render = self.class.instance_variable_get(:@__vc_ancestor_calls)[@__vc_parent_render_level]
-      @__vc_parent_render_level += 1
+      @__vc_parent_render_level ||= 0 # ensure a good starting value
 
-      target_render.bind_call(self, @__vc_requested_details)
-    ensure
-      @__vc_parent_render_level -= 1
+      begin
+        target_render = self.class.instance_variable_get(:@__vc_ancestor_calls)[@__vc_parent_render_level]
+        @__vc_parent_render_level += 1
+
+        target_render.bind_call(self, @__vc_requested_details)
+      ensure
+        @__vc_parent_render_level -= 1
+      end
     end
 
     # Optional content to be returned before the rendered template.
@@ -343,12 +317,12 @@ def __vc_request
     # @return [String]
     def content
       @__vc_content_evaluated = true
-      return @__vc_content if @__vc_content != :unset
+      return @__vc_content if defined?(@__vc_content)
 
       @__vc_content =
         if __vc_render_in_block_provided?
           view_context.capture(self, &@__vc_render_in_block)
-        elsif __vc_content_set_by_with_content?
+        elsif __vc_content_set_by_with_content_defined?
           @__vc_content_set_by_with_content
         end
     end
@@ -357,23 +331,23 @@ def content
     #
     # @return [Boolean]
     def content?
-      __vc_render_in_block_provided? || __vc_content_set_by_with_content?
+      __vc_render_in_block_provided? || __vc_content_set_by_with_content_defined?
     end
 
     private
 
     attr_reader :view_context
 
     def __vc_render_in_block_provided?
-      @view_context && @__vc_render_in_block
+      defined?(@view_context) && @view_context && @__vc_render_in_block
     end
 
-    def __vc_content_set_by_with_content?
-      !@__vc_content_set_by_with_content.nil?
+    def __vc_content_set_by_with_content_defined?
+      defined?(@__vc_content_set_by_with_content)
     end
 
     def content_evaluated?
-      @__vc_content_evaluated
+      defined?(@__vc_content_evaluated) && @__vc_content_evaluated
     end
 
     def maybe_escape_html(text)
@@ -577,7 +551,7 @@ def render_template_for(requested_details)
         child.with_collection_parameter provided_collection_parameter
 
         if instance_methods(false).include?(:render_template_for)
-          vc_ancestor_calls = (!@__vc_ancestor_calls.nil?) ? @__vc_ancestor_calls.dup : []
+          vc_ancestor_calls = defined?(@__vc_ancestor_calls) ? @__vc_ancestor_calls.dup : []
 
           vc_ancestor_calls.unshift(instance_method(:render_template_for))
           child.instance_variable_set(:@__vc_ancestor_calls, vc_ancestor_calls)

lib/view_component/inline_template.rb

@@ -9,7 +9,7 @@ module InlineTemplate
       def method_missing(method, *args)
         return super if !method.end_with?("_template")
 
-        if @__vc_inline_template_defined
+        if defined?(@__vc_inline_template_defined) && @__vc_inline_template_defined
           raise MultipleInlineTemplatesError
         end
 
@@ -38,11 +38,11 @@ def respond_to_missing?(method, include_all = false)
       end
 
       def inline_template
-        @__vc_inline_template
+        @__vc_inline_template if defined?(@__vc_inline_template)
       end
 
       def __vc_inline_template_language
-        @__vc_inline_template_language
+        @__vc_inline_template_language if defined?(@__vc_inline_template_language)
       end
 
       def inherited(subclass)

lib/view_component/slot.rb

@@ -9,18 +9,13 @@ class Slot
     attr_writer :__vc_component_instance, :__vc_content_block, :__vc_content
 
     def initialize(parent)
-      @content = nil
-      @__vc_component_instance = nil
-      @__vc_content = nil
-      @__vc_content_block = nil
-      @__vc_content_set_by_with_content = nil
       @parent = parent
     end
 
     def content?
-      return true if @__vc_content.present?
-      return true if @__vc_content_set_by_with_content.present?
-      return true if @__vc_content_block.present?
+      return true if defined?(@__vc_content) && @__vc_content.present?
+      return true if defined?(@__vc_content_set_by_with_content) && @__vc_content_set_by_with_content.present?
+      return true if defined?(@__vc_content_block) && @__vc_content_block.present?
       return false if !__vc_component_instance?
 
       @__vc_component_instance.content?
@@ -48,31 +43,31 @@ def with_content(args)
     # If there is no slot renderable, we evaluate the block passed to
     # the slot and return it.
     def to_s
-      return @content if !@content.nil?
+      return @content if defined?(@content)
 
       view_context = @parent.send(:view_context)
 
-      if !@__vc_content_block.nil? && !@__vc_content_set_by_with_content.nil? && !@__vc_content_set_by_with_content.nil?
+      if defined?(@__vc_content_block) && defined?(@__vc_content_set_by_with_content)
         raise DuplicateSlotContentError.new(self.class.name)
       end
 
       @content =
         if __vc_component_instance?
           @__vc_component_instance.__vc_original_view_context = @parent.__vc_original_view_context
 
-          if !@__vc_content_block.nil?
+          if defined?(@__vc_content_block)
             # render_in is faster than `parent.render`
             @__vc_component_instance.render_in(view_context) do |*args|
               @__vc_content_block.call(*args)
             end
           else
             @__vc_component_instance.render_in(view_context)
           end
-        elsif !@__vc_content.nil?
+        elsif defined?(@__vc_content)
           @__vc_content
-        elsif !@__vc_content_block.nil?
+        elsif defined?(@__vc_content_block)
           view_context.capture(&@__vc_content_block)
-        elsif !@__vc_content_set_by_with_content.nil?
+        elsif defined?(@__vc_content_set_by_with_content)
           @__vc_content_set_by_with_content
         end
 
@@ -113,7 +108,7 @@ def respond_to_missing?(symbol, include_all = false)
     private
 
     def __vc_component_instance?
-      !@__vc_component_instance.nil?
+      defined?(@__vc_component_instance)
     end
   end
 end

lib/view_component/slotable.rb

@@ -421,6 +421,8 @@ def set_slot(slot_name, slot_definition = nil, *args, **kwargs, &block)
         end
       end
 
+      @__vc_set_slots ||= {}
+
       if slot_definition[:collection]
         @__vc_set_slots[slot_name] ||= []
         @__vc_set_slots[slot_name].push(slot)
@@ -434,7 +436,7 @@ def set_slot(slot_name, slot_definition = nil, *args, **kwargs, &block)
     def set_polymorphic_slot(slot_name, poly_type = nil, *args, **kwargs, &block)
       slot_definition = self.class.registered_slots[slot_name]
 
-      if !slot_definition[:collection] && @__vc_set_slots[slot_name]
+      if !slot_definition[:collection] && defined?(@__vc_set_slots) && @__vc_set_slots[slot_name]
         raise ContentAlreadySetForPolymorphicSlotError.new(slot_name)
       end