[PATCH] pathname: reduce object allocation

[Eric Wong <e@80x24.org>]

 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