drb icon indicating copy to clipboard operation
drb copied to clipboard

Avoid use of id2ref for weak mapping on JRuby

Open headius opened this issue 1 year ago • 7 comments

Revisiting my report in https://bugs.ruby-lang.org/issues/15711 and trying to finally get rid of the use of ObjectSpace._id2ref in drb. That report was closed but never actually fixed in DRb.

The implementation of ObjectSpace._id2ref on JRuby (and TruffleRuby) is very expensive, and on JRuby we normally do not have it enabled because it impacts performance of the entire runtime.

This alternate implementation uses the weakref library to build a weak ID map. It could possibly use the new ObjectSpace::WeakMap, but until recently that did not support Fixnum keys and I did not want to break DRb for older Ruby versions.

Unfortunately, due to the lack of "reference queue" support in Ruby's Weakref, this implementation must do a full scan for dead references. This may be possible to improve, and would probably be resolved by using WeakMap.

We recently got another report pry-remote not working on JRuby. pry-remote uses DRb, and DRb still uses ObjectSpace._id2ref to simulate a weak map of objects. We would like to get this fixed and released so users can use pry-remote without enabling full ObjectSpace support.

Notes:

  • This is only enabled for JRuby in this patch. It could be enabled for all Rubies.
  • When using this implementation on CRuby, all tests pass.
  • No attempt is made to ensure the thread-safety of the weak Hash. A mutex could be introduced for this, or it may be ok if we switch to WeakMap.

Edit: Modified to use existing thread-safe WeakMap-based implementation WeakIdConv by default. Without it being default, users of DRb will always end up using the _id2ref version. We should simply eliminate that possibility.

headius avatar Jul 30 '24 14:07 headius