-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
[Truffle] No more global lock! #3148
Conversation
I wonder if since we already have a GIL here, it would make sense to keep it as a non-default option? Are there unsynchronised multi-threaded applications that work on MRI but not on JRuby? |
I think that's not an option as it would be unmanageable complexity to have both. |
@@ -61,8 +58,7 @@ public RubyThread getRootThread() { | |||
*/ | |||
@TruffleBoundary | |||
public void enterGlobalLock(RubyThread thread) { | |||
globalLock.lock(); | |||
currentThread = thread; | |||
currentThread.set(thread); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this method be renamed if there's no longer a global lock?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that method will go away in the cleanup phase once this is merged.
It has just been a lot more practical to keep a minimal diff until then.
This line is worrying me https://github.com/pitr-ch/jruby/blob/master/truffle/src/main/ruby/core/shims.rb#L165-165 Not sure how many problems we'll face in practice. |
Yeah I don't think anyone is expecting the parallelism to just work perfectly straight away and there is clearly going to be a lot of work to make it correct. We need parallelism stress tests. Obviously we should fix things as we find them (especially this one as it might impact on performance). |
@pitr-ch That one is very wrong, I was not aware of it. Shims should be to implement only a subset of the semantics, but not totally incorrect semantics like that one. |
@pitr-ch I fixed that one. |
Nice 👍 |
Sorry the merge is broken - can you fix and merge? |
* Still a shim as we might have troubles supporting lock/unlock and might change the implementation.
Probably means you are waiting on fetching from RAM most of the time. Not sure how Monte Carlo pi works but if you can try a smaller working set it might fit into cache and so not scale was well with hyper threads. |
@chrisseaton @nirvdrum @pitr-ch
This is the very simple commit to remove the GIL (cleanup will be done later when merged).
I tried a few trivial parallelism problems, a parallel
richards
benchmark and a few benchmarks from NPB (Ruby version).Multi-threaded programs need to be properly synchronized or they might crash in various ways (as in standard JRuby).