Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Draft
jchyb wants to merge 7 commits into scala:main
base: main
Choose a base branch
Loading
from dotty-staging:lazyval-varhandle

Conversation

Copy link
Contributor

@jchyb jchyb commented Sep 29, 2025
edited
Loading

Just a few small tweaks left (should be done by tomorrow, I mostly want to confirm the closed community build works without running locally):

  • extract MethodHandles.lookup into local variable in the static constructor
  • maybe inline objCAS2 (should give us more freedom, allow us to backport into 3.3 in the worst case scenario (hopefully not))

plokhotnyuk, zielinsky, He-Pin, tgodzik, tanishiking, Gedochao, noti0na1, SethTisue, sake92, sjrd, and xuwei-k reacted with rocket emoji
@jchyb jchyb force-pushed the lazyval-varhandle branch 5 times, most recently from a82a297 to 5aafc89 Compare October 2, 2025 14:32
Copy link
Member

sjrd commented Oct 3, 2025
edited
Loading

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.)

Copy link
Contributor Author

jchyb commented Oct 3, 2025

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 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.)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /