[ruby-core:91882] [Ruby trunk Bug#15711] Remove use of _id2ref from DRb

[ruby-core:91882] [Ruby trunk Bug#15711] Remove use of _id2ref from DRb

[hsbt]

Issue #15711 has been updated by hsbt (Hiroshi SHIBATA).

Assignee set to seki (Masatoshi Seki)
Status changed from Open to Assigned

@seki

Can you handle this?


----------------------------------------
Bug #15711: Remove use of _id2ref from DRb
https://bugs.ruby-lang.org/issues/15711#change-77203

* Author: headius (Charles Nutter)
* Status: Assigned
* Priority: Normal
* Assignee: seki (Masatoshi Seki)
* Target version: 
* ruby -v: 
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
This issue relates to https://bugs.ruby-lang.org/issues/15408

DRb uses `_idref` internally to implement a weak map, and this issue seeks to replace that code with an implementation that does not use `_id2ref`.

We will be deprecating `ObjectSpace._id2ref` in the near future since it fails to work like people expect (when implemented as a pointer address) or adds memory and invocation overhead to `object_id`.

An initial patch for this is provided by JRuby, which implements `object_id` using a monotonically-increasing value, and only allows `_id2ref` use with a command line flag.

https://github.com/ruby/ruby/compare/trunk...jruby:jruby-ruby_2_6_0#diff-e979bf2f831d9826629559b8628809e9

This implementation uses the stdlib `weakref` to implement a simple weak map, and it would be suitable as an implementation for now. However there's some inefficiency here because it has to periodically "clean" the hash of vacated references by scanning all entries.

There are two more efficient implementations that require additional work:

Alternate 1: Use `ObjectSpace::WeakMap`, which is an opaque VM-supported implementation of a weak Hash. Unfortunately I don't think `WeakMap` has ever been blessed as a public API, and since we're rapidly moving standard libraries to gems, it would not be appropriate to use an internal API. So, we can either make WeakMap an official part of the public standard API, or do alternate 2.

Alternate 2: Add weak reference queues to the weakref API, so users can implement their own efficient weak maps. Some of this has been discussed (at great length) in https://bugs.ruby-lang.org/issues/4168, and the JRuby team has supported the [weaklink](https://github.com/headius/weakling) gem for many years (which provides a WeakRef+RefQueue implementation for JRuby).

The original patch works well for small numbers of remoted objects.



-- 
https://bugs.ruby-lang.org/

[ruby-core:91898] [Ruby trunk Bug#15711] Remove use of _id2ref from DRb

[eregontp]

Issue #15711 has been updated by Eregon (Benoit Daloze).


@headius Could you attach a diff for the weakref-based approach?
It seems the GitHub link doesn't work well (it shows the list of commits for me).

----------------------------------------
Bug #15711: Remove use of _id2ref from DRb
https://bugs.ruby-lang.org/issues/15711#change-77224

* Author: headius (Charles Nutter)
* Status: Assigned
* Priority: Normal
* Assignee: seki (Masatoshi Seki)
* Target version: 
* ruby -v: 
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
This issue relates to https://bugs.ruby-lang.org/issues/15408

DRb uses `_idref` internally to implement a weak map, and this issue seeks to replace that code with an implementation that does not use `_id2ref`.

We will be deprecating `ObjectSpace._id2ref` in the near future since it fails to work like people expect (when implemented as a pointer address) or adds memory and invocation overhead to `object_id`.

An initial patch for this is provided by JRuby, which implements `object_id` using a monotonically-increasing value, and only allows `_id2ref` use with a command line flag.

https://github.com/ruby/ruby/compare/trunk...jruby:jruby-ruby_2_6_0#diff-e979bf2f831d9826629559b8628809e9

This implementation uses the stdlib `weakref` to implement a simple weak map, and it would be suitable as an implementation for now. However there's some inefficiency here because it has to periodically "clean" the hash of vacated references by scanning all entries.

There are two more efficient implementations that require additional work:

Alternate 1: Use `ObjectSpace::WeakMap`, which is an opaque VM-supported implementation of a weak Hash. Unfortunately I don't think `WeakMap` has ever been blessed as a public API, and since we're rapidly moving standard libraries to gems, it would not be appropriate to use an internal API. So, we can either make WeakMap an official part of the public standard API, or do alternate 2.

Alternate 2: Add weak reference queues to the weakref API, so users can implement their own efficient weak maps. Some of this has been discussed (at great length) in https://bugs.ruby-lang.org/issues/4168, and the JRuby team has supported the [weaklink](https://github.com/headius/weakling) gem for many years (which provides a WeakRef+RefQueue implementation for JRuby).

The original patch works well for small numbers of remoted objects.



-- 
https://bugs.ruby-lang.org/

[ruby-core:91999] [Ruby trunk Bug#15711] Remove use of _id2ref from DRb

[headius]

Issue #15711 has been updated by headius (Charles Nutter).

Assignee deleted (seki (Masatoshi Seki))

@eregon Are you talking about the PR? That has the version of code that should be merged into CRuby. The link in the description was just to show the diff in place in our CRuby fork, but it didn't base that diff against a moment-in-time snapshot of CRuby.

https://github.com/ruby/ruby/pull/2102/files

----------------------------------------
Bug #15711: Remove use of _id2ref from DRb
https://bugs.ruby-lang.org/issues/15711#change-77328

* Author: headius (Charles Nutter)
* Status: Assigned
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
This issue relates to https://bugs.ruby-lang.org/issues/15408

DRb uses `_idref` internally to implement a weak map, and this issue seeks to replace that code with an implementation that does not use `_id2ref`.

We will be deprecating `ObjectSpace._id2ref` in the near future since it fails to work like people expect (when implemented as a pointer address) or adds memory and invocation overhead to `object_id`.

An initial patch for this is provided by JRuby, which implements `object_id` using a monotonically-increasing value, and only allows `_id2ref` use with a command line flag.

https://github.com/ruby/ruby/compare/trunk...jruby:jruby-ruby_2_6_0#diff-e979bf2f831d9826629559b8628809e9

This implementation uses the stdlib `weakref` to implement a simple weak map, and it would be suitable as an implementation for now. However there's some inefficiency here because it has to periodically "clean" the hash of vacated references by scanning all entries.

There are two more efficient implementations that require additional work:

Alternate 1: Use `ObjectSpace::WeakMap`, which is an opaque VM-supported implementation of a weak Hash. Unfortunately I don't think `WeakMap` has ever been blessed as a public API, and since we're rapidly moving standard libraries to gems, it would not be appropriate to use an internal API. So, we can either make WeakMap an official part of the public standard API, or do alternate 2.

Alternate 2: Add weak reference queues to the weakref API, so users can implement their own efficient weak maps. Some of this has been discussed (at great length) in https://bugs.ruby-lang.org/issues/4168, and the JRuby team has supported the [weaklink](https://github.com/headius/weakling) gem for many years (which provides a WeakRef+RefQueue implementation for JRuby).

The original patch works well for small numbers of remoted objects.



-- 
https://bugs.ruby-lang.org/

[ruby-core:92000] [Ruby trunk Bug#15711] Remove use of _id2ref from DRb

[headius]

Issue #15711 has been updated by headius (Charles Nutter).


I have not written up a patch based on a reference queue, but it would basically just use the `IdWeakRef` class and the `_cleanup` method from the `IdHash` example collection in my "weakling" library:

https://github.com/headius/weakling/blob/master/lib/weakling/collections.rb

This implementation polls the reference queue on each "clean", which will be a no-op when there are no vacated references in the queue. Compare to the full Hash scan in the supported weakref version. In order to use the more efficient impl, the weakref library would need to support reference queues.

----------------------------------------
Bug #15711: Remove use of _id2ref from DRb
https://bugs.ruby-lang.org/issues/15711#change-77329

* Author: headius (Charles Nutter)
* Status: Assigned
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
This issue relates to https://bugs.ruby-lang.org/issues/15408

DRb uses `_idref` internally to implement a weak map, and this issue seeks to replace that code with an implementation that does not use `_id2ref`.

We will be deprecating `ObjectSpace._id2ref` in the near future since it fails to work like people expect (when implemented as a pointer address) or adds memory and invocation overhead to `object_id`.

An initial patch for this is provided by JRuby, which implements `object_id` using a monotonically-increasing value, and only allows `_id2ref` use with a command line flag.

https://github.com/ruby/ruby/compare/trunk...jruby:jruby-ruby_2_6_0#diff-e979bf2f831d9826629559b8628809e9

This implementation uses the stdlib `weakref` to implement a simple weak map, and it would be suitable as an implementation for now. However there's some inefficiency here because it has to periodically "clean" the hash of vacated references by scanning all entries.

There are two more efficient implementations that require additional work:

Alternate 1: Use `ObjectSpace::WeakMap`, which is an opaque VM-supported implementation of a weak Hash. Unfortunately I don't think `WeakMap` has ever been blessed as a public API, and since we're rapidly moving standard libraries to gems, it would not be appropriate to use an internal API. So, we can either make WeakMap an official part of the public standard API, or do alternate 2.

Alternate 2: Add weak reference queues to the weakref API, so users can implement their own efficient weak maps. Some of this has been discussed (at great length) in https://bugs.ruby-lang.org/issues/4168, and the JRuby team has supported the [weaklink](https://github.com/headius/weakling) gem for many years (which provides a WeakRef+RefQueue implementation for JRuby).

The original patch works well for small numbers of remoted objects.



-- 
https://bugs.ruby-lang.org/

[ruby-core:92001] [Ruby trunk Bug#15711] Remove use of _id2ref from DRb

[headius]

Issue #15711 has been updated by headius (Charles Nutter).


PR for the reference queue version is here: https://github.com/ruby/ruby/pull/2104

Won't pass CI because of the missing `weakling` dependency, but should be easy to test locally.

----------------------------------------
Bug #15711: Remove use of _id2ref from DRb
https://bugs.ruby-lang.org/issues/15711#change-77330

* Author: headius (Charles Nutter)
* Status: Assigned
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
This issue relates to https://bugs.ruby-lang.org/issues/15408

DRb uses `_idref` internally to implement a weak map, and this issue seeks to replace that code with an implementation that does not use `_id2ref`.

We will be deprecating `ObjectSpace._id2ref` in the near future since it fails to work like people expect (when implemented as a pointer address) or adds memory and invocation overhead to `object_id`.

An initial patch for this is provided by JRuby, which implements `object_id` using a monotonically-increasing value, and only allows `_id2ref` use with a command line flag.

https://github.com/ruby/ruby/compare/trunk...jruby:jruby-ruby_2_6_0#diff-e979bf2f831d9826629559b8628809e9

This implementation uses the stdlib `weakref` to implement a simple weak map, and it would be suitable as an implementation for now. However there's some inefficiency here because it has to periodically "clean" the hash of vacated references by scanning all entries.

There are two more efficient implementations that require additional work:

Alternate 1: Use `ObjectSpace::WeakMap`, which is an opaque VM-supported implementation of a weak Hash. Unfortunately I don't think `WeakMap` has ever been blessed as a public API, and since we're rapidly moving standard libraries to gems, it would not be appropriate to use an internal API. So, we can either make WeakMap an official part of the public standard API, or do alternate 2.

Alternate 2: Add weak reference queues to the weakref API, so users can implement their own efficient weak maps. Some of this has been discussed (at great length) in https://bugs.ruby-lang.org/issues/4168, and the JRuby team has supported the [weaklink](https://github.com/headius/weakling) gem for many years (which provides a WeakRef+RefQueue implementation for JRuby).

The original patch works well for small numbers of remoted objects.



-- 
https://bugs.ruby-lang.org/

[ruby-core:92011] [Ruby trunk Bug#15711] Remove use of _id2ref from DRb

[eregontp]

Issue #15711 has been updated by Eregon (Benoit Daloze).


headius (Charles Nutter) wrote:
> @eregon Are you talking about the PR? That has the version of code that should be merged into CRuby. The link in the description was just to show the diff in place in our CRuby fork, but it didn't base that diff against a moment-in-time snapshot of CRuby.

Yes, that link shows the list of commits rather than the diff for some reason.
 
> https://github.com/ruby/ruby/pull/2102/files

Thanks, that's much easier to review.

@seki Could you review that PR?


----------------------------------------
Bug #15711: Remove use of _id2ref from DRb
https://bugs.ruby-lang.org/issues/15711#change-77340

* Author: headius (Charles Nutter)
* Status: Assigned
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
This issue relates to https://bugs.ruby-lang.org/issues/15408

DRb uses `_idref` internally to implement a weak map, and this issue seeks to replace that code with an implementation that does not use `_id2ref`.

We will be deprecating `ObjectSpace._id2ref` in the near future since it fails to work like people expect (when implemented as a pointer address) or adds memory and invocation overhead to `object_id`.

An initial patch for this is provided by JRuby, which implements `object_id` using a monotonically-increasing value, and only allows `_id2ref` use with a command line flag.

https://github.com/ruby/ruby/compare/trunk...jruby:jruby-ruby_2_6_0#diff-e979bf2f831d9826629559b8628809e9

This implementation uses the stdlib `weakref` to implement a simple weak map, and it would be suitable as an implementation for now. However there's some inefficiency here because it has to periodically "clean" the hash of vacated references by scanning all entries.

There are two more efficient implementations that require additional work:

Alternate 1: Use `ObjectSpace::WeakMap`, which is an opaque VM-supported implementation of a weak Hash. Unfortunately I don't think `WeakMap` has ever been blessed as a public API, and since we're rapidly moving standard libraries to gems, it would not be appropriate to use an internal API. So, we can either make WeakMap an official part of the public standard API, or do alternate 2.

Alternate 2: Add weak reference queues to the weakref API, so users can implement their own efficient weak maps. Some of this has been discussed (at great length) in https://bugs.ruby-lang.org/issues/4168, and the JRuby team has supported the [weaklink](https://github.com/headius/weakling) gem for many years (which provides a WeakRef+RefQueue implementation for JRuby).

The original patch works well for small numbers of remoted objects.



-- 
https://bugs.ruby-lang.org/

[ruby-core:92025] [Ruby trunk Bug#15711] Remove use of _id2ref from DRb

[hsbt]

Issue #15711 has been updated by hsbt (Hiroshi SHIBATA).

Assignee set to seki (Masatoshi Seki)

----------------------------------------
Bug #15711: Remove use of _id2ref from DRb
https://bugs.ruby-lang.org/issues/15711#change-77350

* Author: headius (Charles Nutter)
* Status: Assigned
* Priority: Normal
* Assignee: seki (Masatoshi Seki)
* Target version: 
* ruby -v: 
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
This issue relates to https://bugs.ruby-lang.org/issues/15408

DRb uses `_idref` internally to implement a weak map, and this issue seeks to replace that code with an implementation that does not use `_id2ref`.

We will be deprecating `ObjectSpace._id2ref` in the near future since it fails to work like people expect (when implemented as a pointer address) or adds memory and invocation overhead to `object_id`.

An initial patch for this is provided by JRuby, which implements `object_id` using a monotonically-increasing value, and only allows `_id2ref` use with a command line flag.

https://github.com/ruby/ruby/compare/trunk...jruby:jruby-ruby_2_6_0#diff-e979bf2f831d9826629559b8628809e9

This implementation uses the stdlib `weakref` to implement a simple weak map, and it would be suitable as an implementation for now. However there's some inefficiency here because it has to periodically "clean" the hash of vacated references by scanning all entries.

There are two more efficient implementations that require additional work:

Alternate 1: Use `ObjectSpace::WeakMap`, which is an opaque VM-supported implementation of a weak Hash. Unfortunately I don't think `WeakMap` has ever been blessed as a public API, and since we're rapidly moving standard libraries to gems, it would not be appropriate to use an internal API. So, we can either make WeakMap an official part of the public standard API, or do alternate 2.

Alternate 2: Add weak reference queues to the weakref API, so users can implement their own efficient weak maps. Some of this has been discussed (at great length) in https://bugs.ruby-lang.org/issues/4168, and the JRuby team has supported the [weaklink](https://github.com/headius/weakling) gem for many years (which provides a WeakRef+RefQueue implementation for JRuby).

The original patch works well for small numbers of remoted objects.



-- 
https://bugs.ruby-lang.org/

[ruby-core:92054] [Ruby trunk Bug#15711] Remove use of _id2ref from DRb

[headius]

Issue #15711 has been updated by headius (Charles Nutter).


I'm happy to chat/review this code and the overall _id2ref change at RubyKaigi in a few weeks, btw.

----------------------------------------
Bug #15711: Remove use of _id2ref from DRb
https://bugs.ruby-lang.org/issues/15711#change-77376

* Author: headius (Charles Nutter)
* Status: Assigned
* Priority: Normal
* Assignee: seki (Masatoshi Seki)
* Target version: 
* ruby -v: 
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
This issue relates to https://bugs.ruby-lang.org/issues/15408

DRb uses `_idref` internally to implement a weak map, and this issue seeks to replace that code with an implementation that does not use `_id2ref`.

We will be deprecating `ObjectSpace._id2ref` in the near future since it fails to work like people expect (when implemented as a pointer address) or adds memory and invocation overhead to `object_id`.

An initial patch for this is provided by JRuby, which implements `object_id` using a monotonically-increasing value, and only allows `_id2ref` use with a command line flag.

https://github.com/ruby/ruby/compare/trunk...jruby:jruby-ruby_2_6_0#diff-e979bf2f831d9826629559b8628809e9

This implementation uses the stdlib `weakref` to implement a simple weak map, and it would be suitable as an implementation for now. However there's some inefficiency here because it has to periodically "clean" the hash of vacated references by scanning all entries.

There are two more efficient implementations that require additional work:

Alternate 1: Use `ObjectSpace::WeakMap`, which is an opaque VM-supported implementation of a weak Hash. Unfortunately I don't think `WeakMap` has ever been blessed as a public API, and since we're rapidly moving standard libraries to gems, it would not be appropriate to use an internal API. So, we can either make WeakMap an official part of the public standard API, or do alternate 2.

Alternate 2: Add weak reference queues to the weakref API, so users can implement their own efficient weak maps. Some of this has been discussed (at great length) in https://bugs.ruby-lang.org/issues/4168, and the JRuby team has supported the [weaklink](https://github.com/headius/weakling) gem for many years (which provides a WeakRef+RefQueue implementation for JRuby).

The original patch works well for small numbers of remoted objects.



-- 
https://bugs.ruby-lang.org/