truffleruby icon indicating copy to clipboard operation
truffleruby copied to clipboard

Optimize Thread.detect_recursion

Open eregon opened this issue 4 years ago • 5 comments

We should optimize Thread.detect_recursion, e.g., by moving some of it to Java, using EconomicMap instead of {}.compare_by_identity which forces computing object_id.

That should also do the optimization of not computing the identity hash until needed if few objects are tracked.

We should also avoid the block if possible as that creates more indirections.

From https://github.com/oracle/truffleruby/issues/1992#issuecomment-626207717

eregon avatar May 15 '20 16:05 eregon

@LillianZ has some benchmarks and works in progress towards solving this.

chrisseaton avatar Sep 14 '20 19:09 chrisseaton

We've got some significant work on this now - will merge as much of it as we can soon but please don't refactor anything to do with detect_recursion until then thanks.

chrisseaton avatar Oct 04 '20 19:10 chrisseaton

Re: https://github.com/oracle/truffleruby/pull/2189#issuecomment-746445414 I guess that idea is dead in the water, but just to be very sure: there is no way to leverage some kind of thread-local storage in a way that would be faster than optimizing our own maps, right?

norswap avatar Dec 16 '20 15:12 norswap

Follow-ups for this after https://github.com/oracle/truffleruby/pull/2189:

  • Avoiding an extra lookup in DeleteLastNode: https://github.com/oracle/truffleruby/pull/2189#discussion_r544231540 (done in #2011 too)
  • Make Truffle::ThreadOperations.detect_recursion a primitive so it's always split, in a cheaper way than using Truffle splitting of CallTargets: https://github.com/oracle/truffleruby/pull/2189#issuecomment-746450024

eregon avatar Dec 17 '20 11:12 eregon

Related optimization in CRuby, to avoid using the recursion logic when we know we'd call Kernel#hash: https://github.com/ruby/ruby/pull/3987

eregon avatar Dec 24 '20 10:12 eregon