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.

Sign up for GitHub

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

Jump to bottom

[FFI] Perf improvement via NativeMethodHandle/linkToNative #736

Merged

keithc-ca merged 1 commit into ibmruntimes:openj9 from ChengJin01:ffi_perf_improvement_nativeMH_jdknext

Feb 2, 2024

ChengJin01 commented Jan 23, 2024

The changes aim to generate the NativeMethodHandle instance to ensure the invocation path goes via linkToNative.

Signed-off-by: ChengJin01 [email protected]

This was referenced Jan 23, 2024

[FFI] Perf improvement via NativeMethodHandle/linkToNative ibmruntimes/openj9-openjdk-jdk22#17

Merged

[FFI/JDK22] Performance improvement via NativeMH/LinkToNative eclipse-openj9/openj9#18799

Merged

Author

ChengJin01 commented Jan 23, 2024

Reviewer: @tajila
FYI: @keithc-ca, @pshipton

keithc-ca reviewed

View reviewed changes

src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java Outdated Show resolved Hide resolved

ChengJin01 force-pushed the ffi_perf_improvement_nativeMH_jdknext branch from e9dbc59 to 2f27bf2 Compare

January 24, 2024 19:14

keithc-ca reviewed

View reviewed changes

src/java.base/share/classes/java/lang/invoke/NativeMethodHandle.java Outdated Show resolved Hide resolved

keithc-ca reviewed

View reviewed changes

src/java.base/share/classes/java/lang/invoke/NativeMethodHandle.java Outdated Show resolved Hide resolved

keithc-ca reviewed

View reviewed changes

src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java Outdated Show resolved Hide resolved

keithc-ca reviewed

View reviewed changes

src/java.base/share/classes/java/lang/invoke/NativeMethodHandle.java Show resolved Hide resolved

ChengJin01 force-pushed the ffi_perf_improvement_nativeMH_jdknext branch 2 times, most recently from 44a6478 to 9699dce Compare

January 24, 2024 20:08

keithc-ca reviewed

View reviewed changes

src/java.base/share/classes/java/lang/invoke/NativeMethodHandle.java Outdated Show resolved Hide resolved

ChengJin01 force-pushed the ffi_perf_improvement_nativeMH_jdknext branch from 9699dce to f10378f Compare

January 24, 2024 20:14

keithc-ca requested changes

View reviewed changes

src/java.base/share/classes/java/lang/invoke/NativeMethodHandle.java Show resolved Hide resolved

src/java.base/share/classes/java/lang/invoke/NativeMethodHandle.java Outdated Show resolved Hide resolved

ChengJin01 force-pushed the ffi_perf_improvement_nativeMH_jdknext branch 2 times, most recently from 1c981d8 to fb03ad8 Compare

January 30, 2024 22:45

keithc-ca reviewed

View reviewed changes

Member

keithc-ca left a comment

This change looks good, but the "JDKnext" part of the title and commit message seems redundant (it's implied by the repo).


          [FFI] Perf improvement via NativeMethodHandle/linkToNative

e4d0400

The changes aim to generate the NativeMethodHandle instance
to ensure the invocation path goes via linkToNative.

Signed-off-by: ChengJin01 <[email protected]>

ChengJin01 force-pushed the ffi_perf_improvement_nativeMH_jdknext branch from fb03ad8 to e4d0400 Compare

January 31, 2024 15:04

Author

ChengJin01 commented Jan 31, 2024

This change looks good, but the "JDKnext" part of the title and commit message seems redundant (it's implied by the repo).

Removed JDKnext as suggested.

ChengJin01 changed the title ~~[FFI/JDKnext] Perf improvement via NativeMethodHandle/linkToNative~~ [FFI] Perf improvement via NativeMethodHandle/linkToNative

tajila approved these changes

View reviewed changes

Member

keithc-ca commented Feb 1, 2024

Jenkins test sanity amac,zlinux,win jdknext depends eclipse-openj9/openj9#18799

Author

ChengJin01 commented Feb 2, 2024 •

edited

Loading

The test failure with LoadLibrary on Windows at https://openj9-jenkins.osuosl.org/job/Test_openjdknext_j9_sanity.functional_x86-64_windows_Personal/21/tapResults/ has nothing to do with our changes.

TESTING:
*** Starting test suite: LoadLibrary test intended for CMVC 201408 ***
Testing: LoadLibrary testcase with invalid paths specified in the library path list
Test start time: 2024/02/01 19:39:36 Eastern Standard Time
Running command: C:/Users/jenkins/workspace/Test_openjdknext_j9_sanity.functional_x86-64_windows_Personal_testList_1/jdkbinary/j2sdk-image\bin\java 
-Djava.library.path=".;'C:\\Program Files (x86)\\ibm\\tivoli';C:\\Users\\j9build;C:\\dev;." 
-cp C:/Users/jenkins/workspace/Test_openjdknext_j9_sanity.functional_x86-64_windows_Personal_testList_1/aqa-tests///..//jvmtest\functional\cmdLineTests
\loadLibraryTest\loadLibraryTest.jar org.openj9.test.loadLibrary.TestLoadLibrary
Time spent starting: 78 milliseconds
Time spent executing: 1093 milliseconds
Test result: FAILED

Member

keithc-ca commented Feb 2, 2024

The only test failure was in cmdLineTester_loadLibraryTests, but I don't think that's related to this change.

keithc-ca approved these changes

View reviewed changes

keithc-ca merged commit 482c19e into ibmruntimes:openj9

7 of 9 checks passed

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

Labels

None yet