-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Replace sun.misc.Unsafe used in LazyVals with VarHandles #24109
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
Conversation
a82a297
to
5aafc89
Compare
extract MethodHandles.lookup into local variable in the static constructor
I actually wonder: is that worth it? It seems to bring additional complexity to the transformation. Sure, we need one call for every lazy val
in the class, but does it matter? It's in the static initializer anyway, which is executed O(1) times.
maybe inline objCAS2
IMO that is really worth it, but for a different reason: it removes the last reference to the LazyVals$
module class in the generated code. That's good because we are then reasonably sure that we won't hit load-class-time warnings about the Unsafe
val in that module class. AFAICT, generating the inlined code would not be any harder than generating the call to objCAS2
. It's one method call.
(We can drop the if (debug)
thing at this point anyway.)
extract MethodHandles.lookup into local variable in the static constructor
I actually wonder: is that worth it? It seems to bring additional complexity to the transformation. Sure, we need one call for every
lazy val
in the class, but does it matter? It's in the static initializer anyway, which is executed O(1) times.
It's an obvious improvement so I wanted to take it. Unfortunately, even without this we still had to introduce complexity in the MoveStatics phase (directly linking to the LazyVals phase), so we are not doing that much more (1st commit vs 2nd commit). I'll try running some benchmark tests to see if it's worth it.
maybe inline objCAS2
IMO that is really worth it, but for a different reason: it removes the last reference to the
LazyVals$
module class in the generated code. That's good because we are then reasonably sure that we won't hit load-class-time warnings about theUnsafe
val in that module class. AFAICT, generating the inlined code would not be any harder than generating the call toobjCAS2
. It's one method call.(We can drop the
if (debug)
thing at this point anyway.)
Right, I hadn't even thought about that (we still reference the nested classes but now I realize that's not an issue). Weirdly, even with the objCAS2 call, the warnings disappeared for me.
5aafc89
to
74429df
Compare
74429df
to
3c7b33c
Compare
3c7b33c
to
b969d42
Compare
Uh oh!
There was an error while loading. Please reload this page.
Just a few small tweaks left (should be done by tomorrow, I mostly want to confirm the closed community build works without running locally):