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

Rewrite JavaVersion #15372

Merged
merged 3 commits into from Jul 25, 2019
Merged

Rewrite JavaVersion #15372

merged 3 commits into from Jul 25, 2019

Conversation

@blazember
Copy link
Contributor

blazember commented Jul 24, 2019

Java9 introduced java.lang.Runtime.Version with a major() method that returns the major version as an int that the replaced code tried to figure out by parsing the java.version system property. Since with 4.0 Hazelcast requires at least Java8 runtime, it's easy to detect if the runtime is Java8: this class is not accessible via reflection. Otherwise, we can read out the major version via the mentioned method. Since this is part of the official API dealing with unknown Java versions is not an issue anymore, we can safely compare expected versions against the runtime version. For this purpose JavaVersion.FutureJavaVersion inner class and the JavaMajorVersion interface is introduced.

In the scope of this work, ClassloadingMutexProvider got removed. This provider checked if parallel class loading was in use (if Java7+ version) and returned a mutex safe to be used with the class loading mechanism. With Hazelcast 4.0 and Java8 we always take the Java7+ branch, therefore no need for this class.

Implements #15348

Java9 introduced java.lang.Runtime.Version with a major() method that
returns the major version as an int that the replaced code tried to
figure out by parsing the java.version system property. Since with 4.0
Hazelcast requires at least Java8 runtime, it's easy to detect if the
runtime is Java8: this class is not accessible via reflection.
Otherwise we can read out the major version via the mentioned method.
Since this is part of the official API dealing with unknown Java
versions is not an issue anymore, we can safely compare expected
versions against the runtime version. For this purpose
JavaVersion.FutureJavaVersion inner class and the JavaMajorVersion
interface is added.

In scope of this work ClassloadingMutexProvider got removed. This
provider checked if parallel class loading was in use (if Java7+ version)
and returned a mutex safe to be used with the class loading mechanism.
With Hazelcast 4.0 and Java8 we always take the Java7+ branch, therefore
no need for this class.

Implements #15348
@blazember blazember added this to the 4.0 milestone Jul 24, 2019
@blazember blazember requested review from kwart and vbekiaris Jul 24, 2019
@blazember blazember self-assigned this Jul 24, 2019
Copy link
Contributor

kwart left a comment

Just a minor comment related to reflection usage. Otherwise LGTM.

final Class versionClass;

try {
runtimeClass = Class.forName("java.lang.Runtime");

This comment has been minimized.

Copy link
@kwart

kwart Jul 25, 2019

Contributor

We don't need reflection here, I think. The Runtime is part of Java from its beginning. Runtime.class should do the job.

This comment has been minimized.

Copy link
@blazember

blazember Jul 25, 2019

Author Contributor

Oops. Of course :) Fixed.

@kwart
kwart approved these changes Jul 25, 2019
Copy link
Contributor

vbekiaris left a comment

looks good, a minor comment to avoid too much logging by default

versionClass = Class.forName("java.lang.Runtime$Version");
} catch (ClassNotFoundException e) {
// if Runtime is present but Runtime.Version doesn't, it's Java8 for sure
logger.info("Detected runtime version: Java 8");

This comment has been minimized.

Copy link
@vbekiaris

vbekiaris Jul 25, 2019

Contributor

reduce logging level to fine here?

@blazember

This comment has been minimized.

Copy link
Contributor Author

blazember commented Jul 25, 2019

Thanks for the review @kwart @vbekiaris 👍

@blazember blazember merged commit 3570d78 into hazelcast:master Jul 25, 2019
1 check passed
1 check passed
default Test PASSed.
Details
@blazember blazember deleted the blazember:4.0/java_version branch Jul 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.