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

Fix static lazy field holder for GraalVM #16800

Merged
merged 5 commits into from
Feb 6, 2023

Conversation

szymon-rd
Copy link
Contributor

@szymon-rd szymon-rd commented Feb 1, 2023

The new lazy vals implementation was not working in graalvm's native image. The generated holder for the lazy value was static post-compilation. It caused the generated code to perform CAS operate on bad addresses and throw Bus Error.
Issue on oracle/graalvm: oracle/graal#5863
This PR removes the static modifier from the lazy val holder and makes it accessible via MODULE$ instead. It fixes the CAS on GraalVM and does not have a performance impact, at least judging by the benchmark I've added to this PR.

@szymon-rd
Copy link
Contributor Author

szymon-rd commented Feb 1, 2023

@Kordyjan I removed getStaticFieldOffset that was experimental in pre-3.3.0-RCx. It was made stable in 3.3.0-RC1. As tasty is experimental, I understand that it's ok, right?

@sjrd
Copy link
Member

sjrd commented Feb 1, 2023

@Kordyjan I removed getStaticFieldOffset that was experimental in pre-3.3.0-RCx. It was made stable in 3.3.0-RC1. As tasty is experimental, I understand that it's ok, right?

Yes, that's OK.

@Kordyjan Kordyjan added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Feb 2, 2023
@Kordyjan Kordyjan added this to the 3.3.0 backports milestone Feb 2, 2023
@szymon-rd szymon-rd merged commit 673182e into scala:main Feb 6, 2023
@Kordyjan Kordyjan added backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Feb 13, 2023
Kordyjan added a commit that referenced this pull request Feb 17, 2023
@Kordyjan Kordyjan added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels Feb 17, 2023
@Kordyjan Kordyjan modified the milestones: 3.3.0 backports, 3.3.1, 3.3.0 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants