[ruby-core:114803] [Ruby master Bug#19890] File#realine(chomp: true) slower/more allocations than readline.chomp!
Issue #19890 has been reported by segiddins (Samuel Giddins). ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by tenderlovemaking (Aaron Patterson). This is an implementation detail, but IO#readline is [implemented as a C function](https://github.com/tenderlove/ruby/blob/2334570c8f18d8f2fca3d1d947853e30f7e1...), and currently there is no way to pass keyword args to a C function without allocating a hash. I re-implemeted IO#readline in Ruby and it does eliminate the allocation overhead (I've not tested performance). I sent the patch [here](https://github.com/ruby/ruby/pull/8473). More implementation details, but the challenge with this patch is that `IO#readline` sets `$_` to the last read line, but it does that only in the caller's frame. Take the following program as an example: ```ruby class Foo def call File.open(__FILE__) do |f| read f p __method__ => $_ end end def read f f.readline p __method__ => $_ end end Foo.new.call ``` If you run this, the output is: ``` $ ruby -v test.rb ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22] {:read=>"class Foo\n"} {:call=>nil} ``` So `$_` looks like a global, but it's not really. `$_` is set in the `read` method, but not set in the `call` method. The callee (`IO#readline`) is manipulating values _in the caller's_ environment! The last line value is written via [`rb_lastline_set`](https://github.com/tenderlove/ruby/blob/501aeb3432fd1ed4b4827841b287bf7a44a3...). This function eventually [walks up the call stack, looking for the most recent Ruby call frame](https://github.com/tenderlove/ruby/blob/501aeb3432fd1ed4b4827841b287bf7a44a3...) and sets `$_` in that frame. Since `IO#readline` was implemented in C, "the most recent Ruby call frame" is the user code (in my example `Foo#read`). However, moving `IO#readline` to Ruby means that "the most recent Ruby call frame" is `IO#readline` itself. Of course, setting `$_` in `IO#readline` is of no use to anyone, so in my PR I [added a function that that lets us set special variables in other frames](https://github.com/ruby/ruby/pull/8473/files#diff-2af2e7f2e1c28da5e9d99ad117...). I'm not particularly thrilled with this because it's coupling the implementation of `IO#readline` with its stack depth. That said, it supports `$_` and fixes this issue. I think it would be cool if we could push a special frame for methods like `IO#readline` so that we know where user code is, but I feel a solution like that is beyond the scope of this ticket. ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104648 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by Dan0042 (Daniel DeLorme). tenderlovemaking (Aaron Patterson) wrote in #note-1:
I think it would be cool if we could push a special frame for methods like `IO#readline` so that we know where user code is, but I feel a solution like that is beyond the scope of this ticket.
Something like that would be great. It's not a critical problem but it pops up frequently enough to be annoying. The same issue happens with regexp pseudo-globals like `$1`, and also with `eval`. It's impossible to instrument `#eval` and `Regexp#match` with pass-thru layers (let's say for performance logging) because it then changes the behavior of the method. ex: ```ruby module LogRegexpPerformance def match(...) = log{ super } def match?(...) = log{ super } def =~(...) = log{ super } prepend_features Regexp end /(a)/ =~ "a" #=> 0 $1 #=> nil ... ooops! ``` I think each stack frame should have a "forward_frame" flag which is set when using `super`. So we can walk up the call stack until we find a frame with forward_frame==0, and set `$_` or `$1` in that stack frame (and possibly in all frames in between?) I imagine calling a method with the `foo(...)` forward-everything syntax could also set the "forward_frame" flag. Then we could have something like: ```ruby class Proxy < BasicObject def initialize(obj) @proxied = obj end def method_missing(...) @proxied.send(...) end end proxy = Proxy.new(io) proxy.gets $_ #=> correct value! ``` ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104666 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by Eregon (Benoit Daloze). @Dan0042 There is a significant performance cost to deal with special variables in n callers like this. I think it should remain an internal thing only and not be exposed via that forward flag, as that would slow down such calls significantly and cause extra allocations (at least on some Ruby implementations), and prevent further optimizations. TruffleRuby uses a way to propagate special variables from the caller to the current method, that way things like String#gsub can be implemented in Ruby using Regexp#match internally. That's much more reasonable performance-wise because this only happens explicitly and only for a small numbers of methods. ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104670 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by Dan0042 (Daniel DeLorme). @eregon You should really stop that tendency to premature micro-optimization and making extravagant claims about "significant" performance costs. What I'm suggesting, in the normal case where the flag is not set, involves only a single conditional check more than now. Only for methods that set pseudo-globals. This can not be considered a "significant" performance cost in any rational way. Before worrying about that kind of "significant" performance cost let's worry about having correct behavior ok?
I think it should remain an internal thing only and not be exposed via that forward flag
That forward flag I proposed *is* an internal implementation thing, not something exposed to ruby. ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104684 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by Eregon (Benoit Daloze). Dan0042 (Daniel DeLorme) wrote in #note-4:
@eregon You should really stop that tendency to premature micro-optimization and making extravagant claims about "significant" performance costs. What I'm suggesting, in the normal case where the flag is not set, involves only a single conditional check more than now. Only for methods that set pseudo-globals. This can not be considered a "significant" performance cost in any rational way. Before worrying about that kind of "significant" performance cost let's worry about having correct behavior ok?
I should have clarified I'm considering things based on how it is implemented in TruffleRuby. There such a functionality would cost an extra allocation per call (the special variables objects is passed to the callee and that forces the allocation), which is a significant performance cost. So I think it's pretty clear this would be undesirable to add to all `(...)` methods. If it's only used internally in CRuby and invisible to the user behaviorally, sure, no concern. ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104685 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by Dan0042 (Daniel DeLorme). Eregon (Benoit Daloze) wrote in #note-5:
There such a functionality would cost an extra allocation per call (the special variables objects is passed to the callee and that forces the allocation), which is a significant performance cost.
I have to say there's too many things I don't understand at all in this. The special variables object is passed from the caller to the callee, not the other way? Does that mean a special variables object is allocated for any method call that *might* result in special variables, such as `#send` ? I'm quite confused. Well, regardless of how it is implemented internally, I think that prepending a module (as in my LogRegexpPerformance example) should be transparent to pseudo-globals. I'm sure there's a way to handle that efficiently even in TruffleRuby. ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104690 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by tenderlovemaking (Aaron Patterson). Dan0042 (Daniel DeLorme) wrote in #note-6:
Eregon (Benoit Daloze) wrote in #note-5:
There such a functionality would cost an extra allocation per call (the special variables objects is passed to the callee and that forces the allocation), which is a significant performance cost.
I have to say there's too many things I don't understand at all in this. The special variables object is passed from the caller to the callee, not the other way? Does that mean a special variables object is allocated for any method call that *might* result in special variables, such as `#send` ? I'm quite confused.
I'm not sure how it's implemented in TruffleRuby, but in CRuby the special variable object has reference _location_ allocated on the Ruby stack (in the environment). The svar object itself is lazily allocated only when necessary. So the callee (IO#readline for example) will end up causing the special variable object to be allocated [here](https://github.com/tenderlove/ruby/blob/43c5fde486c7f6183dab9eacc22261e4cea8...).
Well, regardless of how it is implemented internally, I think that prepending a module (as in my LogRegexpPerformance example) should be transparent to pseudo-globals. I'm sure there's a way to handle that efficiently even in TruffleRuby.
Seems like a language level change, and I don't have any opinions on it 😅 ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104698 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by Eregon (Benoit Daloze). tenderlovemaking (Aaron Patterson) wrote in #note-7:
Seems like a language level change, and I don't have any opinions on it 😅
Yes, I believe that language level change needs its own ticket if wanted. This ticket is about fixing the performance of `File#realine(chomp: true)` and I think @tenderlovemaking's PR is fine for that. Of course they might reuse a common underlying mechanism. I think special variables are already one of the most complex things to deal with for a Ruby implementation, and has a non-trivial cost on Ruby performance (e.g. it makes every frame larger on TruffleRuby for the case it needs to pass special variables from caller to callee, so that it can write to them efficiently from the callee). So IMO the less they do/the less special variables the better (currently it's $~ and $_, the rest is not frame-local so not a problem). I'm against extending them in any way, it's already very hacky they get to write to the caller frame (from the POV of e.g. Regexp builtins). ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104709 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by Dan0042 (Daniel DeLorme).
I believe that language level change needs its own ticket if wanted.
I agree. Sorry for semi-hijacking the thread, but this discussion wasn't at the point where I wanted to make a feature request. So if the performance issue was due to keywords arguments, what's your thought on the many other methods that accept `(chomp: true)`, like #gets, #readlines, #each_line ? ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104735 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by tenderlovemaking (Aaron Patterson). Dan0042 (Daniel DeLorme) wrote in #note-9:
I believe that language level change needs its own ticket if wanted.
I agree. Sorry for semi-hijacking the thread, but this discussion wasn't at the point where I wanted to make a feature request.
No problem! 😆
So if the performance issue was due to keywords arguments, what's your thought on the many other methods that accept `(chomp: true)`, like #gets, #readlines, #each_line ?
We should fix those too, but I want to get this patch landed first. ISTM they have a lot in common, so we should be able to basically reuse this code. ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104742 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
Issue #19890 has been updated by segiddins (Samuel Giddins). @tenderlovemaking thanks for looking into this! it completely skipped my mind to check for the hash allocation for the kwargs, which was silly of me (I guess I've been spoiled lately by more things moving to ruby, where the kwargs can be lazily allocated). Of course, a day after that, I ran into https://bugs.ruby-lang.org/issues/16351, so I guess that's karmic justice. ---------------------------------------- Bug #19890: File#realine(chomp: true) slower/more allocations than readline.chomp! https://bugs.ruby-lang.org/issues/19890#change-104743 * Author: segiddins (Samuel Giddins) * Status: Open * Priority: Normal * ruby -v: 3.2.2 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- On ruby 3.2.2 running the following script: ``` ruby #!/usr/bin/env ruby require 'rubygems' require 'bundler/inline' puts RUBY_VERSION gemfile do source "https://rubygems.org" gem "benchmark-ipsa" end Benchmark.ipsa do |x| x.report("f.readline(chomp: true)") do File.open("/usr/share/dict/words") do |f| f.readline(chomp: true) until f.eof? end end x.report("f.readline.chomp!") do File.open("/usr/share/dict/words") do |f| until f.eof? s = f.readline s.chomp! s end end end x.report("f.readline.chomp") do File.open("/usr/share/dict/words") do |f| until f.eof? f.readline.chomp end end end x.compare! end ``` I get the following (surprising) result: ``` 3.2.2 Allocations ------------------------------------- f.readline(chomp: true) 707931/1 alloc/ret 50/1 strings/ret f.readline.chomp! 235979/1 alloc/ret 50/1 strings/ret f.readline.chomp 471955/1 alloc/ret 50/1 strings/ret Warming up -------------------------------------- f.readline(chomp: true) 1.000 i/100ms f.readline.chomp! 2.000 i/100ms f.readline.chomp 2.000 i/100ms Calculating ------------------------------------- f.readline(chomp: true) 16.165 (± 6.2%) i/s - 81.000 f.readline.chomp! 25.246 (± 7.9%) i/s - 126.000 f.readline.chomp 20.997 (± 9.5%) i/s - 106.000 Comparison: f.readline.chomp!: 25.2 i/s f.readline.chomp: 21.0 i/s - 1.20x slower f.readline(chomp: true): 16.2 i/s - 1.56x slower ``` I would expect `File#readline(chomp: true)` to be comparable to `s = f.readline; s.chomp!; s` at a bare minimum, but it is slower and has more allocations even than `readline.chomp` -- https://bugs.ruby-lang.org/
participants (4)
-
Dan0042 (Daniel DeLorme) -
Eregon (Benoit Daloze)
-
segiddins (Samuel Giddins)
-
tenderlovemaking (Aaron Patterson)