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

Add support for Windows on Arm64 (WoA). #559

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chirontt
Copy link

@chirontt chirontt commented Mar 21, 2024

Add support for Windows on Arm64 (WoA)

A new launcher fragment for WoA is added: org.eclipse.equinox.launcher.win32.win32.aarch64

Various features are also updated to include the new fragment.

To build the Equinox launcher binaries for WoA

On a WoA box, run the following commands:

cd features\org.eclipse.equinox.executable.feature\library\win32
ant clean build_eclipse -Dnative=win32.win32.aarch64 -Dos=win32 -Dws=win32 -Darch=aarch64

and the following launcher binaries for WoA are generated in the current directory:

eclipse.exe
eclipse_11900.dll
eclipsec.exe

These WoA binaries need be manually moved over to the equinox.binaries repo, to their correct directories there as followed:

org.eclipse.equinox.executable\bin\win32\win32\aarch64\eclipse.exe
org.eclipse.equinox.executable\bin\win32\win32\aarch64\eclipsec.exe
org.eclipse.equinox.launcher.win32.win32.aarch64\eclipse_11900.dll

@laeubi
Copy link
Member

laeubi commented Mar 21, 2024

@chirontt thanks for bringing this forward, in SWT we used (eclipse) linked folders to make the same sources available for different fragements, would something like this be suitable for the WoA fragemnt as well so we don't need an additional intermediate?

Copy link

github-actions bot commented Mar 21, 2024

Test Results

 3 files   -    25   3 suites   - 25   24s ⏱️ - 11m 16s
14 tests  - 2 156  14 ✅  - 2 110  0 💤  - 46  0 ❌ ±0 
42 runs   - 2 200  42 ✅  - 2 154  0 💤  - 46  0 ❌ ±0 

Results for commit ba48051. ± Comparison against base commit a6ad93f.

This pull request removes 2156 tests.
org.eclipse.core.runtime.tests.FileLocatorTest ‑ testFileLocatorFind
org.eclipse.core.runtime.tests.FileLocatorTest ‑ testFileLocatorGetBundleFile01
org.eclipse.core.runtime.tests.FileLocatorTest ‑ testFileLocatorGetBundleFile02
org.eclipse.equinox.bidi.internal.tests.StructuredTextExtensibilityTest ‑ testOtherContributions
org.eclipse.equinox.bidi.internal.tests.StructuredTextExtensionsTest ‑ testDefaultExtensions
org.eclipse.equinox.bidi.internal.tests.StructuredTextExtensionsTest ‑ testTestExtensions
org.eclipse.equinox.bidi.internal.tests.StructuredTextFullToLeanTest ‑ testFullToLean
org.eclipse.equinox.bidi.internal.tests.StructuredTextMathTest ‑ testRTLarithmetic
org.eclipse.equinox.bidi.internal.tests.StructuredTextMethodsTest ‑ testMethods
org.eclipse.equinox.bidi.internal.tests.StructuredTextProcessorTest ‑ testStructuredTextProcessor
…

♻️ This comment has been updated with latest results.

@chirontt
Copy link
Author

Yes, it may be doable. The benefits will be to remove the new org.eclipse.equinox.security.win32 fragment (as not needed) and keep the existing fragment org.eclipse.equinox.security.win32.x86_64 intact, while the new fragment for WoA org.eclipse.equinox.security.win32.aarch64 will share the existing Java & C source code in the x86_64 folder by making use of the source.. = property in the build.properties file to link to them. Is this what you mean, just to be sure?

@chirontt
Copy link
Author

@laeubi I've done what you suggested, but I encounter some small hiccup: how to include the cpp source files in the final source bundle (like what the x86_64 counterpart is doing)? Here are my various attempts for the build.properties for this org.eclipse.equinox.security.win32.aarch64 module (after removing all unrelated stuff for clarity): the 1st attempt which I consider the simplest and matching closely with the x86_64 counterpart:

source.. = ../org.eclipse.equinox.security.win32.x86_64/src/
src.includes = ../org.eclipse.equinox.security.win32.x86_64/cpp/

but the resulting source bundle won't include the cpp files, as if the src.includes property just ignores the above setting. Then I try these:

source.. = ../org.eclipse.equinox.security.win32.x86_64/src/,\
           ../org.eclipse.equinox.security.win32.x86_64/cpp/

but then the resulting binary (jar) bundle would include those cpp source files too. The next try are these settings:

source.. = ../org.eclipse.equinox.security.win32.x86_64/src/,\
           ../org.eclipse.equinox.security.win32.x86_64/cpp/
bin.excludes = jnicrypt.*

and it almost achieves what I want, but the resulting source bundle won't have a cpp subfolder for those source files (they are left in the root of the bundle), unlike what the source bundle for x86_64 is currently having.

Is there a better way? Otherwise I'd use the bin.excludes way for the PR then.

@HannesWell
Copy link
Member

@chirontt I also looked into this and in order to simplify the org.eclipse.equinox.security.win32 I think it is worth investigating if we can just use JNA in order to perform the only native call. Just like it is done for Mac and Linux. If that works, we can then just use the same fragment for x86_64 and aarch64.

Would you be interested in looking into this in a separate PR? If not, I can do it.

@chirontt
Copy link
Author

@HannesWell Yes, using JNA is a good idea, better than these clunky JNI/C files with re-compile for various platforms. But I have no experience with JNA, so please go ahead and see if JNA is applicable here. Once JNA is proven to work, there are other places in eclipse.platform repo to apply it as well (if possible):

https://github.com/eclipse-platform/eclipse.platform/tree/master/resources/bundles/org.eclipse.core.filesystem/natives
https://github.com/eclipse-platform/eclipse.platform/tree/master/resources/bundles/org.eclipse.core.resources/natives

@HannesWell
Copy link
Member

HannesWell commented Mar 24, 2024

Yes, using JNA is a good idea, better than these clunky JNI/C files with re-compile for various platforms. But I have no experience with JNA, so please go ahead and see if JNA is applicable here.

Absolutely. It looks good, see #564.

Once JNA is proven to work, there are other places in eclipse.platform repo to apply it as well (if possible):

https://github.com/eclipse-platform/eclipse.platform/tree/master/resources/bundles/org.eclipse.core.filesystem/natives
https://github.com/eclipse-platform/eclipse.platform/tree/master/resources/bundles/org.eclipse.core.resources/natives

The filesystem-access implemented in org.eclipse.core.filesystem is very performance sensitive and I'm not sure if migrating it to JNA would be a performance regression for x86_64. As far as I know performance is not the No.1 priority for JNA. So that would have to be measured first.
But as written in eclipse-platform/eclipse.platform.releng.aggregator#577 (comment), the default-impl should already work for WoA (IIRC it is about 10% slower, which is often not dramatic, but nevertheless not acceptable for some users).
Eventually we should try to migrate the native access to the new Foreign Function and Memory Access API finalized in Java-22, but it will probably take some time until we can use that (latest with the next LTS Java-25, which will be usable for Eclipse in about two years). So I'm not sure if it is worth the effort to have that intermediate step.

The situation for org.eclipse.core.resources.win32 is similar. Without it you (probably) only loose refreshing out resources changed outside the workspace through native polling. That is probably not so performance sensitive, but a lot more work to migrate (equinox.security.win32 is even much simplified by using JNA) so it is also the question if it is worth the effort to migrate it to something else than FFM.

Therefore my suggestion is to first focus on the absolute requirements, namely SWT and the Equinox-Launcher.
SWT is on track, but for the Equinox-Launcher (i.e. this change), we also have to adjust the build-pipeline to build the executable.
Unlike SWT the pipeline is not yet migrated to a nice Jenkins pipeline stored in this git-Repo and still exists as manually configured Jenkins builds in https://ci.eclipse.org/releng/view/Launcher/.
I had the intention to migrate that too for a while but will take this as an opportunity to really do it. Once that work is done and with the work done for SWT to support WoA, it should be as simple as the adjustments in the SWT build-pipeline to land this.
Once the basics work and we have even have integration-tests for WoA we can still think about porting o.e.core.filesystem/resources.

@HannesWell
Copy link
Member

@chirontt please rebase this PR and resolve the conflicts. The changes to the equinox.security fragments should not be necessary anymore since #564 is submitted.

@chirontt
Copy link
Author

Branch updated with the latest from master, with PR description updated as well.

A new launcher fragment for WoA is added:
'org.eclipse.equinox.launcher.win32.win32.aarch64'

Various features are also updated to include the new fragment.

To build the Equinox launcher binaries for WoA:

On a WoA box, run the following commands:

cd features\org.eclipse.equinox.executable.feature\library\win32
ant clean build_eclipse -Dnative=win32.win32.aarch64 -Dos=win32 -Dws=win32 -Darch=aarch64

and the following launcher binaries for WoA are generated in the current
directory:

eclipse.exe
eclipse_11900.dll
eclipsec.exe

These WoA binaries need be manually moved over to the 'equinox.binaries'
repo, to their correct directories as followed:

org.eclipse.equinox.executable\bin\win32\win32\aarch64\eclipse.exe
org.eclipse.equinox.executable\bin\win32\win32\aarch64\eclipsec.exe
org.eclipse.equinox.launcher.win32.win32.aarch64\eclipse_11900.dll
@HannesWell
Copy link
Member

@chirontt I have now created #575 in order to track the work on streamlining the Equinox launcher+executable build pipeline. Once that is done, it should be simple to complete this.

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

Successfully merging this pull request may close these issues.

None yet

3 participants