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

Refactor access to VM options and move some VM options to oal.util.Constants #12754

Merged
merged 5 commits into from
Nov 3, 2023

Conversation

uschindler
Copy link
Contributor

This code was previously in RamUsageEstimator and also in PanamaVectorUtilSupport.

In addition this moves detection of Client VM and fast FMA support to Constants class (in preparation for #12737).

…so in PanamaVectorUtilSupport); move detection of Client VM and fast FMA support to Constants
@uschindler uschindler requested a review from rmuir November 3, 2023 16:33
@uschindler uschindler self-assigned this Nov 3, 2023
@uschindler uschindler added this to the 9.9.0 milestone Nov 3, 2023
public static final boolean HAS_FAST_FMA =
(IS_CLIENT_VM == false)
&& Constants.OS_ARCH.equals("amd64")
&& HotspotVMOptions.get("UseFMA").map(Boolean::valueOf).orElse(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update this with the current logic in the other branch? With the newer Neoverse cores that support SVE, they "fixed" their FMA to be fast, so we turn it on there too. It is a good exercise as well since it must check integer parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with both. I just used here the code as is on current main branch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, that makes sense. sorry i've been working off the other branch as a baseline for a while.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will merge it to your branch once merged to main/9.x

@uschindler uschindler changed the title Refactor access to VM options Refactor access to VM options and move some VM options to oal.util.Constants Nov 3, 2023
@uschindler uschindler requested a review from rmuir November 3, 2023 17:46
@uschindler
Copy link
Contributor Author

Hi @rmuir ,
I also fixed the broken security manager and NULL property handling in Constants.java, so we won't crush.
Thats an improvement, but long overdue.

@uschindler uschindler merged commit a35573e into apache:main Nov 3, 2023
4 checks passed
asfgit pushed a commit that referenced this pull request Nov 3, 2023
asfgit pushed a commit that referenced this pull request Nov 5, 2023
asfgit pushed a commit that referenced this pull request Nov 5, 2023
@uschindler
Copy link
Contributor Author

I fixed a regression with OpenJ9 in 5358b72.

@uschindler uschindler deleted the dev/refactorVMOptions branch November 5, 2023 10:27
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

2 participants