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

Rubinius::Synchronized ? #3756

Closed
0x1eef opened this issue Sep 9, 2017 · 10 comments
Closed

Rubinius::Synchronized ? #3756

0x1eef opened this issue Sep 9, 2017 · 10 comments

Comments

@0x1eef
Copy link

0x1eef commented Sep 9, 2017

JRuby has the module JRuby::Synchronized
(https://github.com/jruby/jruby/wiki/Concurrency-in-jruby).

JRuby also provides a non-standard way to convert any object or class into a fully-synchronized data structure: the JRuby::Synchronized module. You can simply require require 'jruby/synchronized' and then include JRuby::Synchronized into any class or obj.extend(JRuby::Synchronized) any object. The result is that all methods on that class or object will be wrapped in simple synchronization; that is, a reentrant lock against the object itself.

It's useful in situations where a lock is a must, it makes taking a thread-unsafe class and turning into something thread-safe trivial:

require "set"
require "jruby/synchronized"
class TSet < Set
  include JRuby::Synchronized
end
GLOBAL = TSet.new []
200.times do
  thrs << Thread.new { GLOBAL.add rand(50000000) }
end
sleep 2
thrs.join

On CRuby supposedly this is considered unnecessary due the VMs lock. Rubinius i think is as parallel as JRuby, but doesn't (appear?) to provide such a convenient facility.

Is there something as convenient in Rubinius? If not can you consider adding ?
Using concurrent-ruby here is not an option. I would like something that makes it as simple as JRuby, the idea is for the concurrent-ruby dependency to not be strictly necessary.

It seems complicated enough on Rubinius to do the same to be necessary, though.
Apologies if there's already a simple API for this.

Thanks.

@brixen
Copy link
Member

brixen commented Sep 10, 2017

@R-obert this is interesting. Are there implementation-specific things that JRuby is using, or could that be put into a gem that just uses Ruby reflection to define wrapper methods?

In Rubinius, any object can be used to synchronize, so you can use Rubinius.synchronize(obj) { } to wrap the existing method. We use the Rubinius.synchronize construct a fair bit in the Ruby core library.

@0x1eef
Copy link
Author

0x1eef commented Sep 10, 2017

I'm not sure. It could be JVM-backed. I looked into this a bit more, and seems similar can be achieved with 'MonitorMixin', which is stdlib but it doesn't look as "nice" or automatic... so doesn't seem practical for wrapping a whole class.

require 'montior'
class TSet < Set
  include ::MonitorMixin
  def add(*a)
    # Note MonitorMixin API doc examples create "add" alias, and call that.
    # Hopefully it's not necessary. 
    synchronize { super(*a) }
  end
end
GLOBAL = TSet.new []

or could that be put into a gem that just uses Ruby reflection to define wrapper methods?

I think this could be solvable with Ruby alone, as a gem, but the issue I'm trying to avoid is adding a gem dependency. I would like either official Ruby solution, or implementation-specific ones, like JRuby has.

so you can use Rubinius.synchronize(obj) { } to wrap the existing method. We use the Rubinius.synchronize construct a fair bit in the Ruby core library.

Seems nice.

It turns out, this feature became unnecessary, since i ended up with something like:

@lock = Monitor.new
@lock.synchronize do
end

Which works on all implementations (i think).

@brixen
Copy link
Member

brixen commented Sep 10, 2017

@R-obert would it be possible to show the code you are working with? I can see using some facility like this as a stop-gap when trying to migrate toward a better concurrent solution, but I'm reluctant to make something official in a way that suggests it's a reasonable long-term solution. This is basically what CRuby does with the GVL, which Rubinius does not use.

I've found that it's generally not very hard to implement a good concurrency solution because Ruby is such a malleable language. I'd rather help you get further along in that direction than institutionalizing a crutch.

Also, I'm working on adding Actors to Rubinius core. These would be multiplexed onto an underlying pthread (machine thread) without any hacky hand-written stack swapping. They may be useful where the problem domain requires a large number of stateful objects that very quickly handle events they receive. If you're curious about that, hop into the Gitter chat channel sometime, or keep an eye on the Rubinius repo. I should get them implemented pretty soon.

@0x1eef
Copy link
Author

0x1eef commented Sep 10, 2017

would it be possible to show the code you are working with?

It's still (very much) WIP:
https://github.com/r-obert/pry-plusplus/blob/d60e10cb6d28a2c8f069a8a1e2e8388544b4d5c3/lib/pry/deprecate.rb

I can see using some facility like this as a stop-gap when trying to migrate toward a better concurrent solution, but I'm reluctant to make something official in a way that suggests it's a reasonable long-term solution.

The situation is that I do not see any other way. I am trying to change internals of that repo to use as much thread-local state as possible, so locks are avoided as much as can be, but in this very specific case, I don't see another way.

I've found that it's generally not very hard to implement a good concurrency solution because Ruby is such a malleable language. I'd rather help you get further along in that direction than institutionalizing a crutch.

Concurrency isn't really my speciality, but here is why global state is currently required in this case (roughly, it's hard to explain but i'll try):

  • Pry wants to offer "Pry-local" deprecations of methods, any method.

  • Deprecations should be inherited by nested sessions, but not outside that, this is what nested means:

pry(main)> binding.pry
pry(main)> # I'm nested now.
pry(main)> exit
pry(main)> # first session
pry(main)> exit
# i'm done, and so are the deprecations.

In this case there's two "Pry" instances.

  • To implement the "temporariness", the deprecated or replaced method reads from a shared data structure, which Pry instances are removed/added to (this is how a deprecation "goes away"). It's global to a single module, but the methods that call it could be accessed across many threads.

There's so much internals to that repo involved here, and background info, that i think it will hard to understand the context entirely. If you see another simpler way I'm all ears though.

@0x1eef
Copy link
Author

0x1eef commented Sep 10, 2017

Also, I'm working on adding Actors to Rubinius core. These would be multiplexed onto an underlying pthread (machine thread) without any hacky hand-written stack swapping. They may be useful where the problem domain requires a large number of stateful objects that very quickly handle events they receive.

It sounds cool. Except, i can't be too implementation-specific. I guess the ultimate solution is for Ruby to have these primitives in a stdlib. Seems long way off. It's being out-sourced to gems as a problem for now (concurrent-ruby etc).

@0x1eef
Copy link
Author

0x1eef commented Sep 10, 2017

One reason you might consider adding something like JRuby::Synchronized is for concurrent-ruby.
There's this pull request: ruby-concurrency/concurrent-ruby#666
Compare the JRuby and Rubinius implementations. Rubinius looks more complicated, and requires more knowledge to implement the equivalent.

According to https://github.com/jruby/jruby/wiki/Concurrency-in-jruby JRuby::Synchronized is also used in other places in concurrent-ruby.

@brixen
Copy link
Member

brixen commented Sep 10, 2017

@R-obert what I'm wondering still is whether JRuby's facility isn't just something like this (and I haven't gone to research it yet).

If the gem dependency is an issue for you, you'll need to have a way to load Ruby engine specific code anyway, since JRuby::Synchronized is JRuby-specific. In that case, you may as well copy the above concurrent-ruby code for Rubinius that I linked to.

I'd be more inclined to add a specific concurrent data structure that would be appropriate to your need than to add something like this. It's truly unfortunate that Ruby has such an absence of useful concurrent data structures in the core or standard library, and you are likely correct that the situation won't get addressed any time soon.

However, I don't see a way around you having some aspect of Ruby engine specific code, and in that case, why not model the problem with a useful concurrent data structure rather than some hidden magic locks around methods. The latter approach is gross and doesn't get you very far, other than to keep layering it on. As the saying goes, if locks aren't working, you're not using enough of them. But that path ends in lots of grief and suffering.

A useful exercise would be this: 1. map clearly what the shared state is; 2. map clearly where concurrent modification of that shared state occurs; 3. research what concurrent data structure would be useful in that case (ie the data structure that would properly serialize the concurrent modification of shared state preserving the data structure invariants that you need); 4. let me know what data structure you need.

@0x1eef
Copy link
Author

0x1eef commented Sep 10, 2017

If the gem dependency is an issue for you, you'll need to have a way to load Ruby engine specific code anyway,

It is, and I think it's unnecessary (i'll explain why).
Engine detection can be simple with rbconfig.rb and i think since recent RUBY_ENGINE.

... since JRuby::Synchronized is JRuby-specific. In that case, you may as well copy the above concurrent-ruby code for Rubinius that I linked to.

But that's a lot of code compared to include JRuby::Synchronized, and not straight forward. The issue is that Rubinius, in a way, forces you to depend on concurrent-ruby, because it requires a non-trivial implementation of JRuby's JRuby::Synchronized equivalent.

If there were Rubinius::Synchronized and JRuby::Synchronized, the need for concurrent-ruby is reduced, and it becomes a lot easier to create the same thread-safe structures as concurrent-ruby, in simple Ruby code.

Imagine:

class TSet < Set
  case 
  when rbx?
    require "rbx/synchronized"
    include Rubinius::Synchronized
  when jruby?
    require "jruby/synchronized"
    include JRuby::Synchronized
  else
    # Give the GVL and MRI a big hug.
  end
end

If you only want one, two, or three "core" or "standard library" classes to be "synchronized" with support for all Ruby engines then it's nice to not have to bring in a gem that includes and require other things (rubygems being one).

I'd be more inclined to add a specific concurrent data structure that would be appropriate to your need than to add something like this. It's truly unfortunate that Ruby has such an absence of useful concurrent data structures in the core or standard library, and you are likely correct that the situation won't get addressed any time soon.

This would be great.

However, I don't see a way around you having some aspect of Ruby engine specific code, and in that case,

Right, this is basically what I want to avoid as much as possible. Engine-specific code with varying levels of complexity, with Rubinius being most complex (in context of this issue).

why not model the problem with a useful concurrent data structure rather than some hidden magic locks around methods.

True. I'd prefer to avoid locks. I'd also like to avoid engine headaches as much as I can.

  1. let me know what data structure you need.

SortedSet seems fine, but will always have the potential to be read from and modified by multiple threads, in context I am in, and remodelling that right now isn't feasible and maybe even not workable.

edit: sleepy coding

@0x1eef 0x1eef closed this as completed Dec 30, 2019
@brixen
Copy link
Member

brixen commented Jan 2, 2020

@rg-3 I might be making all the Ruby data structures synchronized by default via an opt-in runtime flag, so this mechanism wouldn't be needed.

@0x1eef
Copy link
Author

0x1eef commented Jan 2, 2020

Alright; good to know

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

No branches or pull requests

2 participants