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

Support for ARM/aarch64 #2784

Closed
edjeffreys opened this issue Jul 31, 2023 · 8 comments · Fixed by #2783
Closed

Support for ARM/aarch64 #2784

edjeffreys opened this issue Jul 31, 2023 · 8 comments · Fixed by #2783

Comments

@edjeffreys
Copy link
Contributor

Is there any reason aarch64 isn't already supported?

Running x86_64 on M1 mac with Rosetta has proven problematic with large projects (there seems to be some memory leak), but so far I've had pretty good success locally with #2783, though it's possible I'm missing some known reason this can't be supported.

@rgrunber
Copy link
Contributor

In #2680 we recommended the x86_64 + Rosetta way. If there's an issue with this maybe we could look at adding the build option.

@gayanper are you aware of any recent issue with the x86_64 fragments on Mac M1 (or M2?)

With #2783 I am seeing :

[INFO] Installing product languageServer.product for environment win32/win32/aarch64 to /home/rgrunber/git/eclipse.jdt.ls/org.eclipse.jdt.ls.product/target/products/languageServer.product/win32/win32/aarch64
Installing languageServer.product 1.0.0.
Installation failed.
Cannot complete the install because one or more required items could not be found.
	Software being installed: Java Language Server 1.0.0 (languageServer.product 1.0.0)
	Missing requirement for filter properties ~= $0: languageServer.product.executable.win32.win32.aarch64 1.0.0 requires 'org.eclipse.equinox.p2.iu; org.eclipse.equinox.launcher.win32.win32.aarch64 0.0.0' but it could not be found
	Cannot satisfy dependency:
		From: Java Language Server 1.0.0 (languageServer.product 1.0.0)
		To: org.eclipse.equinox.p2.iu; toolinglanguageServer.product.application [1.0.0,1.0.0]
	Cannot satisfy dependency:
		From: toolinglanguageServer.product.application 1.0.0
		To: org.eclipse.equinox.p2.iu; languageServer.product.executable.win32.win32.aarch64 [1.0.0,1.0.0]
[ERROR] [34abd8ca-b17f-4800-a07c-5e515c272875][extension>org.eclipse.tycho:tycho-maven-plugin:4.0.0] Cannot complete the install because one or more required items could not be found.

It seems like some of the platform-specific bundles we use don't have a version for aarch64 :\

@edjeffreys
Copy link
Contributor Author

edjeffreys commented Jul 31, 2023

It seems like some of the platform-specific bundles we use don't have a version for aarch64 :\

Ah my bad - I assumed win would work the same... seems to only build aarch64 for linux/mac. It does build with the windows config removed.

@rgrunber rgrunber linked a pull request Aug 1, 2023 that will close this issue
@rgrunber
Copy link
Contributor

rgrunber commented Aug 1, 2023

Looks like we're just adding native support for MacOS M1/M2 (ie. no more need to rely on x86_64 + Rosetta) and linux aarch64.

New bundles with the proposed change
44K plugins/org.eclipse.equinox.launcher.cocoa.macosx_1.2.700.v20221108-1024.jar
44K plugins/org.eclipse.equinox.launcher.cocoa.macosx.aarch64_1.2.700.v20221108-1024.jar
88K plugins/org.eclipse.equinox.launcher.gtk.linux.aarch64_1.2.700.v20221108-1024.jar

Seems reasonable.

@jdneo , @fbricon , thoughts ? Seems we could provide support for these platforms natively. I would just want to do a smoke test on MacOs to confirm it works as before.

Do we know of any problems with large projects plaguing only MacOS ?

@gayanper
Copy link
Contributor

gayanper commented Aug 2, 2023

@rgrunber with Eclipse IDE i have not come across any issues running on Macbook M1 with aarch64 builds.

If you provide a nightly with native image for aarch64 i could try out with jdt.core, jdt.ui and jdt.debug workspace i have.

@fbricon
Copy link
Contributor

fbricon commented Aug 2, 2023

I'm fine with that enhancement. Although I'm not aware of any existing issues with aarch64 machines

@rgrunber
Copy link
Contributor

rgrunber commented Aug 8, 2023

@edjeffreys, How did you use the aarch64 bundles ? Were you using it in some custom JDT-LS client ? Was it based on vscode-java (the VS Code client) itself ? I ask because looking at your proposed change, there was just one piece missing. Mainly : https://github.com/eclipse-jdtls/eclipse.jdt.ls/pull/2783/files#diff-18e0fca9cf6cbaf893039113439f4e2df96c36566891af550cef196d33cc1cae . Without that, the final tarball generated under org.eclipse.jdt.ls.product/distro/ wouldn't contain the configuration for the new aarch64 platforms. But maybe you were using the content directly from org.eclipse.jdt.ls.product/target/products (or the repository folder) ?

Furthermore, vscode-java still requires changes to adopt the new platform-specific bundles at https://github.com/redhat-developer/vscode-java/blob/8fa2464e16028b248fd89efc14622aeab251e890/src/javaServerStarter.ts#L166-L172 .

If you were using any of these methods above and didn't make these changes, then it's likely you would still be running off the x86_64 bundles so just want to confirm how you were testing to verify the improvement you were seeing was from the bundles and not some random behaviour of the old approach.

@edjeffreys
Copy link
Contributor Author

Yes, I'm using it in nvim-jdtls which directly uses the contents of org.eclipse.jdt.ls.product/target/repository and manually defines where to look for the configuration. Happy to share my full config if that's helpful, but here's a snippet:

  cmd = {
    "java",
    "-Declipse.application=org.eclipse.jdt.ls.core.id1",
    "-Declipse.product=org.eclipse.jdt.ls.core.product",
    ...
    vim.fn.glob(
      vim.fn.stdpath("data")
        .. "/lazy/eclipse.jdt.ls/"
        .. "org.eclipse.jdt.ls.product/target/repository/plugins/org.eclipse.equinox.launcher_*.jar"
    ),
    "-configuration",
    vim.fn.stdpath("data")
      .. "/lazy/eclipse.jdt.ls/"
      .. "org.eclipse.jdt.ls.product/target/repository/config_mac_arm",
    "-data",
    workspace_dir,
  },

@rgrunber
Copy link
Contributor

rgrunber commented Aug 8, 2023

Ok 👍 that's good enough for me. We can include the change as part of the build and because of the additional client-side configuration necessary, at least clients can explicitly opt in/out depending on their preference.

This does increase the (skipping tests) build from ~50s to 1m but there are ways to keep the build going faster for testing purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants