* [PATCH] pathname: reduce object allocation
@ 2015-09-16 9:25 Eric Wong
2015-09-16 9:26 ` Eric Wong
0 siblings, 1 reply; 2+ messages in thread
From: Eric Wong @ 2015-09-16 9:25 UTC (permalink / raw)
To: spew
From schneems:
* ext/pathname/lib/pathname.rb: freeze string literals for
reduced object allocation.
[Feature #11375]
[ew:
- tweak to avoid introducing long lines, keep at 80 columns
(I use gigantic fonts)
- avoid freezing for case dispatch, that is already optimized
- favor concatstrings insn ("#{path}a" instead of: path + 'a'.freeze)
since concatstrings already avoids duplicating literals
... Possible future optimizations I noticed:
* Avoid: while !foo.empty? && foo.first == '.'.freeze
Instead: while foo[0] == '.'.freeze
This avoids an extra method call and Array#[] has optimized
dispatch for a single numeric arg. An empty array will
return nil which does not match '.'
]
ref: https://bugs.ruby-lang.org/issues/11375
---
ext/pathname/lib/pathname.rb | 55 +++++++++++++++++++++++++-------------------
1 file changed, 31 insertions(+), 24 deletions(-)
diff --git a/ext/pathname/lib/pathname.rb b/ext/pathname/lib/pathname.rb
index 9f23ba5..b41ec99 100644
--- a/ext/pathname/lib/pathname.rb
+++ b/ext/pathname/lib/pathname.rb
@@ -61,7 +61,9 @@ class Pathname
File.dirname(prefix)
elsif /#{SEPARATOR_PAT}/o =~ prefix
prefix = File.dirname(prefix)
- prefix = File.join(prefix, "") if File.basename(prefix + 'a') != 'a'
+ if File.basename("#{prefix}a") != 'a'.freeze
+ prefix = File.join(prefix, "".freeze)
+ end
prefix + relpath
else
prefix + relpath
@@ -102,7 +104,7 @@ class Pathname
when '..'
names.unshift base
else
- if names[0] == '..'
+ if names[0] == '..'.freeze
names.shift
else
names.unshift base
@@ -111,7 +113,7 @@ class Pathname
end
pre.tr!(File::ALT_SEPARATOR, File::SEPARATOR) if File::ALT_SEPARATOR
if /#{SEPARATOR_PAT}/o =~ File.basename(pre)
- names.shift while names[0] == '..'
+ names.shift while names[0] == '..'.freeze
end
self.class.new(prepend_prefix(pre, File.join(*names)))
end
@@ -130,10 +132,11 @@ class Pathname
# add_trailing_separator(path) -> path
def add_trailing_separator(path) # :nodoc:
- if File.basename(path + 'a') == 'a'
+ if File.basename("#{path}a") == 'a'.freeze
path
else
- File.join(path, "") # xxx: Is File.join is appropriate to add separator?
+ # xxx: Is File.join is appropriate to add separator?
+ File.join(path, "".freeze)
end
end
private :add_trailing_separator
@@ -156,16 +159,16 @@ class Pathname
pre = path
while r = chop_basename(pre)
pre, base = r
- names.unshift base if base != '.'
+ names.unshift base if base != '.'.freeze
end
pre.tr!(File::ALT_SEPARATOR, File::SEPARATOR) if File::ALT_SEPARATOR
if /#{SEPARATOR_PAT}/o =~ File.basename(pre)
- names.shift while names[0] == '..'
+ names.shift while names[0] == '..'.freeze
end
if names.empty?
self.class.new(File.dirname(pre))
else
- if names.last != '..' && File.basename(path) == '.'
+ if names.last != '..'.freeze && File.basename(path) == '.'.freeze
names << '.'
end
result = prepend_prefix(pre, File.join(*names))
@@ -182,7 +185,7 @@ class Pathname
#
# This is same as <code>self + '..'</code>.
def parent
- self + '..'
+ self + '..'.freeze
end
# Returns +true+ if +self+ points to a mountpoint.
@@ -239,7 +242,7 @@ class Pathname
while r = chop_basename(path)
path, = r
end
- path == ''
+ path == ''.freeze
end
#
@@ -359,17 +362,19 @@ class Pathname
index_list2.unshift prefix2.length
basename_list2.unshift basename2
end
- return path2 if prefix2 != ''
+ return path2 if prefix2 != ''.freeze
prefix1 = path1
while true
- while !basename_list2.empty? && basename_list2.first == '.'
+ while !basename_list2.empty? && basename_list2.first == '.'.freeze
index_list2.shift
basename_list2.shift
end
break unless r1 = chop_basename(prefix1)
prefix1, basename1 = r1
- next if basename1 == '.'
- if basename1 == '..' || basename_list2.empty? || basename_list2.first != '..'
+ next if basename1 == '.'.freeze
+ if basename1 == '..'.freeze ||
+ basename_list2.empty? ||
+ basename_list2.first != '..'.freeze
prefix1 = prefix1 + basename1
break
end
@@ -378,7 +383,7 @@ class Pathname
end
r1 = chop_basename(prefix1)
if !r1 && /#{SEPARATOR_PAT}/o =~ File.basename(prefix1)
- while !basename_list2.empty? && basename_list2.first == '..'
+ while !basename_list2.empty? && basename_list2.first == '..'.freeze
index_list2.shift
basename_list2.shift
end
@@ -436,10 +441,10 @@ class Pathname
# the directory because they are not children.
#
def children(with_directory=true)
- with_directory = false if @path == '.'
+ with_directory = false if @path == '.'.freeze
result = []
Dir.foreach(@path) {|e|
- next if e == '.' || e == '..'
+ next if e == '.'.freeze || e == '..'.freeze
if with_directory
result << self.class.new(File.join(@path, e))
else
@@ -507,13 +512,13 @@ class Pathname
dest_names = []
while r = chop_basename(dest_prefix)
dest_prefix, basename = r
- dest_names.unshift basename if basename != '.'
+ dest_names.unshift basename if basename != '.'.freeze
end
base_prefix = base_directory
base_names = []
while r = chop_basename(base_prefix)
base_prefix, basename = r
- base_names.unshift basename if basename != '.'
+ base_names.unshift basename if basename != '.'.freeze
end
unless SAME_PATHS[dest_prefix, base_prefix]
raise ArgumentError, "different prefix: #{dest_prefix.inspect} and #{base_directory.inspect}"
@@ -524,13 +529,13 @@ class Pathname
dest_names.shift
base_names.shift
end
- if base_names.include? '..'
+ if base_names.include? '..'.freeze
raise ArgumentError, "base_directory has ..: #{base_directory.inspect}"
end
- base_names.fill('..')
+ base_names.fill('..'.freeze)
relpath_names = base_names + dest_names
if relpath_names.empty?
- Pathname.new('.')
+ Pathname.new('.'.freeze)
else
Pathname.new(File.join(*relpath_names))
end
@@ -556,8 +561,10 @@ class Pathname # * Find *
def find(ignore_error: true) # :yield: pathname
return to_enum(__method__, ignore_error: ignore_error) unless block_given?
require 'find'
- if @path == '.'
- Find.find(@path, ignore_error: ignore_error) {|f| yield self.class.new(f.sub(%r{\A\./}, '')) }
+ if @path == '.'.freeze
+ Find.find(@path, ignore_error: ignore_error) do |f|
+ yield self.class.new(f.sub(%r{\A\./}, ''.freeze))
+ end
else
Find.find(@path, ignore_error: ignore_error) {|f| yield self.class.new(f) }
end
--
EW
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] pathname: reduce object allocation
2015-09-16 9:25 [PATCH] pathname: reduce object allocation Eric Wong
@ 2015-09-16 9:26 ` Eric Wong
0 siblings, 0 replies; 2+ messages in thread
From: Eric Wong @ 2015-09-16 9:26 UTC (permalink / raw)
To: spew
diff -u b/ext/pathname/lib/pathname.rb b/ext/pathname/lib/pathname.rb
--- b/ext/pathname/lib/pathname.rb
+++ b/ext/pathname/lib/pathname.rb
@@ -61,7 +61,9 @@
File.dirname(prefix)
elsif /#{SEPARATOR_PAT}/o =~ prefix
prefix = File.dirname(prefix)
- prefix = File.join(prefix, "".freeze) if File.basename(prefix + 'a'.freeze) != 'a'.freeze
+ if File.basename("#{prefix}a") != 'a'.freeze
+ prefix = File.join(prefix, "".freeze)
+ end
prefix + relpath
else
prefix + relpath
@@ -98,8 +100,8 @@
while r = chop_basename(pre)
pre, base = r
case base
- when '.'.freeze
- when '..'.freeze
+ when '.'
+ when '..'
names.unshift base
else
if names[0] == '..'.freeze
@@ -130,10 +132,11 @@
# add_trailing_separator(path) -> path
def add_trailing_separator(path) # :nodoc:
- if File.basename(path + 'a'.freeze) == 'a'.freeze
+ if File.basename("#{path}a") == 'a'.freeze
path
else
- File.join(path, "".freeze) # xxx: Is File.join is appropriate to add separator?
+ # xxx: Is File.join is appropriate to add separator?
+ File.join(path, "".freeze)
end
end
private :add_trailing_separator
@@ -369,7 +372,9 @@
break unless r1 = chop_basename(prefix1)
prefix1, basename1 = r1
next if basename1 == '.'.freeze
- if basename1 == '..'.freeze || basename_list2.empty? || basename_list2.first != '..'.freeze
+ if basename1 == '..'.freeze ||
+ basename_list2.empty? ||
+ basename_list2.first != '..'.freeze
prefix1 = prefix1 + basename1
break
end
@@ -557,7 +562,9 @@
return to_enum(__method__, ignore_error: ignore_error) unless block_given?
require 'find'
if @path == '.'.freeze
- Find.find(@path, ignore_error: ignore_error) {|f| yield self.class.new(f.sub(%r{\A\./}, ''.freeze)) }
+ Find.find(@path, ignore_error: ignore_error) do |f|
+ yield self.class.new(f.sub(%r{\A\./}, ''.freeze))
+ end
else
Find.find(@path, ignore_error: ignore_error) {|f| yield self.class.new(f) }
end
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-09-16 9:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-16 9:25 [PATCH] pathname: reduce object allocation Eric Wong
2015-09-16 9:26 ` Eric Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).