Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Frozen string literal changes #967

Merged
merged 11 commits into from Oct 29, 2017
Merged

Frozen string literal changes #967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bbea4b3
Frozen string
dillonwelch Oct 26, 2017
9d22438
Revert changes that are slower
dillonwelch Oct 26, 2017
285c988
Add object to map conversion for temple engine
dillonwelch Oct 26, 2017
d3447c7
Revert more frozen strings
dillonwelch Oct 26, 2017
57b1e7a
Get rid of unnecessary extra array
dillonwelch Oct 26, 2017
b5a0d7b
Revert this change because it's slower in benchmarks
dillonwelch Oct 26, 2017
a7abfae
parser
dillonwelch Oct 28, 2017
24d97ab
true for filters
dillonwelch Oct 28, 2017
4cfb541
compiler
dillonwelch Oct 28, 2017
f32a81d
temple
dillonwelch Oct 28, 2017
17dd57a
finalize
dillonwelch Oct 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/haml/compiler.rb
View file
Open in desktop
@@ -1,4 +1,4 @@
# frozen_string_literal: false
# frozen_string_literal: true
require 'haml/attribute_builder'
require 'haml/attribute_compiler'
require 'haml/temple_line_counter'
Expand Down Expand Up @@ -315,7 +315,7 @@ def rstrip_buffer!(index = -1)

case last.first
when :text
last[1].rstrip!
last[1] = last[1].rstrip
if last[1].empty?
@to_merge.slice! index
rstrip_buffer! index
Expand Down
11 changes: 8 additions & 3 deletions lib/haml/engine.rb
View file
Open in desktop
@@ -1,4 +1,4 @@
# frozen_string_literal: false
# frozen_string_literal: true
require 'forwardable'

require 'haml/parser'
Expand Down Expand Up @@ -166,8 +166,13 @@ def render_proc(scope = Object.new, *local_names)
end

begin
eval("Proc.new { |*_haml_locals| _haml_locals = _haml_locals[0] || {};" <<
@temple_engine.precompiled_with_ambles(local_names) << "}\n", scope, @options.filename, @options.line)
str = @temple_engine.precompiled_with_ambles(local_names)
eval(
"Proc.new { |*_haml_locals| _haml_locals = _haml_locals[0] || {}; #{str}}\n",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is faster on micro-benchmarking as well as saving memory (2 strings per method call).

# frozen_string_literal: false
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"
  gem "rails"
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

def master_version
  "hi there" << "cool" << "yeah"
end

def fast_version
  "#{'hi there'}#{'cool'}#{'yeah'}"
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
master_version
{:FREE=>-2846, :T_STRING=>3052}
fast_version
{:FREE=>-1001, :T_STRING=>1000}
Warming up --------------------------------------
      master_version   120.220k i/100ms
        fast_version   181.848k i/100ms
Calculating -------------------------------------
      master_version      2.527M (±14.8%) i/s -     12.383M in   5.039857s
        fast_version      6.634M (±17.6%) i/s -     32.005M in   5.002381s

Comparison:
        fast_version:  6633843.3 i/s
      master_version:  2526801.8 i/s - 2.63x  slower

scope,
@options.filename,
@options.line
)
rescue ::SyntaxError => e
raise SyntaxError, e.message
end
Expand Down
10 changes: 3 additions & 7 deletions lib/haml/filters.rb
View file
Open in desktop
@@ -1,4 +1,4 @@
# frozen_string_literal: false
# frozen_string_literal: true
require "tilt"

module Haml
Expand Down Expand Up @@ -183,8 +183,7 @@ def compile(compiler, text)
end

rendered = Haml::Helpers::find_and_preserve(filter.render_with_options(text, compiler.options), compiler.options[:preserve])
rendered.rstrip!
push_text("#{rendered}\n")
push_text("#{rendered.rstrip}\n")
end
end
end
Expand Down Expand Up @@ -247,10 +246,7 @@ module Cdata

# @see Base#render
def render(text)
text = "\n#{text}"
text.rstrip!
text.gsub!("\n", "\n ")
"<![CDATA[#{text}\n]]>"
"<![CDATA[#{"\n#{text.rstrip}".gsub("\n", "\n ")}\n]]>"
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/haml/generator.rb
View file
Open in desktop
@@ -1,4 +1,4 @@
# frozen_string_literal: false
# frozen_string_literal: true
module Haml
# Ruby code generator, which is a limited version of Temple::Generator.
# Limit methods since Haml doesn't need most of them.
Expand Down
17 changes: 6 additions & 11 deletions lib/haml/helpers.rb
View file
Open in desktop
@@ -1,4 +1,4 @@
# frozen_string_literal: false
# frozen_string_literal: true
require 'erb'

module Haml
Expand Down Expand Up @@ -109,10 +109,7 @@ def non_haml
# @yield The block within which to escape newlines
def find_and_preserve(input = nil, tags = haml_buffer.options[:preserve], &block)
return find_and_preserve(capture_haml(&block), input || tags) if block
tags = tags.each_with_object('') do |t, s|
s << '|' unless s.empty?
s << Regexp.escape(t)
end
tags = tags.map { |tag| Regexp.escape(tag) }.join('|')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't impact performance or memory either way but was necessary for the frozen_string_literal: true change

# frozen_string_literal: false
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"
  gem "rails"
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

@tags = %w(hi there I am a list of tags)

def master_version
  @tags.each_with_object('') do |t, s|
    s << '|'.freeze unless s.empty?
    s << Regexp.escape(t)
  end
end

def fast_version
  @tags.map { |tag| Regexp.escape(tag) }.join('|'.freeze)
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
master_version
{:FREE=>-9790, :T_STRING=>9052, :T_IMEMO=>1001}
fast_version
{:FREE=>-10001, :T_STRING=>9000, :T_ARRAY=>1000}
Warming up --------------------------------------
      master_version    13.960k i/100ms
        fast_version    16.597k i/100ms
Calculating -------------------------------------
      master_version    200.727k (±15.4%) i/s -    991.160k in   5.057382s
        fast_version    206.033k (±11.7%) i/s -      1.029M in   5.072112s

Comparison:
        fast_version:   206033.4 i/s
      master_version:   200727.0 i/s - same-ish: difference falls within error

re = /<(#{tags})([^>]*)>(.*?)(<\/\1>)/im
input.to_s.gsub(re) do |s|
s =~ re # Can't rely on $1, etc. existing since Rails' SafeBuffer#gsub is incompatible
Expand Down Expand Up @@ -200,8 +197,8 @@ def preserve(input = nil, &block)
# @yield [item] A block which contains Haml code that goes within list items
# @yieldparam item An element of `enum`
def list_of(enum, opts={}, &block)
opts_attributes = opts.each_with_object('') {|(k, v), s| s << " #{k}='#{v}'"}
enum.each_with_object('') do |i, ret|
opts_attributes = opts.map { |k, v| " #{k}='#{v}'" }.join
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

# frozen_string_literal: false
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"
  gem "rails"
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

@strings = %w(hi there I am an array of strings)

def master_version
  @strings.each_with_object('') do |string, builder|
    builder << "#{string} is cool "
  end
end

def fast_version
  @strings.map do |string|
    "#{string} is cool "
  end.join
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
master_version
{:FREE=>-9836, :T_STRING=>9052, :T_IMEMO=>1001}
fast_version
{:FREE=>-10001, :T_STRING=>9000, :T_ARRAY=>1000}
Warming up --------------------------------------
      master_version    23.988k i/100ms
        fast_version    29.611k i/100ms
Calculating -------------------------------------
      master_version    302.295k (± 8.2%) i/s -      1.511M in   5.034657s
        fast_version    359.883k (±12.2%) i/s -      1.777M in   5.015867s

Comparison:
        fast_version:   359883.3 i/s
      master_version:   302295.1 i/s - same-ish: difference falls within error

enum.map do |i|
result = capture_haml(i, &block)

if result.count("\n") > 1
Expand All @@ -211,9 +208,8 @@ def list_of(enum, opts={}, &block)
result.strip!
end

ret << "\n" unless ret.empty?
ret << %Q!<li#{opts_attributes}>#{result}</li>!
end
%Q!<li#{opts_attributes}>#{result}</li>!
end.join("\n")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

# frozen_string_literal: false
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"
  gem "rails"
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

@strings = %w(hi there I am an array of strings)

def master_version
  @strings.each_with_object('') do |string, builder|
    builder << "\n".freeze unless builder.empty?
    builder << "#{string} is cool "
  end
end

def fast_version
  @strings.map do |string|
    "#{string} is cool "
  end.join("\n".freeze)
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
master_version
{:FREE=>-9773, :T_STRING=>9052, :T_IMEMO=>1001}
fast_version
{:FREE=>-10001, :T_STRING=>9000, :T_ARRAY=>1000}
Warming up --------------------------------------
      master_version    23.523k i/100ms
        fast_version    18.124k i/100ms
Calculating -------------------------------------
      master_version    271.699k (± 9.1%) i/s -      1.364M in   5.074300s
        fast_version    280.265k (±13.2%) i/s -      1.377M in   5.012751s

Comparison:
        fast_version:   280265.0 i/s
      master_version:   271698.9 i/s - same-ish: difference falls within error

end

# Returns a hash containing default assignments for the `xmlns`, `lang`, and `xml:lang`
Expand Down Expand Up @@ -704,4 +700,3 @@ def is_haml?
false
end
end

6 changes: 3 additions & 3 deletions lib/haml/parser.rb
View file
Open in desktop
@@ -1,4 +1,4 @@
# frozen_string_literal: false
# frozen_string_literal: true
require 'strscan'

module Haml
Expand Down Expand Up @@ -698,7 +698,7 @@ def parse_new_attributes(text)
end

static_attributes = {}
dynamic_attributes = "{"
dynamic_attributes = "{".dup
attributes.each do |name, (type, val)|
if type == :static
static_attributes[name] = val
Expand Down Expand Up @@ -738,7 +738,7 @@ def parse_new_attribute(scanner)

return name, [:static, content.first[1]] if content.size == 1
return name, [:dynamic,
%!"#{content.each_with_object('') {|(t, v), s| s << (t == :str ? inspect_obj(v)[1...-1] : "\#{#{v}}")}}"!]
%!"#{content.each_with_object(''.dup) {|(t, v), s| s << (t == :str ? inspect_obj(v)[1...-1] : "\#{#{v}}")}}"!]
end

def next_line
Expand Down
14 changes: 7 additions & 7 deletions lib/haml/temple_engine.rb
View file
Open in desktop
@@ -1,4 +1,4 @@
# frozen_string_literal: false
# frozen_string_literal: true
require 'temple'
require 'haml/escapable'
require 'haml/generator'
Expand Down Expand Up @@ -49,7 +49,7 @@ def compile(template)
# @return [String]
def precompiled
encoding = Encoding.find(@encoding || '')
return @precompiled.force_encoding(encoding) if encoding == Encoding::ASCII_8BIT
return @precompiled.dup.force_encoding(encoding) if encoding == Encoding::ASCII_8BIT
return @precompiled.encode(encoding)
end

Expand All @@ -64,14 +64,14 @@ def precompiled_with_return_value
#
# @return [String]
def precompiled_with_ambles(local_names, after_preamble: '')
preamble = <<END.tr!("\n", ';')
preamble = <<END.tr("\n", ';')
begin
extend Haml::Helpers
_hamlout = @haml_buffer = Haml::Buffer.new(haml_buffer, #{Options.new(options).for_buffer.inspect})
_erbout = _hamlout.buffer
#{after_preamble}
END
postamble = <<END.tr!("\n", ';')
postamble = <<END.tr("\n", ';')
#{precompiled_method_return_value}
ensure
@haml_buffer = @haml_buffer.upper if @haml_buffer
Expand Down Expand Up @@ -99,12 +99,12 @@ def precompiled_method_return_value
def locals_code(names)
names = names.keys if Hash === names

names.each_with_object('') do |name, code|
names.map do |name|
# Can't use || because someone might explicitly pass in false with a symbol
sym_local = "_haml_locals[#{inspect_obj(name.to_sym)}]"
str_local = "_haml_locals[#{inspect_obj(name.to_s)}]"
code << "#{name} = #{sym_local}.nil? ? #{str_local} : #{sym_local};"
end
"#{name} = #{sym_local}.nil? ? #{str_local} : #{sym_local};"
end.join
end

def inspect_obj(obj)
Expand Down
6 changes: 3 additions & 3 deletions lib/haml/util.rb
View file
Open in desktop
@@ -1,4 +1,4 @@
# frozen_string_literal: false
# frozen_string_literal: true

begin
require 'erubis/tiny'
Expand Down Expand Up @@ -166,7 +166,7 @@ def handle_interpolation(str)
# and the rest of the string.
# `["Foo (Bar (Baz bang) bop)", " (Bang (bop bip))"]` in the example above.
def balance(scanner, start, finish, count = 0)
str = ''
str = ''.dup
scanner = StringScanner.new(scanner) unless scanner.is_a? StringScanner
regexp = Regexp.new("(.*?)[\\#{start.chr}\\#{finish.chr}]", Regexp::MULTILINE)
while scanner.scan(regexp)
Expand Down Expand Up @@ -199,7 +199,7 @@ def contains_interpolation?(str)
end

def unescape_interpolation(str, escape_html = nil)
res = ''
res = ''.dup
rest = Haml::Util.handle_interpolation str.dump do |scan|
escapes = (scan[2].size - 1) / 2
char = scan[3] # '{', '@' or '$'
Expand Down
1 change: 1 addition & 0 deletions test/attribute_parser_test.rb
View file
Open in desktop
@@ -1,3 +1,4 @@
# frozen_string_literal: true
require 'test_helper'

class AttributeParserTeset < Haml::TestCase
Expand Down
1 change: 1 addition & 0 deletions test/filters_test.rb
View file
Open in desktop
@@ -1,3 +1,4 @@
# frozen_string_literal: true
require 'test_helper'

class FiltersTest < Haml::TestCase
Expand Down
1 change: 1 addition & 0 deletions test/helper_test.rb
View file
Open in desktop
@@ -1,3 +1,4 @@
# frozen_string_literal: true
require 'test_helper'
require "active_model/naming"

Expand Down
1 change: 1 addition & 0 deletions test/options_test.rb
View file
Open in desktop
@@ -1,3 +1,4 @@
# frozen_string_literal: true
require 'test_helper'

module Haml
Expand Down
3 changes: 2 additions & 1 deletion test/parser_test.rb
View file
Open in desktop
@@ -1,3 +1,4 @@
# frozen_string_literal: true
require 'test_helper'

module Haml
Expand Down Expand Up @@ -66,7 +67,7 @@ class ParserTest < Haml::TestCase
flunk 'else clause after if containing unless should be accepted'
end
end

test "loud script with else is accepted" do
begin
parse "= if true\n - 'A'\n-else\n - 'B'"
Expand Down
1 change: 1 addition & 0 deletions test/template_test_helper.rb
View file
Open in desktop
@@ -1,3 +1,4 @@
# frozen_string_literal: true
module TemplateTestHelper
TEMPLATE_PATH = File.join(__dir__, "templates")
end
Expand Down
1 change: 1 addition & 0 deletions test/temple_line_counter_test.rb
View file
Open in desktop
@@ -1,3 +1,4 @@
# frozen_string_literal: true
require 'test_helper'

class TempleLineCounterTest < Haml::TestCase
Expand Down
1 change: 1 addition & 0 deletions test/util_test.rb
View file
Open in desktop
@@ -1,3 +1,4 @@
# frozen_string_literal: true
require 'test_helper'

class UtilTest < Haml::TestCase
Expand Down