From 53f175516b0c2be876a7a597c28359175436f71c Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Thu, 24 Sep 2020 01:43:05 +0530 Subject: [PATCH] Implement custom delegators for drop methods (#8183) Merge pull request 8183 --- benchmark/static-drop-vs-forwarded.rb | 83 +++++++++++++++++++++++++++ lib/jekyll/drops/collection_drop.rb | 6 +- lib/jekyll/drops/document_drop.rb | 19 ++---- lib/jekyll/drops/drop.rb | 78 +++++++++++++++++++++---- lib/jekyll/drops/site_drop.rb | 6 +- lib/jekyll/drops/static_file_drop.rb | 8 +-- lib/jekyll/drops/url_drop.rb | 4 +- 7 files changed, 165 insertions(+), 39 deletions(-) create mode 100755 benchmark/static-drop-vs-forwarded.rb diff --git a/benchmark/static-drop-vs-forwarded.rb b/benchmark/static-drop-vs-forwarded.rb new file mode 100755 index 00000000..9f8365d3 --- /dev/null +++ b/benchmark/static-drop-vs-forwarded.rb @@ -0,0 +1,83 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require "forwardable" +require "colorator" +require "liquid" +require "benchmark/ips" +require "memory_profiler" + +# Set up (memory) profiler + +class Profiler + def self.run + yield new(ARGV[0] || 10_000) + end + + def initialize(count) + @count = count.to_i + end + + def report(label, color, &block) + prof_report = MemoryProfiler.report { @count.to_i.times(&block) } + + allocated_memory = prof_report.scale_bytes(prof_report.total_allocated_memsize) + allocated_objects = prof_report.total_allocated + retained_memory = prof_report.scale_bytes(prof_report.total_retained_memsize) + retained_objects = prof_report.total_retained + + puts <<~MSG.send(color) + With #{label} calls + + Total allocated: #{allocated_memory} (#{allocated_objects} objects) + Total retained: #{retained_memory} (#{retained_objects} objects) + MSG + end +end + +# Set up stage + +class Drop < Liquid::Drop + def initialize(obj) + @obj = obj + end +end + +class ForwardDrop < Drop + extend Forwardable + def_delegators :@obj, :name +end + +class StaticDrop < Drop + def name + @obj.name + end +end + +class Document + def name + "lipsum" + end +end + +# Set up actors + +document = Document.new +alpha = ForwardDrop.new(document) +beta = StaticDrop.new(document) +count = ARGV[0] || 10_000 + +# Run profilers +puts "\nMemory profiles for #{count} calls to invoke drop key:" +Profiler.run do |x| + x.report("forwarded", :cyan) { alpha["name"] } + x.report("static", :green) { beta["name"] } +end + +# Benchmark +puts "\nBenchmarking the two scenarios..." +Benchmark.ips do |x| + x.report("forwarded".cyan) { alpha["name"] } + x.report("static".green) { beta["name"] } + x.compare! +end diff --git a/lib/jekyll/drops/collection_drop.rb b/lib/jekyll/drops/collection_drop.rb index ff28039b..4fe95a55 100644 --- a/lib/jekyll/drops/collection_drop.rb +++ b/lib/jekyll/drops/collection_drop.rb @@ -7,10 +7,10 @@ module Jekyll mutable false - def_delegator :@obj, :write?, :output - def_delegators :@obj, :label, :docs, :files, :directory, :relative_directory + delegate_method_as :write?, :output + delegate_methods :label, :docs, :files, :directory, :relative_directory - private def_delegator :@obj, :metadata, :fallback_data + private delegate_method_as :metadata, :fallback_data def to_s docs.to_s diff --git a/lib/jekyll/drops/document_drop.rb b/lib/jekyll/drops/document_drop.rb index ebe23bee..fef5920b 100644 --- a/lib/jekyll/drops/document_drop.rb +++ b/lib/jekyll/drops/document_drop.rb @@ -11,10 +11,11 @@ module Jekyll mutable false - def_delegator :@obj, :relative_path, :path - def_delegators :@obj, :id, :output, :content, :to_s, :relative_path, :url, :date + delegate_method_as :relative_path, :path + private delegate_method_as :data, :fallback_data - private def_delegator :@obj, :data, :fallback_data + delegate_methods :id, :output, :content, :to_s, :relative_path, :url, :date + data_delegators "title", "categories", "tags" def collection @obj.collection.label @@ -64,18 +65,6 @@ module Jekyll result[key] = doc[key] unless NESTED_OBJECT_FIELD_BLACKLIST.include?(key) end end - - def title - @obj.data["title"] - end - - def categories - @obj.data["categories"] - end - - def tags - @obj.data["tags"] - end end end end diff --git a/lib/jekyll/drops/drop.rb b/lib/jekyll/drops/drop.rb index c883bc0c..f60c16e6 100644 --- a/lib/jekyll/drops/drop.rb +++ b/lib/jekyll/drops/drop.rb @@ -7,19 +7,73 @@ module Jekyll NON_CONTENT_METHODS = [:fallback_data, :collapse_document].freeze - # Get or set whether the drop class is mutable. - # Mutability determines whether or not pre-defined fields may be - # overwritten. - # - # is_mutable - Boolean set mutability of the class (default: nil) - # - # Returns the mutability of the class - def self.mutable(is_mutable = nil) - @is_mutable = is_mutable || false - end + class << self + # Get or set whether the drop class is mutable. + # Mutability determines whether or not pre-defined fields may be + # overwritten. + # + # is_mutable - Boolean set mutability of the class (default: nil) + # + # Returns the mutability of the class + def mutable(is_mutable = nil) + @is_mutable = is_mutable || false + end - def self.mutable? - @is_mutable + def mutable? + @is_mutable + end + + # public delegation helper methods that calls onto Drop's instance + # variable `@obj`. + + # Generate private Drop instance_methods for each symbol in the given list. + # + # Returns nothing. + def private_delegate_methods(*symbols) + symbols.each { |symbol| private delegate_method(symbol) } + nil + end + + # Generate public Drop instance_methods for each symbol in the given list. + # + # Returns nothing. + def delegate_methods(*symbols) + symbols.each { |symbol| delegate_method(symbol) } + nil + end + + # Generate public Drop instance_method for given symbol that calls `@obj.`. + # + # Returns delegated method symbol. + def delegate_method(symbol) + define_method(symbol) { @obj.send(symbol) } + end + + # Generate public Drop instance_method named `delegate` that calls `@obj.`. + # + # Returns delegated method symbol. + def delegate_method_as(original, delegate) + define_method(delegate) { @obj.send(original) } + end + + # Generate public Drop instance_methods for each string entry in the given list. + # The generated method(s) access(es) `@obj`'s data hash. + # + # Returns nothing. + def data_delegators(*strings) + strings.each do |key| + data_delegator(key) if key.is_a?(String) + end + nil + end + + # Generate public Drop instance_methods for given string `key`. + # The generated method access(es) `@obj`'s data hash. + # + # Returns method symbol. + def data_delegator(key) + define_method(key.to_sym) { @obj.data[key] } + end end # Create a new Drop diff --git a/lib/jekyll/drops/site_drop.rb b/lib/jekyll/drops/site_drop.rb index a13f87f2..cc3c60fb 100644 --- a/lib/jekyll/drops/site_drop.rb +++ b/lib/jekyll/drops/site_drop.rb @@ -7,10 +7,10 @@ module Jekyll mutable false - def_delegator :@obj, :site_data, :data - def_delegators :@obj, :time, :pages, :static_files, :tags, :categories + delegate_method_as :site_data, :data + delegate_methods :time, :pages, :static_files, :tags, :categories - private def_delegator :@obj, :config, :fallback_data + private delegate_method_as :config, :fallback_data def [](key) if key != "posts" && @obj.collections.key?(key) diff --git a/lib/jekyll/drops/static_file_drop.rb b/lib/jekyll/drops/static_file_drop.rb index e76566b0..d00973b0 100644 --- a/lib/jekyll/drops/static_file_drop.rb +++ b/lib/jekyll/drops/static_file_drop.rb @@ -4,11 +4,11 @@ module Jekyll module Drops class StaticFileDrop < Drop extend Forwardable - def_delegators :@obj, :name, :extname, :modified_time, :basename - def_delegator :@obj, :relative_path, :path - def_delegator :@obj, :type, :collection + delegate_methods :name, :extname, :modified_time, :basename + delegate_method_as :relative_path, :path + delegate_method_as :type, :collection - private def_delegator :@obj, :data, :fallback_data + private delegate_method_as :data, :fallback_data end end end diff --git a/lib/jekyll/drops/url_drop.rb b/lib/jekyll/drops/url_drop.rb index a710368f..de58c95f 100644 --- a/lib/jekyll/drops/url_drop.rb +++ b/lib/jekyll/drops/url_drop.rb @@ -7,8 +7,8 @@ module Jekyll mutable false - def_delegator :@obj, :cleaned_relative_path, :path - def_delegator :@obj, :output_ext, :output_ext + delegate_method :output_ext + delegate_method_as :cleaned_relative_path, :path def collection @obj.collection.label