Add support for -XX:[+/-]EnableDynamicAgentLoading flag #17215

dnakamura · 2023-04-18T15:50:44Z

Flag controls runtime loading of dynamic agents

When EnableDynamicAgentLoading is enabled, OSR/HCR flags are enabled and dynamic agent loading is also enabled.
When EnableDynamicAgentLoading is disabled, OSR/HCR flags default to off, and are enabled if/when agents are attached at startup. Dynamic loading of agents is disabled.

JasonFengJ9 · 2023-04-18T18:56:39Z

runtime/vm/jvminit.c

+			vm->extendedRuntimeFlags &= ~(UDATA)(J9_EXTENDED_RUNTIME_OSR_SAFE_POINT | (UDATA) J9_EXTENDED_RUNTIME_ENABLE_HCR);
+		} else {
+			vm->extendedRuntimeFlags |= (J9_EXTENDED_RUNTIME_OSR_SAFE_POINT | (UDATA)J9_EXTENDED_RUNTIME_ENABLE_HCR);
+		}


A couple of comments:

-XX:[+|-]EnableDynamicAgentLoading is a new option introduced since Java 9, and should not be applied to Java 8;

Is -XX:[+|-]EnableDynamicAgentLoading going to take precedence over -XX:[+|-]EnableHCR or -XX:[+|-]OSRSafePoint? if so, this code block needs to be after processing of VMOPT_XXENABLEOSRSAFEPOINT and VMOPT_XXENABLEHCR.

We should consider if it can apply to Java 8. As long as it doesn't change the default behaviour I don't see why it can't apply.

-XX:[+|-]EnableHCR and -XX:[+|-]OSRSafePoint are Java 8 compatible, if there is a need to disable such capabilities for Java 8, existing flags can be applied.
~~If a new flag is introduced, we can apply it to Java 8 if required w/o changing default behaviours.~~

tajila · 2023-05-08T14:31:58Z

I've updated the wording in the issue, #17085.

This PR is a good starting point. Additionally, we need to add the OSR/HCR flags whenever an agentlib is attached. We should also add a vm->extendedRuntimeFlag to toggles whether dynamic enablement of agents is possible or not.

fengxue-IS · 2023-06-05T18:13:30Z

Is this change ready for merge? #17508 will need to use this option to determine if skipping jvmti notify can be skipped

JasonFengJ9 · 2023-06-05T18:15:46Z

Is this change ready for merge? #17508 will need to use this option to determine if skipping jvmti notify can be skipped

I guess this PR can be cherry-pick by

Investigate perf impact of VirtualThread notifyJVMTI APIs #17508

dnakamura · 2023-06-09T17:37:51Z

Should be ready now.

runtime/jvmti/j9jvmti.tdf

runtime/jvmti/jvmtiStartup.c

runtime/vm/jvminit.c

dnakamura · 2023-06-13T18:19:26Z

Issues should be fixed

pshipton · 2023-06-14T17:13:47Z

@JasonFengJ9 do you have any further concerns?

runtime/jvmti/jvmtiStartup.c

runtime/vm/jvminit.c

tajila · 2023-07-05T17:00:26Z

@keithc-ca Any more concerns with this PR?

tajila · 2023-07-11T13:36:48Z

@dnakamura can you address Keiths comment

I think improving the comment on line 710 would solve my remaining concerns; yes, those flags were cleared earlier, but explaining why we want to set them now would help.

dnakamura · 2023-07-11T16:45:10Z

@tajila should be fixed

tajila · 2023-07-11T17:19:57Z

@keithc-ca any further concerns?

runtime/jvmti/jvmtiStartup.c

dnakamura · 2023-07-11T18:41:11Z

@keithc-ca fixed

keithc-ca · 2023-07-11T19:20:16Z

The description and the commit summary could be improved; the option is for enabling or disabling dynamic agents. The reference to the issue helps, but people reviewing the commit history shouldn't need to consult the internet.

Flag controls runtime loading of dynamic agents - When EnableDynamicAgentLoading is enabled, OSR/HCR flags are enabled and dynamic agent loading is also enabled. - When EnableDynamicAgentLoading is disabled, OSR/HCR flags default to off, and are enabled if/when agents are attached at startup. Dynamic loading of agents is disabled. Issue eclipse-openj9#17085 Signed-off-by: Devin Nakamura <[email protected]>

tajila · 2023-07-11T22:05:38Z

jenkins test sanity xlinux jdk20

tajila · 2023-07-11T22:05:47Z

jenkins compile win jdk11

pshipton · 2023-07-12T23:53:51Z

Pls create a doc issue for the new option at https://github.com/eclipse-openj9/openj9-docs/issues/

dnakamura mentioned this pull request Apr 18, 2023

Add support for -XX:+EnableDynamicAgentLoading #17085

Closed

tajila requested a review from JasonFengJ9 April 18, 2023 17:28

JasonFengJ9 reviewed Apr 18, 2023

View reviewed changes

tajila added this to the Java 21 milestone Apr 24, 2023

dnakamura force-pushed the dynamic_agent branch from efefe76 to 86e7238 Compare June 9, 2023 16:36

dnakamura marked this pull request as ready for review June 9, 2023 17:37

pshipton reviewed Jun 9, 2023

View reviewed changes

runtime/jvmti/j9jvmti.tdf Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiStartup.c Outdated Show resolved Hide resolved

runtime/vm/jvminit.c Outdated Show resolved Hide resolved

dnakamura force-pushed the dynamic_agent branch 2 times, most recently from a023106 to e724073 Compare June 13, 2023 18:18

pshipton requested a review from keithc-ca June 14, 2023 17:13

pshipton added the comp:vm label Jun 14, 2023

keithc-ca requested changes Jun 14, 2023

View reviewed changes

runtime/jvmti/jvmtiStartup.c Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiStartup.c Outdated Show resolved Hide resolved

runtime/vm/jvminit.c Outdated Show resolved Hide resolved

dnakamura force-pushed the dynamic_agent branch 3 times, most recently from d6bcdfd to 591c814 Compare June 27, 2023 17:56

JasonFengJ9 approved these changes Jul 5, 2023

View reviewed changes

dnakamura force-pushed the dynamic_agent branch from 591c814 to 1ee44c0 Compare July 11, 2023 15:16

keithc-ca requested changes Jul 11, 2023

View reviewed changes

runtime/jvmti/jvmtiStartup.c Outdated Show resolved Hide resolved

dnakamura force-pushed the dynamic_agent branch from 1ee44c0 to 408ef12 Compare July 11, 2023 18:40

dnakamura changed the title ~~Add command line option for dynamic agent loading~~ Add support for -XX:[+/-]EnableDynamicAgentLoading flag Jul 11, 2023

dnakamura force-pushed the dynamic_agent branch from 408ef12 to 780d568 Compare July 11, 2023 20:09

keithc-ca approved these changes Jul 11, 2023

View reviewed changes

tajila merged commit f7d246a into eclipse-openj9:master Jul 12, 2023

pshipton added the depends:doc label Jul 12, 2023

JasonFengJ9 mentioned this pull request Aug 14, 2023

Documentation for -XX:[+|-]EnableDynamicAgentLoading eclipse-openj9/openj9-docs#1140

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for -XX:[+/-]EnableDynamicAgentLoading flag #17215

Add support for -XX:[+/-]EnableDynamicAgentLoading flag #17215

dnakamura commented Apr 18, 2023 •

edited

Loading

JasonFengJ9 Apr 18, 2023

pshipton Apr 18, 2023

JasonFengJ9 Apr 18, 2023 •

edited

Loading

tajila commented May 8, 2023

fengxue-IS commented Jun 5, 2023

JasonFengJ9 commented Jun 5, 2023

dnakamura commented Jun 9, 2023

dnakamura commented Jun 13, 2023

pshipton commented Jun 14, 2023

tajila commented Jul 5, 2023

tajila commented Jul 11, 2023

dnakamura commented Jul 11, 2023

tajila commented Jul 11, 2023

dnakamura commented Jul 11, 2023

keithc-ca commented Jul 11, 2023

tajila commented Jul 11, 2023

tajila commented Jul 11, 2023

pshipton commented Jul 12, 2023 •

edited

Loading

Add support for -XX:[+/-]EnableDynamicAgentLoading flag #17215

Add support for -XX:[+/-]EnableDynamicAgentLoading flag #17215

Conversation

dnakamura commented Apr 18, 2023 • edited Loading

JasonFengJ9 Apr 18, 2023

Choose a reason for hiding this comment

pshipton Apr 18, 2023

Choose a reason for hiding this comment

JasonFengJ9 Apr 18, 2023 • edited Loading

Choose a reason for hiding this comment

tajila commented May 8, 2023

fengxue-IS commented Jun 5, 2023

JasonFengJ9 commented Jun 5, 2023

dnakamura commented Jun 9, 2023

dnakamura commented Jun 13, 2023

pshipton commented Jun 14, 2023

tajila commented Jul 5, 2023

tajila commented Jul 11, 2023

dnakamura commented Jul 11, 2023

tajila commented Jul 11, 2023

dnakamura commented Jul 11, 2023

keithc-ca commented Jul 11, 2023

tajila commented Jul 11, 2023

tajila commented Jul 11, 2023

pshipton commented Jul 12, 2023 • edited Loading

dnakamura commented Apr 18, 2023 •

edited

Loading

JasonFengJ9 Apr 18, 2023 •

edited

Loading

pshipton commented Jul 12, 2023 •

edited

Loading