mirror of
https://github.com/ruby/ruby.git
synced 2022-11-09 12:17:21 -05:00
fa12e3e2f7
I think it's debatable which is the most common usage of
`FileUtils.mkdir_p`, but even assuming the most common use case is
creating a folder when it doesn't previously exist but the parent does,
this optimization doesn't seem to have a noticiable effect there while
harming other use cases.
For benchmarks, I created this script
```ruby
require "benchmark/ips"
Benchmark.ips do |x|
x.report("old mkdir_p - exists") do
FileUtils.mkdir_p "/tmp"
end
x.report("new_mkdir_p - exists") do
FileUtils.mkdir_p_new "/tmp"
end
x.compare!
end
FileUtils.rm_rf "/tmp/foo"
Benchmark.ips do |x|
x.report("old mkdir_p - doesnt exist, parent exists") do
FileUtils.mkdir_p "/tmp/foo"
FileUtils.rm_rf "/tmp/foo"
end
x.report("new_mkdir_p - doesnt exist, parent exists") do
FileUtils.mkdir_p_new "/tmp/foo"
FileUtils.rm_rf "/tmp/foo"
end
x.compare!
end
Benchmark.ips do |x|
x.report("old mkdir_p - doesnt exist, parent either") do
FileUtils.mkdir_p "/tmp/foo/bar"
FileUtils.rm_rf "/tmp/foo"
end
x.report("new_mkdir_p - doesnt exist, parent either") do
FileUtils.mkdir_p_new "/tmp/foo/bar"
FileUtils.rm_rf "/tmp/foo"
end
x.compare!
end
Benchmark.ips do |x|
x.report("old mkdir_p - more levels") do
FileUtils.mkdir_p "/tmp/foo/bar/baz"
FileUtils.rm_rf "/tmp/foo"
end
x.report("new_mkdir_p - more levels") do
FileUtils.mkdir_p_new "/tmp/foo/bar/baz"
FileUtils.rm_rf "/tmp/foo"
end
x.compare!
end
```
and copied the method with the "optimization" removed as
`FileUtils.mkdir_p_new`. The results are as below:
```
Warming up --------------------------------------
old mkdir_p - exists 15.914k i/100ms
new_mkdir_p - exists 46.512k i/100ms
Calculating -------------------------------------
old mkdir_p - exists 161.461k (± 3.2%) i/s - 811.614k in 5.032315s
new_mkdir_p - exists 468.192k (± 2.9%) i/s - 2.372M in 5.071225s
Comparison:
new_mkdir_p - exists: 468192.1 i/s
old mkdir_p - exists: 161461.0 i/s - 2.90x (± 0.00) slower
Warming up --------------------------------------
old mkdir_p - doesnt exist, parent exists
2.142k i/100ms
new_mkdir_p - doesnt exist, parent exists
1.961k i/100ms
Calculating -------------------------------------
old mkdir_p - doesnt exist, parent exists
21.242k (± 6.7%) i/s - 107.100k in 5.069206s
new_mkdir_p - doesnt exist, parent exists
19.682k (± 4.2%) i/s - 100.011k in 5.091961s
Comparison:
old mkdir_p - doesnt exist, parent exists: 21241.7 i/s
new_mkdir_p - doesnt exist, parent exists: 19681.7 i/s - same-ish: difference falls within error
Warming up --------------------------------------
old mkdir_p - doesnt exist, parent either
945.000 i/100ms
new_mkdir_p - doesnt exist, parent either
1.002k i/100ms
Calculating -------------------------------------
old mkdir_p - doesnt exist, parent either
9.689k (± 4.4%) i/s - 49.140k in 5.084342s
new_mkdir_p - doesnt exist, parent either
10.806k (± 4.6%) i/s - 54.108k in 5.020714s
Comparison:
new_mkdir_p - doesnt exist, parent either: 10806.3 i/s
old mkdir_p - doesnt exist, parent either: 9689.3 i/s - 1.12x (± 0.00) slower
Warming up --------------------------------------
old mkdir_p - more levels
702.000 i/100ms
new_mkdir_p - more levels
775.000 i/100ms
Calculating -------------------------------------
old mkdir_p - more levels
7.046k (± 3.5%) i/s - 35.802k in 5.087548s
new_mkdir_p - more levels
7.685k (± 5.5%) i/s - 38.750k in 5.061351s
Comparison:
new_mkdir_p - more levels: 7685.1 i/s
old mkdir_p - more levels: 7046.4 i/s - same-ish: difference falls within error
```
I think it's better to keep the code simpler is the optimization is not
so clear like in this case.
https://github.com/ruby/fileutils/commit/e842a0e70e
|
||
|---|---|---|
| .. | ||
| benchmark | ||
| bundler | ||
| cgi | ||
| csv | ||
| delegate | ||
| did_you_mean | ||
| drb | ||
| erb | ||
| error_highlight | ||
| forwardable | ||
| getoptlong | ||
| irb | ||
| logger | ||
| net | ||
| observer | ||
| optparse | ||
| ostruct | ||
| pstore | ||
| racc | ||
| rdoc | ||
| reline | ||
| rinda | ||
| rubygems | ||
| set | ||
| singleton | ||
| timeout | ||
| unicode_normalize | ||
| uri | ||
| weakref | ||
| yaml | ||
| .document | ||
| abbrev.gemspec | ||
| abbrev.rb | ||
| base64.gemspec | ||
| base64.rb | ||
| benchmark.rb | ||
| bundler.rb | ||
| cgi.rb | ||
| csv.rb | ||
| delegate.rb | ||
| did_you_mean.rb | ||
| drb.rb | ||
| English.gemspec | ||
| English.rb | ||
| erb.gemspec | ||
| erb.rb | ||
| error_highlight.rb | ||
| fileutils.gemspec | ||
| fileutils.rb | ||
| find.gemspec | ||
| find.rb | ||
| forwardable.rb | ||
| getoptlong.rb | ||
| ipaddr.gemspec | ||
| ipaddr.rb | ||
| irb.rb | ||
| logger.rb | ||
| mkmf.rb | ||
| mutex_m.gemspec | ||
| mutex_m.rb | ||
| observer.rb | ||
| open-uri.gemspec | ||
| open-uri.rb | ||
| open3.gemspec | ||
| open3.rb | ||
| optionparser.rb | ||
| optparse.rb | ||
| ostruct.rb | ||
| pp.gemspec | ||
| pp.rb | ||
| prettyprint.gemspec | ||
| prettyprint.rb | ||
| pstore.rb | ||
| racc.rb | ||
| rdoc.rb | ||
| readline.gemspec | ||
| readline.rb | ||
| reline.rb | ||
| resolv-replace.gemspec | ||
| resolv-replace.rb | ||
| resolv.gemspec | ||
| resolv.rb | ||
| ruby2_keywords.gemspec | ||
| rubygems.rb | ||
| securerandom.gemspec | ||
| securerandom.rb | ||
| set.rb | ||
| shellwords.gemspec | ||
| shellwords.rb | ||
| singleton.rb | ||
| tempfile.gemspec | ||
| tempfile.rb | ||
| time.gemspec | ||
| time.rb | ||
| timeout.rb | ||
| tmpdir.gemspec | ||
| tmpdir.rb | ||
| tsort.gemspec | ||
| tsort.rb | ||
| un.gemspec | ||
| un.rb | ||
| uri.rb | ||
| weakref.rb | ||
| yaml.rb | ||