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

(Apple Silicon) aarch64 OpenJDK detect and install #273

Merged
merged 8 commits into from
Mar 7, 2023

Conversation

r1tsuu
Copy link
Contributor

@r1tsuu r1tsuu commented Mar 2, 2023

On macOS with Apple Silicon processors ARM OpenJDK has ~100% perfomance boost over AMD64.

On the computer with ARM processor:

  1. Added auto detect installation path of ARM type OpenJDK and changed to ARM type detection only.
  2. Changed Amazon Corretta url resolving from amd64 to aarch version

@dscalzi dscalzi self-requested a review March 2, 2023 23:37
@GeekCornerGH
Copy link
Contributor

Linking this pr to the old one if someone came across the old but not the new: #272

Copy link
Contributor

@GeekCornerGH GeekCornerGH left a comment

Choose a reason for hiding this comment

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

LGTM, while I didn't tested the code (don't own any apple product), it seems alright

@dscalzi
Copy link
Owner

dscalzi commented Mar 7, 2023

@r1tsuu I revised the logic a bit, look okay to you? I put in a note since I have to rewrite this as part of the assetguard 2 effort. I don't want to go too crazy on v1 and this seems like it would probably work. I don't have an m1 mac so I can't test.

If you're good with it I'll merge.

@dscalzi dscalzi added the java Issues related to Java label Mar 7, 2023
@dscalzi
Copy link
Owner

dscalzi commented Mar 7, 2023

A little preview of some of the improvements we'll get from the rewritten code. https://github.com/dscalzi/helios-core/blob/40c568a6a9683b3f11815d9202375a0fa32f5623/lib/java/JavaGuard.ts#L315
Much better parsing of -XshowSettings with typings around the properties.

app/assets/js/assetguard.js Outdated Show resolved Hide resolved
app/assets/js/assetguard.js Outdated Show resolved Hide resolved
app/assets/js/assetguard.js Outdated Show resolved Hide resolved
@r1tsuu r1tsuu requested a review from dscalzi March 7, 2023 09:11
@dscalzi dscalzi merged commit fb586cc into dscalzi:master Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
java Issues related to Java
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants