-
-
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] Add ThreadGroup layout and nodes #4110
Conversation
bjfish
commented
Aug 24, 2016
•
edited
Loading
edited
- Layout needs review, Atomic/Volatile maybe needed
Just wondering, why does this need to be done in Java and not just Ruby? |
@eregon I can't think of any issues of doing it in ruby, I'll try it in ruby |
@eregon I've updated now to use Ruby |
@@ -425,7 +440,8 @@ public DynamicObject allocate( | |||
new AtomicReference<>(null), | |||
new AtomicReference<>(null), | |||
new AtomicBoolean(false), | |||
new AtomicInteger(0)); | |||
new AtomicInteger(0), | |||
currentGroup); |
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.
I tried to set the group in Thread#initialize
method but I don't think initialize
method was being called. Is allocate okay to set this for now?
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 sounds fine.
Well done, this looks much cleaner and smaller now :) |
Let's get this merged once the CI passes! |
end | ||
Truffle.invoke_primitive :thread_set_group, Thread.current, ThreadGroup::Default |
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.
This is probably causing the CI failure.
@chrisseaton So should thread_set_group not have UnsafeGroup.THREADS
since it's a primitive only called internally or is there another solution?
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, no need for UnsafeGroup.THREADS
on thread_set_group
since it doesn't really do anything unsafe (IO, syscalls, creating new threads, etc).
Build is green, merged! Thanks for going through all my comments! |