From f70a7e534c27c4d65ca453d2bad4aabbee702055 Mon Sep 17 00:00:00 2001 From: Dillon Welch Date: Tue, 17 Oct 2017 15:38:39 -0700 Subject: [PATCH 1/6] Freeze strings to prevent re-allocation of newline/empty strings --- lib/haml/compiler.rb | 31 ++++++++++++++++++---------- lib/haml/generator.rb | 2 +- lib/haml/helpers.rb | 5 ++--- lib/haml/helpers/action_view_mods.rb | 4 +++- lib/haml/parser.rb | 6 +++--- lib/haml/temple_engine.rb | 4 ++-- lib/haml/util.rb | 4 ++-- 7 files changed, 33 insertions(+), 23 deletions(-) diff --git a/lib/haml/compiler.rb b/lib/haml/compiler.rb index 7fcd5130fd..bab25576db 100644 --- a/lib/haml/compiler.rb +++ b/lib/haml/compiler.rb @@ -110,14 +110,14 @@ def compile_tag push_temple(@attribute_compiler.compile(t[:attributes], object_ref, dynamic_attributes)) push_text( if t[:self_closing] && @options.xhtml? - " />#{"\n" unless t[:nuke_outer_whitespace]}" + " />#{"\n".freeze unless t[:nuke_outer_whitespace]}" else - ">#{"\n" unless (t[:self_closing] && @options.html?) ? t[:nuke_outer_whitespace] : (!block_given? || t[:preserve_tag] || t[:nuke_inner_whitespace])}" + ">#{"\n".freeze unless (t[:self_closing] && @options.html?) ? t[:nuke_outer_whitespace] : (!block_given? || t[:preserve_tag] || t[:nuke_inner_whitespace])}" end ) if value && !parse - push_text("#{value}#{"\n" unless t[:nuke_outer_whitespace]}") + push_text("#{value}#{"\n".freeze unless t[:nuke_outer_whitespace]}") end return if t[:self_closing] @@ -125,13 +125,13 @@ def compile_tag if value.nil? yield if block_given? rstrip_buffer! if t[:nuke_inner_whitespace] - push_text("#{"\n" unless t[:nuke_outer_whitespace]}") + push_text("#{"\n".freeze unless t[:nuke_outer_whitespace]}") return end if parse push_script(value, t.merge(:in_tag => true)) - push_text("#{"\n" unless t[:nuke_outer_whitespace]}") + push_text("#{"\n".freeze unless t[:nuke_outer_whitespace]}") end end @@ -219,9 +219,9 @@ def text_for_doctype def push_silent(text, can_suppress = false) flush_merged_text return if can_suppress && @options.suppress_eval? - newline = (text == "end") ? ";" : "\n" + newline = (text == "end") ? ";" : "\n".freeze @temple << [:code, "#{resolve_newlines}#{text}#{newline}"] - @output_line = @output_line + text.count("\n") + newline.count("\n") + @output_line = @output_line + text.count("\n".freeze) + newline.count("\n".freeze) end # Adds `text` to `@buffer`. @@ -231,7 +231,7 @@ def push_text(text) def push_temple(temple) flush_merged_text - @temple.concat([[:newline]] * resolve_newlines.count("\n")) + @temple.concat([[:newline]] * resolve_newlines.count("\n".freeze)) @temple << temple @output_line += TempleLineCounter.count_lines(temple) end @@ -265,6 +265,14 @@ def push_script(text, opts = {}) unless block_given? push_generated_script(no_format ? "(#{text}\n).to_s" : build_script_formatter("(#{text}\n)", opts)) + # TODO this \n should be able to be frozen but the following tests fails + # + # bin/rails test test/helper_test.rb:596 + # bin/rails test test/engine_test.rb:534 + # + # Both appear to be testing outer whitespace nuking but at this point + # of the code there is no nuke_outer_whitespace key and the + # nuke_inner_whitespace key is false. push_text("\n") unless opts[:in_tag] || opts[:nuke_inner_whitespace] return end @@ -294,14 +302,15 @@ def build_script_formatter(text, opts) def push_generated_script(text) @to_merge << [:script, resolve_newlines + text] - @output_line += text.count("\n") + @output_line += text.count("\n".freeze) end def resolve_newlines diff = @node.line - @output_line - return "" if diff <= 0 + return "".freeze if diff <= 0 @output_line = @node.line - "\n" * diff + return "\n".freeze if diff == 1 + "\n".freeze * diff end # Get rid of and whitespace at the end of the buffer diff --git a/lib/haml/generator.rb b/lib/haml/generator.rb index 0dadf02308..66a3364b3c 100644 --- a/lib/haml/generator.rb +++ b/lib/haml/generator.rb @@ -29,7 +29,7 @@ def on_code(exp) end def on_newline - "\n" + "\n".freeze end private diff --git a/lib/haml/helpers.rb b/lib/haml/helpers.rb index d2e79eb591..38580a6809 100644 --- a/lib/haml/helpers.rb +++ b/lib/haml/helpers.rb @@ -383,7 +383,7 @@ def capture_haml(*args, &block) captured = haml_buffer.buffer.slice!(position..-1) - if captured == '' and value != haml_buffer.buffer + if captured == ''.freeze and value != haml_buffer.buffer captured = (value.is_a?(String) ? value : nil) end @@ -641,7 +641,7 @@ def is_haml? # @param block [Proc] A Ruby block # @return [Boolean] Whether or not `block` is defined directly in a Haml template def block_is_haml?(block) - eval('!!defined?(_hamlout)', block.binding) + eval('!!defined?(_hamlout)'.freeze, block.binding) end private @@ -704,4 +704,3 @@ def is_haml? false end end - diff --git a/lib/haml/helpers/action_view_mods.rb b/lib/haml/helpers/action_view_mods.rb index 014d131074..5b245e1210 100644 --- a/lib/haml/helpers/action_view_mods.rb +++ b/lib/haml/helpers/action_view_mods.rb @@ -55,7 +55,9 @@ module TagHelper def content_tag_with_haml(name, *args, &block) return content_tag_without_haml(name, *args, &block) unless is_haml? - preserve = haml_buffer.options.fetch(:preserve, %w[textarea pre code]).include?(name.to_s) + @_content_tag_name_cache ||= {} + name_string = @_content_tag_name_cache[name] ||= name.to_s + preserve = haml_buffer.options.fetch(:preserve, %w[textarea pre code]).include?(name_string) if block_given? && block_is_haml?(block) && preserve return content_tag_without_haml(name, *args) do diff --git a/lib/haml/parser.rb b/lib/haml/parser.rb index 29a0c89832..5ca4bf24fb 100644 --- a/lib/haml/parser.rb +++ b/lib/haml/parser.rb @@ -559,9 +559,9 @@ def self.parse_class_and_id(list) case type when '.' if attributes[CLASS_KEY] - attributes[CLASS_KEY] += " " + attributes[CLASS_KEY] += " ".freeze else - attributes[CLASS_KEY] = "" + attributes[CLASS_KEY] = "".freeze end attributes[CLASS_KEY] += property when '#'; attributes[ID_KEY] = property @@ -637,7 +637,7 @@ def parse_tag(text) end if value.nil? - value = '' + value = ''.freeze else value.strip! end diff --git a/lib/haml/temple_engine.rb b/lib/haml/temple_engine.rb index 1c7928f065..11e193ba32 100644 --- a/lib/haml/temple_engine.rb +++ b/lib/haml/temple_engine.rb @@ -64,14 +64,14 @@ def precompiled_with_return_value # # @return [String] def precompiled_with_ambles(local_names, after_preamble: '') - preamble = < Date: Fri, 20 Oct 2017 12:35:57 -0700 Subject: [PATCH 2/6] CHANGELOG.md note --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07b492b84a..0e79059b9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Haml Changelog +* Freeze and cache strings to prevent re-allocation of newlines, empty strings, and common content tags. [#961](https://github.com/haml/haml/pull/961) (thanks [Dillon Welch](https://github.com/oniofchaos)) + ## 5.0.4 Released on October 13, 2017 From b9390158fcf94a9fe48076b0b73c456aaf4c9a53 Mon Sep 17 00:00:00 2001 From: Dillon Welch Date: Fri, 20 Oct 2017 13:05:02 -0700 Subject: [PATCH 3/6] Freeze even more strings --- lib/haml/compiler.rb | 44 ++++++++++++++-------------- lib/haml/engine.rb | 2 +- lib/haml/filters.rb | 26 ++++++++--------- lib/haml/generator.rb | 2 +- lib/haml/helpers.rb | 44 +++++++++++++++++----------- lib/haml/parser.rb | 61 ++++++++++++++++++++------------------- lib/haml/temple_engine.rb | 10 +++---- lib/haml/util.rb | 12 ++++---- 8 files changed, 106 insertions(+), 95 deletions(-) diff --git a/lib/haml/compiler.rb b/lib/haml/compiler.rb index bab25576db..de7a0fbea1 100644 --- a/lib/haml/compiler.rb +++ b/lib/haml/compiler.rb @@ -103,7 +103,7 @@ def compile_tag end if @options[:trace] - t[:attributes].merge!({"data-trace" => @options.filename.split('/views').last << ":" << @node.line.to_s}) + t[:attributes].merge!({"data-trace".freeze => @options.filename.split('/views'.freeze).last << ":".freeze << @node.line.to_s}) end push_text("<#{t[:name]}") @@ -179,36 +179,36 @@ def compile_filter end def text_for_doctype - if @node.value[:type] == "xml" + if @node.value[:type] == "xml".freeze return nil if @options.html? wrapper = @options.attr_wrapper return "" end if @options.html5? - '' + ''.freeze else if @options.xhtml? - if @node.value[:version] == "1.1" - '' - elsif @node.value[:version] == "5" - '' + if @node.value[:version] == "1.1".freeze + ''.freeze + elsif @node.value[:version] == "5".freeze + ''.freeze else case @node.value[:type] - when "strict"; '' - when "frameset"; '' - when "mobile"; '' - when "rdfa"; '' - when "basic"; '' - else '' + when "strict".freeze; ''.freeze + when "frameset".freeze; ''.freeze + when "mobile".freeze; ''.freeze + when "rdfa".freeze; ''.freeze + when "basic".freeze; ''.freeze + else ''.freeze end end elsif @options.html4? case @node.value[:type] - when "strict"; '' - when "frameset"; '' - else '' + when "strict".freeze; ''.freeze + when "frameset".freeze; ''.freeze + else ''.freeze end end end @@ -219,7 +219,7 @@ def text_for_doctype def push_silent(text, can_suppress = false) flush_merged_text return if can_suppress && @options.suppress_eval? - newline = (text == "end") ? ";" : "\n".freeze + newline = (text == "end".freeze) ? ";".freeze : "\n".freeze @temple << [:code, "#{resolve_newlines}#{text}#{newline}"] @output_line = @output_line + text.count("\n".freeze) + newline.count("\n".freeze) end @@ -246,7 +246,7 @@ def flush_merged_text when :script @temple << [:dynamic, val] else - raise SyntaxError.new("[HAML BUG] Undefined entry in Haml::Compiler@to_merge.") + raise SyntaxError.new("[HAML BUG] Undefined entry in Haml::Compiler@to_merge.".freeze) end end @@ -281,7 +281,7 @@ def push_script(text, opts = {}) push_silent "haml_temp = #{text}" yield push_silent('end', :can_suppress) unless @node.value[:dont_push_end] - @temple << [:dynamic, no_format ? 'haml_temp.to_s;' : build_script_formatter('haml_temp', opts)] + @temple << [:dynamic, no_format ? 'haml_temp.to_s;'.freeze : build_script_formatter('haml_temp', opts)] end def build_script_formatter(text, opts) @@ -318,7 +318,7 @@ def resolve_newlines def rstrip_buffer!(index = -1) last = @to_merge[index] if last.nil? - push_silent("_hamlout.rstrip!", false) + push_silent("_hamlout.rstrip!".freeze, false) return end @@ -330,10 +330,10 @@ def rstrip_buffer!(index = -1) rstrip_buffer! index end when :script - last[1].gsub!(/\(haml_temp, (.*?)\);$/, '(haml_temp.rstrip, \1);') + last[1].gsub!(/\(haml_temp, (.*?)\);$/, '(haml_temp.rstrip, \1);'.freeze) rstrip_buffer! index - 1 else - raise SyntaxError.new("[HAML BUG] Undefined entry in Haml::Compiler@to_merge.") + raise SyntaxError.new("[HAML BUG] Undefined entry in Haml::Compiler@to_merge.".freeze) end end end diff --git a/lib/haml/engine.rb b/lib/haml/engine.rb index 953dd0e230..d37e214320 100644 --- a/lib/haml/engine.rb +++ b/lib/haml/engine.rb @@ -167,7 +167,7 @@ def render_proc(scope = Object.new, *local_names) begin eval("Proc.new { |*_haml_locals| _haml_locals = _haml_locals[0] || {};" << - @temple_engine.precompiled_with_ambles(local_names) << "}\n", scope, @options.filename, @options.line) + @temple_engine.precompiled_with_ambles(local_names) << "}\n".freeze, scope, @options.filename, @options.line) rescue ::SyntaxError => e raise SyntaxError, e.message end diff --git a/lib/haml/filters.rb b/lib/haml/filters.rb index d935a425d9..543d598ef2 100644 --- a/lib/haml/filters.rb +++ b/lib/haml/filters.rb @@ -42,7 +42,7 @@ def register_tilt_filter(name, options = {}) end filter = const_set(name, Module.new) - filter.extend const_get(options[:extend] || "Plain") + filter.extend const_get(options[:extend] || "Plain".freeze) filter.extend TiltFilter filter.extend PrecompiledTiltFilter if options.has_key? :precompiled @@ -105,7 +105,7 @@ module Base # # @param base [Module, Class] The module that this is included in def self.included(base) - Filters.defined[base.name.split("::").last.downcase] = base + Filters.defined[base.name.split("::".freeze).last.downcase] = base base.extend(base) end @@ -175,7 +175,7 @@ def compile(compiler, text) # filter name). Then we need to escape the trailing # newline so that the whole filter block doesn't take up # too many. - text = %[\n#{text.sub(/\n"\Z/, "\\n\"")}] + text = %[\n#{text.sub(/\n"\Z/, "\\n\"".freeze)}] push_script < false find_and_preserve(#{filter.inspect}.render_with_options(#{text}, _hamlout.options)) RUBY @@ -206,15 +206,15 @@ module Javascript # @see Base#render_with_options def render_with_options(text, options) - indent = options[:cdata] ? ' ' : ' ' # 4 or 2 spaces + indent = options[:cdata] ? ' '.freeze : ' '.freeze # 4 or 2 spaces if options[:format] == :html5 - type = '' + type = ''.freeze else type = " type=#{options[:attr_wrapper]}text/javascript#{options[:attr_wrapper]}" end text = text.rstrip - text.gsub!("\n", "\n#{indent}") + text.gsub!("\n".freeze, "\n#{indent}") %!\n#{" //\n" if options[:cdata]}! end @@ -227,15 +227,15 @@ module Css # @see Base#render_with_options def render_with_options(text, options) - indent = options[:cdata] ? ' ' : ' ' # 4 or 2 spaces + indent = options[:cdata] ? ' '.freeze : ' '.freeze # 4 or 2 spaces if options[:format] == :html5 - type = '' + type = ''.freeze else type = " type=#{options[:attr_wrapper]}text/css#{options[:attr_wrapper]}" end text = text.rstrip - text.gsub!("\n", "\n#{indent}") + text.gsub!("\n".freeze, "\n#{indent}") %(\n#{" /**/\n" if options[:cdata]}) end @@ -249,7 +249,7 @@ module Cdata def render(text) text = "\n#{text}" text.rstrip! - text.gsub!("\n", "\n ") + text.gsub!("\n".freeze, "\n ".freeze) "" end end @@ -282,9 +282,9 @@ module Ruby def compile(compiler, text) return if compiler.options[:suppress_eval] compiler.instance_eval do - push_silent "#{<<-FIRST.tr("\n", ';')}#{text}#{<<-LAST.tr("\n", ';')}" + push_silent "#{<<-FIRST.tr("\n".freeze, ';'.freeze)}#{text}#{<<-LAST.tr("\n", ';')}" begin - haml_io = StringIO.new(_hamlout.buffer, 'a') + haml_io = StringIO.new(_hamlout.buffer, 'a'.freeze) FIRST ensure haml_io.close @@ -320,7 +320,7 @@ def template_class @template_class = Tilt["t.#{tilt_extension}"] or raise Error.new(Error.message(:cant_run_filter, tilt_extension)) rescue LoadError => e - dep = e.message.split('--').last.strip + dep = e.message.split('--'.freeze).last.strip raise Error.new(Error.message(:gem_install_filter_deps, tilt_extension, dep)) end end diff --git a/lib/haml/generator.rb b/lib/haml/generator.rb index 66a3364b3c..3d1ac05ba1 100644 --- a/lib/haml/generator.rb +++ b/lib/haml/generator.rb @@ -13,7 +13,7 @@ def call(exp) end def on_multi(*exp) - exp.map { |e| compile(e) }.join('; ') + exp.map { |e| compile(e) }.join('; '.freeze) end def on_static(text) diff --git a/lib/haml/helpers.rb b/lib/haml/helpers.rb index 38580a6809..5d2c8fe7ad 100644 --- a/lib/haml/helpers.rb +++ b/lib/haml/helpers.rb @@ -134,9 +134,9 @@ def find_and_preserve(input = nil, tags = haml_buffer.options[:preserve], &block # @yield The block within which to escape newlines def preserve(input = nil, &block) return preserve(capture_haml(&block)) if block - s = input.to_s.chomp("\n") - s.gsub!(/\n/, ' ') - s.delete!("\r") + s = input.to_s.chomp("\n".freeze) + s.gsub!(/\n/, ' '.freeze) + s.delete!("\r".freeze) s end alias_method :flatten, :preserve @@ -204,14 +204,14 @@ def list_of(enum, opts={}, &block) enum.each_with_object('') do |i, ret| result = capture_haml(i, &block) - if result.count("\n") > 1 - result.gsub!("\n", "\n ") + if result.count("\n".freeze) > 1 + result.gsub!("\n".freeze, "\n ".freeze) result = "\n #{result.strip}\n" else result.strip! end - ret << "\n" unless ret.empty? + ret << "\n".freeze unless ret.empty? ret << %Q!#{result}! end end @@ -228,9 +228,13 @@ def list_of(enum, opts={}, &block) # # @param lang [String] The value of `xml:lang` and `lang` # @return [{#to_s => String}] The attribute hash - def html_attrs(lang = 'en-US') + def html_attrs(lang = 'en-US'.freeze) if haml_buffer.options[:format] == :xhtml - {:xmlns => "http://www.w3.org/1999/xhtml", 'xml:lang' => lang, :lang => lang} + { + :xmlns => "http://www.w3.org/1999/xhtml".freeze, + 'xml:lang' => lang, + :lang => lang + } else {:lang => lang} end @@ -374,7 +378,7 @@ def succeed(str, &block) # @yield [args] A block of Haml code that will be converted to a string # @yieldparam args [Array] `args` def capture_haml(*args, &block) - buffer = eval('if defined? _hamlout then _hamlout else nil end', block.binding) || haml_buffer + buffer = eval('if defined? _hamlout then _hamlout else nil end'.freeze, block.binding) || haml_buffer with_haml_buffer(buffer) do position = haml_buffer.buffer.length @@ -396,7 +400,7 @@ def capture_haml(*args, &block) # Outputs text directly to the Haml buffer, with the proper indentation. # # @param text [#to_s] The text to output - def haml_concat(text = "") + def haml_concat(text = "".freeze) haml_internal_concat text ErrorReturn.new("haml_concat") end @@ -413,11 +417,11 @@ def haml_concat(text = "") # @param text [#to_s] The text to output # @param newline [Boolean] Whether to add a newline after the text # @param indent [Boolean] Whether to add indentation to the first line - def haml_internal_concat(text = "", newline = true, indent = true) + def haml_internal_concat(text = "".freeze, newline = true, indent = true) if haml_buffer.tabulation == 0 - haml_buffer.buffer << "#{text}#{"\n" if newline}" + haml_buffer.buffer << "#{text}#{"\n".freeze if newline}" else - haml_buffer.buffer << %[#{haml_indent if indent}#{text.to_s.gsub("\n", "\n#{haml_indent}")}#{"\n" if newline}] + haml_buffer.buffer << %[#{haml_indent if indent}#{text.to_s.gsub("\n".freeze, "\n#{haml_indent}")}#{"\n".freeze if newline}] end end private :haml_internal_concat @@ -505,7 +509,7 @@ def haml_tag(name, *rest, &block) attrs) if text.nil? && block.nil? && (haml_buffer.options[:autoclose].include?(name) || flags.include?(:/)) - haml_internal_concat_raw "<#{name}#{attributes}#{' /' if haml_buffer.options[:format] == :xhtml}>" + haml_internal_concat_raw "<#{name}#{attributes}#{' /'.freeze if haml_buffer.options[:format] == :xhtml}>" return ret end @@ -518,7 +522,7 @@ def haml_tag(name, *rest, &block) end_tag = "" if block.nil? text = text.to_s - if text.include?("\n") + if text.include?("\n".freeze) haml_internal_concat_raw tag tab_up haml_internal_concat text @@ -596,7 +600,13 @@ def haml_tag_if(condition, *tag) end # Characters that need to be escaped to HTML entities from user input - HTML_ESCAPE = { '&' => '&', '<' => '<', '>' => '>', '"' => '"', "'" => ''' } + HTML_ESCAPE = { + '&'.freeze => '&'.freeze, + '<'.freeze => '<'.freeze, + '>'.freeze => '>'.freeze, + '"'.freeze => '"'.freeze, + "'".freeze => '''.freeze + }.freeze HTML_ESCAPE_REGEX = /['"><&]/ @@ -652,7 +662,7 @@ def merge_name_and_attributes(name, attributes_hash = {}) # skip merging if no ids or classes found in name return name, attributes_hash unless name =~ /^(.+?)?([\.#].*)$/ - return $1 || "div", AttributeBuilder.merge_attributes!( + return $1 || "div".freeze, AttributeBuilder.merge_attributes!( Haml::Parser.parse_class_and_id($2), attributes_hash) end diff --git a/lib/haml/parser.rb b/lib/haml/parser.rb index 5ca4bf24fb..364db03741 100644 --- a/lib/haml/parser.rb +++ b/lib/haml/parser.rb @@ -106,7 +106,7 @@ def call(template) Line.new(whitespace, text.rstrip, full, index, self, false) end # Append special end-of-document marker - @template << Line.new(nil, '-#', '-#', @template.size, self, true) + @template << Line.new(nil, '-#'.freeze, '-#'.freeze, @template.size, self, true) @root = @parent = ParseNode.new(:root) @flat = false @@ -123,7 +123,7 @@ def call(template) if flat? text = @line.full.dup - text = "" unless text.gsub!(/^#{@flat_spaces}/, '') + text = "".freeze unless text.gsub!(/^#{@flat_spaces}/, ''.freeze) @filter_buffer << "#{text}\n" @line = @next_line next @@ -210,14 +210,14 @@ def inspect class DynamicAttributes < Struct.new(:new, :old) def old=(value) unless value =~ /\A{.*}\z/m - raise ArgumentError.new('Old attributes must start with "{" and end with "}"') + raise ArgumentError.new('Old attributes must start with "{" and end with "}"'.freeze) end super end # This will be a literal for Haml::Buffer#attributes's last argument, `attributes_hashes`. def to_literal - [new, stripped_old].compact.join(', ') + [new, stripped_old].compact.join(', '.freeze) end private @@ -225,7 +225,7 @@ def to_literal # For `%foo{ { foo: 1 }, bar: 2 }`, :old is "{ { foo: 1 }, bar: 2 }" and this method returns " { foo: 1 }, bar: 2 " for last argument. def stripped_old return nil if old.nil? - old.sub!(/\A{/, '').sub!(/}\z/m, '') + old.sub!(/\A{/, ''.freeze).sub!(/}\z/m, ''.freeze) end end @@ -258,10 +258,10 @@ def process_line(line) when ELEMENT; push tag(line) when COMMENT; push comment(line.text[1..-1].lstrip) when SANITIZE - return push plain(line.strip!(3), :escape_html) if line.text[1, 2] == '==' + return push plain(line.strip!(3), :escape_html) if line.text[1, 2] == '=='.freeze return push script(line.strip!(2), :escape_html) if line.text[1] == SCRIPT return push flat_script(line.strip!(2), :escape_html) if line.text[1] == FLAT_SCRIPT - return push plain(line.strip!(1), :escape_html) if line.text[1] == ?\s || line.text[1..2] == '#{' + return push plain(line.strip!(1), :escape_html) if line.text[1] == ?\s || line.text[1..2] == '#{'.freeze push plain(line) when SCRIPT return push plain(line.strip!(2)) if line.text[1] == SCRIPT @@ -273,11 +273,11 @@ def process_line(line) push silent_script(line) when FILTER; push filter(line.text[1..-1].downcase) when DOCTYPE - return push doctype(line.text) if line.text[0, 3] == '!!!' - return push plain(line.strip!(3), false) if line.text[1, 2] == '==' + return push doctype(line.text) if line.text[0, 3] == '!!!'.freeze + return push plain(line.strip!(3), false) if line.text[1, 2] == '=='.freeze return push script(line.strip!(2), false) if line.text[1] == SCRIPT return push flat_script(line.strip!(2), false) if line.text[1] == FLAT_SCRIPT - return push plain(line.strip!(1), false) if line.text[1] == ?\s || line.text[1..2] == '#{' + return push plain(line.strip!(1), false) if line.text[1] == ?\s || line.text[1..2] == '#{'.freeze push plain(line) when ESCAPE line.text = line.text[1..-1] @@ -311,7 +311,7 @@ def plain(line, escape_html = nil) end def script(line, escape_html = nil, preserve = false) - raise SyntaxError.new(Error.message(:no_ruby_code, '=')) if line.text.empty? + raise SyntaxError.new(Error.message(:no_ruby_code, '='.freeze)) if line.text.empty? line = handle_ruby_multiline(line) escape_html = @options.escape_html if escape_html.nil? @@ -323,7 +323,7 @@ def script(line, escape_html = nil, preserve = false) end def flat_script(line, escape_html = nil) - raise SyntaxError.new(Error.message(:no_ruby_code, '~')) if line.text.empty? + raise SyntaxError.new(Error.message(:no_ruby_code, '~'.freeze)) if line.text.empty? script(line, escape_html, :preserve) end @@ -335,12 +335,12 @@ def silent_script(line) check_push_script_stack(keyword) - if ["else", "elsif", "when"].include?(keyword) + if ["else".freeze, "elsif".freeze, "when".freeze].include?(keyword) if @script_level_stack.empty? raise Haml::SyntaxError.new(Error.message(:missing_if, keyword), @line.index) end - if keyword == 'when' and !@script_level_stack.last[2] + if keyword == 'when'.freeze and !@script_level_stack.last[2] if @script_level_stack.last[1] + 1 == @line.tabs @script_level_stack.last[1] += 1 end @@ -358,11 +358,11 @@ def silent_script(line) end def check_push_script_stack(keyword) - if ["if", "case", "unless"].include?(keyword) + if ["if".freeze, "case".freeze, "unless".freeze].include?(keyword) # @script_level_stack contents are arrays of form # [:keyword, stack_level, other_info] @script_level_stack.push([keyword.to_sym, @line.tabs]) - @script_level_stack.last << false if keyword == 'case' + @script_level_stack.last << false if keyword == 'case'.freeze @tab_up = true end end @@ -386,18 +386,18 @@ def tag(line) preserve_tag = @options.preserve.include?(tag_name) nuke_inner_whitespace ||= preserve_tag - escape_html = (action == '&' || (action != '!' && @options.escape_html)) + escape_html = (action == '&'.freeze || (action != '!'.freeze && @options.escape_html)) case action - when '/'; self_closing = true - when '~'; parse = preserve_script = true - when '=' + when '/'.freeze; self_closing = true + when '~'.freeze; parse = preserve_script = true + when '='.freeze parse = true if value[0] == ?= value = unescape_interpolation(value[1..-1].strip, escape_html) escape_html = false end - when '&', '!' + when '&'.freeze, '!'.freeze if value[0] == ?= || value[0] == ?~ parse = true preserve_script = (value[0] == ?~) @@ -465,7 +465,7 @@ def div(line) # Renders an XHTML comment. def comment(text) - if text[0..1] == '![' + if text[0..1] == '!['.freeze revealed = true text = text[1..-1] else @@ -530,12 +530,12 @@ def close_flat_section end def close_silent_script(node) - @script_level_stack.pop if ["if", "case", "unless"].include? node.value[:keyword] + @script_level_stack.pop if ["if".freeze, "case".freeze, "unless".freeze].include? node.value[:keyword] # Post-process case statements to normalize the nesting of "when" clauses - return unless node.value[:keyword] == "case" + return unless node.value[:keyword] == "case".freeze return unless first = node.children.first - return unless first.type == :silent_script && first.value[:keyword] == "when" + return unless first.type == :silent_script && first.value[:keyword] == "when".freeze return if first.children.empty? # If the case node has a "when" child with children, it's the # only child. Then we want to put everything nested beneath it @@ -564,7 +564,7 @@ def self.parse_class_and_id(list) attributes[CLASS_KEY] = "".freeze end attributes[CLASS_KEY] += property - when '#'; attributes[ID_KEY] = property + when '#'.freeze; attributes[ID_KEY] = property end end attributes @@ -698,7 +698,7 @@ def parse_new_attributes(text) end static_attributes = {} - dynamic_attributes = "{" + dynamic_attributes = ["{".freeze] attributes.each do |name, (type, val)| if type == :static static_attributes[name] = val @@ -706,8 +706,9 @@ def parse_new_attributes(text) dynamic_attributes << "#{inspect_obj(name)} => #{val}," end end - dynamic_attributes << "}" - dynamic_attributes = nil if dynamic_attributes == "{}" + dynamic_attributes << "}".freeze + dynamic_attributes = dynamic_attributes.join(''.freeze) + dynamic_attributes = nil if dynamic_attributes == "{}".freeze return [static_attributes, dynamic_attributes], scanner.rest, last_line end @@ -731,7 +732,7 @@ def parse_new_attribute(scanner) content = [] loop do return false unless scanner.scan(re) - content << [:str, scanner[1].gsub(/\\(.)/, '\1')] + content << [:str, scanner[1].gsub(/\\(.)/, '\1'.freeze)] break if scanner[2] == quote content << [:ruby, balance(scanner, ?{, ?}, 1).first[0...-1]] end diff --git a/lib/haml/temple_engine.rb b/lib/haml/temple_engine.rb index 11e193ba32..62adf234ec 100644 --- a/lib/haml/temple_engine.rb +++ b/lib/haml/temple_engine.rb @@ -6,18 +6,18 @@ module Haml class TempleEngine < Temple::Engine define_options( - attr_wrapper: "'", + attr_wrapper: "'".freeze, autoclose: %w(area base basefont br col command embed frame hr img input isindex keygen link menuitem meta param source track wbr), encoding: nil, escape_attrs: true, escape_html: false, - filename: '(haml)', + filename: '(haml)'.freeze, format: :html5, hyphenate_data_attrs: true, line: 1, - mime_type: 'text/html', + mime_type: 'text/html'.freeze, preserve: %w(textarea pre code), remove_whitespace: false, suppress_eval: false, @@ -48,7 +48,7 @@ def compile(template) # # @return [String] def precompiled - encoding = Encoding.find(@encoding || '') + encoding = Encoding.find(@encoding || ''.freeze) return @precompiled.force_encoding(encoding) if encoding == Encoding::ASCII_8BIT return @precompiled.encode(encoding) end @@ -63,7 +63,7 @@ def precompiled_with_return_value # (see {file:REFERENCE.md#encodings the `:encoding` option}). # # @return [String] - def precompiled_with_ambles(local_names, after_preamble: '') + def precompiled_with_ambles(local_names, after_preamble: ''.freeze) preamble = < Date: Mon, 23 Oct 2017 16:52:13 -0700 Subject: [PATCH 4/6] Replace interpolated interpolation with cleaner method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In micro-benchmarking, this change performs within margin of error of the existing code but saves a string allocation each time the method is called (assuming the quote strings in the old code were frozen). ``` begin require "bundler/inline" rescue LoadError => e $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler" raise e end gemfile(true) do source "https://rubygems.org" gem "benchmark-ips" end def allocate_count GC.disable before = ObjectSpace.count_objects yield after = ObjectSpace.count_objects after.each { |k,v| after[k] = v - before[k] } after[:T_HASH] -= 1 # probe effect - we created the before hash. GC.enable result = after.reject { |k,v| v == 0 } GC.start result end interpolated = "hi" puts "'"' + interpolated + '"'" puts allocate_count { 1000.times { '"' + interpolated + '"' } } puts "%(\"\#{interpolated}\")" puts allocate_count { 1000.times { %("#{interpolated}") } } puts "\"\#{interpolated}\"" puts allocate_count { 1000.times { "\"#{interpolated}\"" } } Benchmark.ips do |x| x.report("'"' + interpolated + '"'") { '"' + interpolated + '"' } x.report("%(\"\#{interpolated}\")") { %("#{interpolated}") } x.report("\"\#{interpolated}\"") { "\"#{interpolated}\"" } x.compare! end ```ruby ``` ' + interpolated + ' {:FREE=>-1892, :T_STRING=>2052} %("#{interpolated}") {:FREE=>-1001, :T_STRING=>1000} "#{interpolated}" {:FREE=>-1001, :T_STRING=>1000} Warming up -------------------------------------- ' + interpolated + ' 81.706k i/100ms %("#{interpolated}") 106.128k i/100ms "#{interpolated}" 137.855k i/100ms Calculating ------------------------------------- ' + interpolated + ' 3.892M (±23.2%) i/s - 17.975M in 5.007068s %("#{interpolated}") 3.722M (±17.3%) i/s - 17.830M in 5.022549s "#{interpolated}" 3.725M (±15.0%) i/s - 18.059M in 5.023493s Comparison: ' + interpolated + ': 3892392.6 i/s "#{interpolated}": 3725385.8 i/s - same-ish: difference falls within error %("#{interpolated}"): 3722401.7 i/s - same-ish: difference falls within error ``` --- lib/haml/util.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/haml/util.rb b/lib/haml/util.rb index c03a77c19f..7a06a8f8be 100644 --- a/lib/haml/util.rb +++ b/lib/haml/util.rb @@ -212,7 +212,7 @@ def unescape_interpolation(str, escape_html = nil) else scan.scan(/\w+/) end - content = eval(%("#{interpolated}")) + content = eval("\"#{interpolated}\"") content.prepend(char) if char == '@'.freeze || char == '$'.freeze content = "Haml::Helpers.html_escape((#{content}))" if escape_html From de05e610e9aa158830c4d7a2c0374243ba7117eb Mon Sep 17 00:00:00 2001 From: Dillon Welch Date: Mon, 23 Oct 2017 17:10:02 -0700 Subject: [PATCH 5/6] Roll back changes to use array joins because it's slower --- lib/haml/parser.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/haml/parser.rb b/lib/haml/parser.rb index 364db03741..07840d3705 100644 --- a/lib/haml/parser.rb +++ b/lib/haml/parser.rb @@ -698,7 +698,7 @@ def parse_new_attributes(text) end static_attributes = {} - dynamic_attributes = ["{".freeze] + dynamic_attributes = "{" attributes.each do |name, (type, val)| if type == :static static_attributes[name] = val @@ -707,7 +707,6 @@ def parse_new_attributes(text) end end dynamic_attributes << "}".freeze - dynamic_attributes = dynamic_attributes.join(''.freeze) dynamic_attributes = nil if dynamic_attributes == "{}".freeze return [static_attributes, dynamic_attributes], scanner.rest, last_line From 2130e84b3feb0b9532b14e57872b161b7ae5eb5e Mon Sep 17 00:00:00 2001 From: Dillon Welch Date: Mon, 23 Oct 2017 17:34:53 -0700 Subject: [PATCH 6/6] Rewrite old attribute mapping to save many object allocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stringifying the keys of a hash can be done without allocating many arrays like the previous approach did. ```ruby begin require "bundler/inline" rescue LoadError => e $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler" raise e end gemfile(true) do source "https://rubygems.org" gem "benchmark-ips" end def allocate_count GC.disable before = ObjectSpace.count_objects yield after = ObjectSpace.count_objects after.each { |k,v| after[k] = v - before[k] } after[:T_HASH] -= 1 # probe effect - we created the before hash. GC.enable result = after.reject { |k,v| v == 0 } GC.start result end @old = {a: :b, c: :d, e: :f} def master_version Hash[@old.map { |k, v| [k.to_s, v] }] end def fast_version result = {} @old.each { |k, v| result[k.to_s] = v } end puts "master_version" puts allocate_count { 1000.times { master_version } } puts "fast_version" puts allocate_count { 1000.times { fast_version } } Benchmark.ips do |x| x.report("master_version") { master_version } x.report("fast_version") { fast_version } x.compare! end ``` ```ruby master_version {:FREE=>-14768, :T_STRING=>6054, :T_ARRAY=>7000, :T_HASH=>1000, :T_IMEMO=>1000} fast_version {:FREE=>-7001, :T_STRING=>6000, :T_HASH=>1000} Warming up -------------------------------------- master_version 38.137k i/100ms fast_version 50.133k i/100ms Calculating ------------------------------------- master_version 451.898k (±19.2%) i/s - 2.174M in 5.002186s fast_version 633.579k (±19.4%) i/s - 3.058M in 5.019391s Comparison: fast_version: 633578.7 i/s master_version: 451897.6 i/s - same-ish: difference falls within error ``` --- lib/haml/buffer.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/haml/buffer.rb b/lib/haml/buffer.rb index 81a4e853b0..8fd92bbc4e 100644 --- a/lib/haml/buffer.rb +++ b/lib/haml/buffer.rb @@ -132,7 +132,9 @@ def adjust_tabs(tab_change) def attributes(class_id, obj_ref, *attributes_hashes) attributes = class_id attributes_hashes.each do |old| - AttributeBuilder.merge_attributes!(attributes, Hash[old.map {|k, v| [k.to_s, v]}]) + result = {} + old.each { |k, v| result[k.to_s] = v } + AttributeBuilder.merge_attributes!(attributes, result) end AttributeBuilder.merge_attributes!(attributes, parse_object_ref(obj_ref)) if obj_ref AttributeBuilder.build_attributes(