From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-2.9 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, URIBL_BLOCKED shortcircuit=no autolearn=unavailable version=3.3.2 X-Original-To: spew@80x24.org Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 24056200FE; Wed, 16 Sep 2015 09:25:28 +0000 (UTC) Date: Wed, 16 Sep 2015 09:25:28 +0000 From: Eric Wong To: spew@80x24.org Subject: [PATCH] pathname: reduce object allocation Message-ID: <20150916-Feature-11375-tweak@v1> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline List-Id: 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 self + '..'. 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