Issue #20652 has been updated by mame (Yusuke Endoh). Assignee set to jeremyevans0 (Jeremy Evans) @jeremyevans0 What do you think? ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109219 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by byroot (Jean Boussier).

If anyone can come up with an approach that fixes #17507 without causing an increase in memory usage

I guess I'm a bit surprised by that because I wouldn't have thought the backref is only accessible by the same fiber, but maybe I'm missing something. But on a more general note, I very very often wish I'd have a way to not have a `MatchData` created by methods that take a Regexp, it's very rare you need the `MatchData` created by `gsub` or `Regexp===`. Most of the time it's fine, but when you are trying to optimize a hotspot, it's really something you'd want to be able to skip. It's in big part why `Regexp#match?` was added and popularized, but is only usable in a subset of cases. Maybe we could add a new Regexp flag to turn off this behavior? e.g. ```ruby case str when /^a/ p $~ # => #<MatchData "a"> when /^b/c p $~ # => nil end ``` ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109225 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by byroot (Jean Boussier).

it could be better if we had a gsub variant that doesn't touch $~.

Right, but the problem is beyond `gsub`, e.g.: ```ruby

...

"abba"[/(bb|[^b]{2})/] => "bb" $~ => #<MatchData "bb" 1:"bb">

So introducing `gsub( backref: false)` or `gsub_no_backref` would solve one case but would leave many others.

Hence why I think the only way to handle this general problem without introducing tons of new methods and lots of code churn would be a new Regexp flag. 

----------------------------------------
Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3
https://bugs.ruby-lang.org/issues/20652#change-109228

* Author: orisano (Nao Yonashiro)
* Status: Open
* Assignee: jeremyevans0 (Jeremy Evans)
----------------------------------------
I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased.
When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased.

The problem was code like this:
```ruby
s = "foo              "
s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" }

When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by ko1 (Koichi Sasada). Eregon (Benoit Daloze) wrote in #note-7:

FWIW, what TruffleRuby does for this is to store `$~` as a frame-local thread-local variable, but thread-local only if more than 1 thread has been seen, otherwise it's stored directly in the frame:

off-topic: what does it happen when creating a thread just after storing `$~`? ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109230 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by austin (Austin Ziegler). byroot (Jean Boussier) wrote in #note-9:

...

I don't really like a Regexp flag for this because a Regexp might be used in different contexts and some usages might want $~ and some might not (which could lead to a bunch of duplication).

I see what you mean, but such flag would only really be worth using in places where saving that allocation is worth it, where right now you usually use a literal anyway, so I don't think duplication would be a concern.

Of course you could expect Rubocop and such to try to force it on everyone because it's faster™, but...

It could be a flag only accessible through `Regexp.new` (`Regexp.new(source, Regexp::OPTIMIZED_MATCHDATA)`) instead of having it with a literal (`/…/z`). The ugliness would make it less likely that Rubocop would try to force it on everyone. ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109234 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by byroot (Jean Boussier). I don't follow, how can it be re-used in your example? : ```ruby def foo if /foo/ =~ "foo" # generate MatchData1 bar() p $~ # #<MatchData "all"> end end def bar m = "all".match(/all/) # reuse MatchData1 p $~ # #<MatchData "foo"> end foo ``` ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109242 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by Eregon (Benoit Daloze). ko1 (Koichi Sasada) wrote in #note-8:

what does it happen when creating a thread just after storing `$~`?

It's simply not set in the new thread, and that seems already the same behavior on CRuby: ``` $ ruby -ve '"a" =~ /a/; p $~; Thread.new { p $~ }.join' ruby 3.3.1 (2024-04-23 revision c56cd86388) [x86_64-linux] #<MatchData "a"> nil $ ruby -ve '"a" =~ /a/; p $~; Thread.new { p $~ }.join' truffleruby 24.1.0-dev-c2e5209c, like ruby 3.2.2, GraalVM CE Native [x86_64-linux] #<MatchData "a"> nil ``` ko1 (Koichi Sasada) wrote in #note-11:

I found an idea that each thread points to unescaped MatchData rather than `$~` and reuse it.

I think that's too incompatible because `$~` is frame-local and thread-local, so we need multiple `$~` per thread, as @byroot showed. ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109244 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by ko1 (Koichi Sasada). Eregon (Benoit Daloze) wrote in #note-14:

ko1 (Koichi Sasada) wrote in #note-11:

...

I found an idea that each thread points to unescaped MatchData rather than `$~` and reuse it.

I think that's too incompatible because `$~` is frame-local and thread-local, so we need multiple `$~` per thread, as @byroot showed.

No. It is not user visible behavior so no incompatiblity. (don't change `$~`) ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109246 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by Dan0042 (Daniel DeLorme). byroot (Jean Boussier) wrote in #note-4:

Maybe we could add a new Regexp flag to turn off this behavior?

My first reaction was "Yes! This is exactly was we need!" but after thinking more it feels un-rubyish. We shouldn't have to write code to micro-tweak the performance like that. Ideally the interpreter/jit should handle micro-optimizations. I'd be happier to see something like: ```ruby def foo(str) str if str =~ /rx/ #in this method we don't use $~ and friends, so the interpreter doesn't have to allocate MatchData #yes in theory there's an incompatibility with eval, but in practice I believe that's a non-issue (I'm open to be shown otherwise) end ``` shyouhei (Shyouhei Urabe) wrote in #note-5:

Nobody thinks `gsub` modifies `$~` behind the scene.

I'm not sure this is what you meant, but I definitely expect ANY regexp operation to modify `$~` (except where documented otherwise, like `Regexp#match?`) BTW this includes String#sub; I sometimes write code like: `name, prefix = str.sub(/^(the) /i), $1` ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109255 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by jeremyevans0 (Jeremy Evans). Dan0042 (Daniel DeLorme) wrote in #note-19:

After reading over https://github.com/ruby/ruby/pull/4734/files it seems there's two parts to it. 1. use a `set_match` pointer to return the match (this fixes the race condition) 2. always allocate a MatchData, never using `rb_backref_get()`

But it seems to me that #2 is only necessary if `set_match` is used. So what about using `rb_backref_get()` when possible? Like ```ruby match = set_match ? Qnil : rb_backref_get(); ```

@jeremyevans0 wdyt?

I'm not sure it is thread-safe. This would modify a shared backref in code paths where `set_match` is `NULL`. I haven't audited the related code, so I'm not sure what code calls `rb_reg_search0` and `rb_reg_match`. Feel free to give it a try and see if passes the test and reduces allocations. ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109258 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by ko1 (Koichi Sasada). ko1 (Koichi Sasada) wrote in #note-16:

No. It is not user visible behavior so no incompatiblity.

Sorry my wrong. Please ignore about it. ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109264 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by Eregon (Benoit Daloze). @byroot It depends in which situation but generally yes it's able to avoid the allocation. If there is no block around, partial escape analysis avoids the allocation of the Ruby MatchData object as long as it's not leaked/stored globally (like `$m = $~`), even if it is accessed in that method. There might be still be an allocation of the internal data structure representing the group offsets, if the Regexp sometimes matches and sometimes not (but tail duplication can fix this in some cases, e.g. if there is not too much code to duplicate). If there is 1/multiple block(s) around the regexp match, then `$~` is stored in the method's frame and not the block's frame and then it's allocated unless there is a compilation covering the method and inlining everything until that block. The case I checked is: ```ruby def foo "a" =~ /(a)/ $1 end loop { foo() } ``` with `cd truffleruby && chruby truffleruby+graalvm-24.0.2 && jt -u ruby graph test.rb` and that shows the only allocation is for a new Ruby String (the return value). ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109275 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by Dan0042 (Daniel DeLorme).

I'd be surprised if it was accepted.

Same here. Although perhaps I should clarify that `$~` would be accessible in eval if also present as a literal in the method. ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-109292 * Author: orisano (Nao Yonashiro) * Status: Open * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/

Issue #20652 has been updated by byroot (Jean Boussier). Sorry, I wasn't clear. The allocation is eluded if the previous MatchData wasn't accessed. If it was, I'm afraid there's nothing we can safely do. ---------------------------------------- Misc #20652: Memory allocation for gsub has increased from Ruby 2.7 to 3.3 https://bugs.ruby-lang.org/issues/20652#change-112090 * Author: orisano (Nao Yonashiro) * Status: Closed * Assignee: jeremyevans0 (Jeremy Evans) ---------------------------------------- I recently upgraded from ruby 2.7.7 to 3.3.1 and noticed that the GC load increased. When I used the allocation profiler to investigate, I found that memory allocation from gsub had increased. The problem was code like this: ```ruby s = "foo " s.gsub(/ (\s+)/) { " #{' ' * Regexp.last_match(1).length}" } ``` When I compared the results of heap-profiler between 2.7.7 and 3.3.1, I found that MatchData was increasing. https://gist.github.com/orisano/98792dee260106e9b6fcb45bbabeb1e6 https://github.com/ruby/ruby/commit/abc0304cb28cb9dcc3476993bc487884c139fd11 I discovered that the cause is this commit, which stopped reusing backref to avoid race conditions. Is there a way to reuse backref while still avoiding race conditions? -- https://bugs.ruby-lang.org/