-
-
Notifications
You must be signed in to change notification settings - Fork 571
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.
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
Frozen string literal changes #967
Changes from 4 commits
bbea4b3
9d22438
285c988
d3447c7
57b1e7a
b5a0d7b
a7abfae
24d97ab
4cfb541
f32a81d
17dd57a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# frozen_string_literal: false | ||
# frozen_string_literal: true | ||
require 'erb' | ||
|
||
module Haml | ||
|
@@ -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('|') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: 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
|
||
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 | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
enum.map do |i| | ||
result = capture_haml(i, &block) | ||
|
||
if result.count("\n") > 1 | ||
|
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
end | ||
|
||
# Returns a hash containing default assignments for the `xmlns`, `lang`, and `xml:lang` | ||
|
@@ -704,4 +700,3 @@ def is_haml? | |
false | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -698,7 +698,7 @@ def parse_new_attributes(text) | |
end | ||
|
||
static_attributes = {} | ||
dynamic_attributes = "{" | ||
dynamic_attributes = ["{"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't impact performance or memory either way but is necessary for future # 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
dynamic_attributes = '{'
if rand(2) == 1
dynamic_attributes << 'hi there'.freeze
end
dynamic_attributes << "}".freeze
dynamic_attributes = nil if dynamic_attributes == "{}".freeze
end
def fast_version
dynamic_attributes = ['{'.freeze]
if rand(2) == 1
dynamic_attributes << 'hi there'.freeze
end
dynamic_attributes << "}".freeze
dynamic_attributes = nil if dynamic_attributes.first == "{}".freeze && dynamic_attributes.length == 1
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for your effort of gradual changes and to put benchmark, but please exclude intermediate changes that have no benefit. It makes hard to read commit log to know why this change was necessary, and this change should be committed with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And there's no need to change this to Array buffer for introducing It uglifies code after this, so please use |
||
attributes.each do |name, (type, val)| | ||
if type == :static | ||
static_attributes[name] = val | ||
|
@@ -707,8 +707,12 @@ def parse_new_attributes(text) | |
end | ||
end | ||
dynamic_attributes << "}" | ||
dynamic_attributes = nil if dynamic_attributes == "{}" | ||
|
||
if dynamic_attributes.first == "{}" && | ||
dynamic_attributes.length == 1 | ||
dynamic_attributes = nil | ||
else | ||
dynamic_attributes = dynamic_attributes.join | ||
end | ||
return [static_attributes, dynamic_attributes], scanner.rest, last_line | ||
end | ||
|
||
|
@@ -737,8 +741,10 @@ def parse_new_attribute(scanner) | |
end | ||
|
||
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}}")}}"!] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't introduce unnecessary changes. Obviously we can enable |
||
str = content.map do |t, v| | ||
t == :str ? inspect_obj(v)[1...-1] : "\#{#{v}}" | ||
end.join | ||
return name, [:dynamic, %!"#{str}"!] | ||
end | ||
|
||
def next_line | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# frozen_string_literal: true | ||
require 'test_helper' | ||
|
||
class AttributeParserTeset < Haml::TestCase | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# frozen_string_literal: true | ||
require 'test_helper' | ||
|
||
class FiltersTest < Haml::TestCase | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# frozen_string_literal: true | ||
require 'test_helper' | ||
require "active_model/naming" | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# frozen_string_literal: true | ||
require 'test_helper' | ||
|
||
module Haml | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# frozen_string_literal: true | ||
module TemplateTestHelper | ||
TEMPLATE_PATH = File.join(__dir__, "templates") | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# frozen_string_literal: true | ||
require 'test_helper' | ||
|
||
class TempleLineCounterTest < Haml::TestCase | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
# frozen_string_literal: true | ||
require 'test_helper' | ||
|
||
class UtilTest < Haml::TestCase | ||
|
There was a problem hiding this comment.
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).