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 JVMTI callback/hook for virtualThread #15254

Merged
merged 2 commits into from
Jun 17, 2022

Conversation

fengxue-IS
Copy link
Contributor

Related: #15183

Signed-off-by: Jack Lu Jack.S.Lu@ibm.com

Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

minor formatting nitpicks. otherwise, LGTM.

runtime/jvmti/jvmtiCapability.c Outdated Show resolved Hide resolved
runtime/jvmti/jvmtiHelpers.c Outdated Show resolved Hide resolved
runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved
Comment on lines +149 to +150
static void jvmtiHookVirtualThreadStarted (J9HookInterface** hook, UDATA eventNum, void* eventData, void* userData);
static void jvmtiHookVirtualThreadEnd (J9HookInterface** hook, UDATA eventNum, void* eventData, void* userData);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static void jvmtiHookVirtualThreadStarted (J9HookInterface** hook, UDATA eventNum, void* eventData, void* userData);
static void jvmtiHookVirtualThreadEnd (J9HookInterface** hook, UDATA eventNum, void* eventData, void* userData);
static void jvmtiHookVirtualThreadStarted (J9HookInterface **hook, UDATA eventNum, void *eventData, void *userData);
static void jvmtiHookVirtualThreadEnd (J9HookInterface **hook, UDATA eventNum, void *eventData, void *userData);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am leaning towards keeping the existing format here, or else we could just change all the function headers to match standard

runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved
runtime/jvmti/jvmtiHook.c Show resolved Hide resolved
runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved
runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved
runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved
runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved
@fengxue-IS fengxue-IS force-pushed the jvmti_stub branch 2 times, most recently from 88be440 to 18642a4 Compare June 8, 2022 06:11
@fengxue-IS fengxue-IS marked this pull request as ready for review June 8, 2022 15:19
Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

LGTM, approving.

I would suggest strengthening the content in the commit message and PR description by highlighting the specifics for which support is introduced:

  • jvmtiHookVirtualThreadStarted
  • jvmtiHookVirtualThreadEnd
  • can_support_virtual_threads

runtime/oti/j9vm.hdf Show resolved Hide resolved
@babsingh babsingh requested a review from tajila June 8, 2022 20:03
runtime/include/jvmti.h.m4 Outdated Show resolved Hide resolved
runtime/include/jvmti.h.m4 Outdated Show resolved Hide resolved
@fengxue-IS fengxue-IS force-pushed the jvmti_stub branch 2 times, most recently from 78ad61e to 237b9f4 Compare June 14, 2022 20:36
- New hook methods:
    jvmtiHookVirtualThreadStarted
    jvmtiHookVirtualThreadEnd
- Enable can_support_virtual_threads by default for Java19

Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>
Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>
@tajila
Copy link
Contributor

tajila commented Jun 15, 2022

Jenkins test sanity amac jdknext

@tajila
Copy link
Contributor

tajila commented Jun 15, 2022

Jenkins test compile win jdknext

@fengxue-IS
Copy link
Contributor Author

amac compile passed, PR testing failed due to infra issue
@tajila can you restart the testing

@tajila
Copy link
Contributor

tajila commented Jun 16, 2022

Jenkins test compile win jdknext

@tajila
Copy link
Contributor

tajila commented Jun 16, 2022

Jenkins test sanity amac jdknext

@tajila tajila merged commit dbe2071 into eclipse-openj9:master Jun 17, 2022
@babsingh babsingh mentioned this pull request Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project:loom Used to track Project Loom related work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants