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

Add safe unsafe methods to TruffleRuntime #2031

Closed
wants to merge 1 commit into from

Conversation

chrisseaton
Copy link
Contributor

@chrisseaton chrisseaton commented Dec 27, 2019

Are we interested in reducing the need for Unsafe in Truffle guest languages?

Add these safe methods from Unsafe to TruffleRuntime, as a starting point.

This reduces the use of Unsafe in TruffleRuby, GraalJS (PRs available if this is merged), and Sulong (included in this PR.)

Let's cut out the need for direct references to Unsafe and make more Truffle interpreters pure Java code without the need for explicit reflection to access internal fields!

Shopify/truffleruby#1

@eregon
Copy link
Member

eregon commented Dec 28, 2019

Would these methods make more sense as static methods of CompilerDirectives which do somewhat similar things?
These fence methods have the same implementation regardless of the TruffleRuntime, so TruffleRuntime feels suboptimal to me.

@chrisseaton
Copy link
Contributor Author

They could go there if you wanted, but I didn't see them as a compiler issue. Unlike other compiler methods, these aren't no-ops in the interpreter.

@chumer
Copy link
Member

chumer commented Jan 6, 2020

Hm... does not fit either CompilerDirectives nor TruffleRuntime.
I was thinking maybe we should backport parts of the VarHandle API in Truffle instead?

@chrisseaton
Copy link
Contributor Author

We could backport it, but do you mean provide an interface, an adapter for the real API, and a backport implementation? That's a lot of of API faff for what is in this case a couple of static methods.

I'm inclined to be much more pragmatic and just put them here.

@chrisseaton
Copy link
Contributor Author

What do we think in general about this PR?

I was planning to follow it up with access to some more of the unsafe methods, guarded by allowNativeAccess.

@chrisseaton
Copy link
Contributor Author

What are people's thought's on this PR?

@eregon
Copy link
Member

eregon commented Mar 21, 2020

VarHandle has 5 of these fence methods: https://docs.oracle.com/javase/9/docs/api/java/lang/invoke/VarHandle.html#fullFence--

@chumer So how should we name the Truffle VarHandle, TruffleVarHandle?

Those five methods seem safe, and they are already static methods of VarHandle, so I think we need no permission check here for those, and they should remain static methods.

@chrisseaton
Copy link
Contributor Author

A TruffleVarHandle class would be a great solution.

@chumer
Copy link
Member

chumer commented Mar 23, 2020

Yes TruffleVarHandle seems consistent with other naming. Package probably should be com.oracle.truffle.api. Adding static methods as a start seems unproblematic and can be done right away. Should probably be implemented using overlays.

I am not quite sure yet how languages should lookup var handles. In order to check permissions the lookup method should be in TruffleLanguage.Env. That makes the lookup pretty awkward to use. But do we actually need permissions? Maybe we should just limit them to be used on classes loaded using the language loader somehow? Ideas?

@chrisseaton
Copy link
Contributor Author

Should probably be implemented using overlays.

Sorry what's an overlay?

@eregon
Copy link
Member

eregon commented Apr 21, 2021

Similar API for memory barriers/fences were added in 8319322 by @steve-s.
I'll adopt the new API in TruffleRuby.

@eregon eregon closed this Apr 21, 2021
@eregon eregon added this to the 21.2 milestone Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants