Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deopt creating Regexp #3492

Closed
nirvdrum opened this issue Mar 14, 2024 · 2 comments
Closed

Deopt creating Regexp #3492

nirvdrum opened this issue Mar 14, 2024 · 2 comments
Assignees

Comments

@nirvdrum
Copy link
Collaborator

@rwstauner and I spent some time tracking down a regex issue in a Rails app we’re running on TruffleRuby. It looks like a pathological case in regex caching. We have a Rails action that creates a new Regexp object via Regexp.union. When the Regexp is created TruffleRuby adds the RubyRegexp object to the RubyLanguage#regexpTable cache. Then when we create a TRegex object, we attach it to the cached RubyRegexp instance.

The problem is that RubyLanguage#regexpTable is a WeakValueCache and nothing is retaining a reference to these dynamically created RubyRegexp objects. That’s convenient for memory savings, but because the TRegex object is attached to the RubyRegexp object, when the RubyRegexp is purged from the cache we also lose the TRegex object. Since TRegex lazily builds its call target, this leads to a deopt when populating the local TRegex object cache. Consequently, we’re seeing a ton of deopts for the same Truffle::RegexpOperations.match_in_region split.

It looks like this could manifest in other ways as well. E.g., anywhere we call Truffle::Type.coerce_to_regexp (e.g., String#scan) could dynamically create a Regexp that will not be retained.

A contrived example that illustrates the problem is:

pairs = 1_000.times.map do |i|
  [/(re?)#{i}/, "str#{i}"]
end

100.times do
  pairs.each.with_index do |(re, str), i|
    Truffle::RegexpOperations.match_from(Regexp.union([re, /#{re.source.upcase}/]), str, 0)
  end
end

In reality, the loop we're executing comes from access the same page repeatedly in a load test. The "loop body" in this case is the Rails action.

I've discussed this with @eregon on the GraalVM Slack.

@andrykonchin
Copy link
Member

Fixed in efcfd98

@eregon
Copy link
Member

eregon commented Mar 27, 2024

This fix should be included for the 24.0.1 Release (Apr 16, 2024).
(and of course it's fixed on master and in truffleruby-dev/head)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants