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

Linux aarch64 support for EIP196 constantine #203

Merged
merged 30 commits into from
Aug 21, 2024

Conversation

NickSneo
Copy link
Contributor

Fixes #198

Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
@NickSneo NickSneo requested a review from garyschulte August 15, 2024 00:57
@NickSneo
Copy link
Contributor Author

@garyschulte Please review.

@NickSneo NickSneo changed the title Linux aarch64 support Linux aarch64 support for EIP196 constantine Aug 15, 2024
Copy link
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

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

LGTM, minor nit about the the openjdk install source

native-build.sh Outdated Show resolved Hide resolved
build.sh Show resolved Hide resolved
task macArmLibCopy(type: Copy) {
from "build/darwin-aarch64/lib/libconstantine.dylib"
from "build/darwin-aarch64/lib/libconstantineeip196.jnilib"
into 'build/resources/main/lib/darwin-aarch64'
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, testling locally I am getting
Exception in thread "main" java.lang.UnsatisfiedLinkError: no constantineeip196 in java.library.path: ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which OS are you testing this in?
I assume first you ran the ./build.sh and then the test files?

I tested in Mac and Linux x86_64, working fine for me

Copy link
Contributor

@garyschulte garyschulte Aug 20, 2024

Choose a reason for hiding this comment

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

I am testing by integrating into besu on Darwin aarch64, when I build I am seeing the libs getting embedded in the jar inlib/{arch} rather than {arch}. The 0.9.4 build shows the same for the x86_64 libs.

Perhaps there is a classpath difference / expectation in besu-native/constantine that allows it to find the libs, but that same native classpath isn't in besu (since besu is using the jar dep instead of a filesystem build artifact). Looking into it now

Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
@NickSneo
Copy link
Contributor Author

NickSneo commented Aug 20, 2024

@garyschulte I haven't changed anything for linux x86_64 but seems the pipeline is failing due to -
/tmp/choosenim-0.8.5_linux_amd64: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found (required by /tmp/choosenim-0.8.5_linux_amd64)
Do you have any idea about this?

Edit: Updated Ubuntu to 22.04 for linux x86_64 pipeline, fixed the problem with GLIBC_2.34

Signed-off-by: Nischal Sharma <[email protected]>
@NickSneo
Copy link
Contributor Author

@garyschulte Pipeline for Mac Arm is failing due to -

 ############################
  ####### build gnark #######
  ############################
./build.sh: line 312: go: command not found

@garyschulte
Copy link
Contributor

@garyschulte Pipeline for Mac Arm is failing due to -

 ############################
  ####### build gnark #######
  ############################
./build.sh: line 312: go: command not found

We can add go, it builds fine locally. It is just the packaging that is presenting a problem for me at the moment

Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
@NickSneo
Copy link
Contributor Author

@garyschulte Pipelines are passing now.

@garyschulte
Copy link
Contributor

garyschulte commented Aug 21, 2024

Converting to JNA does seem to include a little overhead for the first call to the library, but subsequent performance is essentially identical:

Signed-off-by: Nischal Sharma <[email protected]>
Signed-off-by: Nischal Sharma <[email protected]>
build.sh Outdated Show resolved Hide resolved
@garyschulte garyschulte enabled auto-merge (squash) August 21, 2024 18:18
Signed-off-by: Nischal Sharma <[email protected]>
auto-merge was automatically disabled August 21, 2024 18:42

Head branch was pushed to by a user without write access

garyschulte and others added 2 commits August 21, 2024 13:35
Signed-off-by: Nischal Sharma <[email protected]>
@garyschulte
Copy link
Contributor

The weird requirement about darwin vs linux shared library location notwithstanding, lets MERGE this and we can sort out that minor oddity in a subsequent PR 👍

@garyschulte garyschulte merged commit e996d8e into hyperledger:main Aug 21, 2024
11 checks passed
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.

Add linux arm64 support in Constantine build
2 participants