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 -XX:[+/-]EnableDynamicAgentLoading flag #17215

Merged
merged 1 commit into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions runtime/jvmti/j9jvmti.tdf
Original file line number Diff line number Diff line change
Expand Up @@ -658,3 +658,5 @@ TraceExit=Trc_JVMTI_jvmtiHookVirtualThreadUnmount_Exit Overhead=1 Level=5 Noenv

TraceEntry=Trc_JVMTI_jvmtiHookVirtualThreadMount_Entry Overhead=1 Level=5 Noenv Template="HookVirtualThreadMount"
TraceExit=Trc_JVMTI_jvmtiHookVirtualThreadMount_Exit Overhead=1 Level=5 Noenv Template="HookVirtualThreadMount"

TraceEvent=Trc_JVMTI_loadAgentLibraryOnAttach_agentLoadingDisabled Overhead=1 Level=5 Noenv Template="loadAgentLibraryOnAttach: Dynamic agent loading is disabled"
20 changes: 18 additions & 2 deletions runtime/jvmti/jvmtiStartup.c
Original file line number Diff line number Diff line change
Expand Up @@ -563,9 +563,9 @@ I_32 JNICALL
loadAgentLibraryOnAttach(struct J9JavaVM * vm, const char * library, const char *options, UDATA decorate)
{
PORT_ACCESS_FROM_JAVAVM(vm);
UDATA rc;
UDATA rc = JNI_OK;
UDATA optionsLength = 0;
UDATA libraryLength;
UDATA libraryLength = 0;
J9JVMTIAgentLibrary *agentLibrary = NULL;
J9JVMTIData * jvmtiData = J9JVMTI_DATA_FROM_VM(vm);
char loadFunctionName[J9JVMTI_BUFFER_LENGTH + 1] = {0};
Expand All @@ -575,6 +575,12 @@ loadAgentLibraryOnAttach(struct J9JavaVM * vm, const char * library, const char

Trc_JVMTI_loadAgentLibraryOnAttach_Entry(library);

if (J9_ARE_NO_BITS_SET(vm->runtimeFlags, J9_RUNTIME_ALLOW_DYNAMIC_AGENT)) {
Trc_JVMTI_loadAgentLibraryOnAttach_agentLoadingDisabled();
rc = JNI_ERR;
goto exit;
}

Assert_JVMTI_true(NULL != library); /* Library name must be non-null. */

if (NULL != options) {
Expand Down Expand Up @@ -701,6 +707,16 @@ loadAgentLibrary(J9JavaVM * vm, J9JVMTIAgentLibrary * agentLibrary)

Trc_JVMTI_loadAgentLibrary_Entry(agentLibrary->nativeLib.name);

/* If dynamic agent loading is disabled, the HCR/OSR flags default to off.
* However, adding agents at launch is allowed. In this case we need to
* enable the flags for proper functionality.
*/
if (J9_ARE_NO_BITS_SET(vm->runtimeFlags, J9_RUNTIME_ALLOW_DYNAMIC_AGENT)) {
omrthread_monitor_enter(vm->runtimeFlagsMutex);
vm->extendedRuntimeFlags |= J9_EXTENDED_RUNTIME_OSR_SAFE_POINT | J9_EXTENDED_RUNTIME_ENABLE_HCR;
omrthread_monitor_exit(vm->runtimeFlagsMutex);
}

/* Attempt linking the agent statically, looking for Agent_OnLoad_L.
* If this is not found, fall back on the regular, dynamic linking way.
*/
Expand Down
2 changes: 1 addition & 1 deletion runtime/oti/j9consts.h
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ extern "C" {
#define J9_RUNTIME_ARGENCODING_UNICODE 0x2000
#define J9_RUNTIME_ARGENCODING_LATIN 0x4000
#define J9_RUNTIME_ARGENCODING_UTF8 0x8000
#define J9_RUNTIME_UNUSED_0x10000 0x10000
#define J9_RUNTIME_ALLOW_DYNAMIC_AGENT 0x10000
#define J9_RUNTIME_UNUSED_0x20000 0x20000
#define J9_RUNTIME_UNUSED_0x40000 0x40000
#define J9_RUNTIME_UNUSED_0x80000 0x80000
Expand Down
3 changes: 3 additions & 0 deletions runtime/oti/jvminit.h
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,9 @@ enum INIT_STAGE {
#define VMOPT_XSYSLOG_OPT "-Xsyslog"
#define MAPOPT_XSYSLOG_OPT_COLON "-Xsyslog:"

#define VMOPT_XXENABLEDYNAMICAGENTLOADING "-XX:+EnableDynamicAgentLoading"
#define VMOPT_XXNOENABLEDYNAMICAGENTLOADING "-XX:-EnableDynamicAgentLoading"

/* Modularity command line options */
#define VMOPT_MODULE_UPGRADE_PATH "--upgrade-module-path"
#define VMOPT_MODULE_PATH "--module-path"
Expand Down
14 changes: 12 additions & 2 deletions runtime/vm/jvminit.c
Original file line number Diff line number Diff line change
Expand Up @@ -3569,10 +3569,20 @@ processVMArgsFromFirstToLast(J9JavaVM * vm)
if (J2SE_VERSION(vm) >= J2SE_V11) {
vm->extendedRuntimeFlags |= (UDATA)J9_EXTENDED_RUNTIME_RESTRICT_IFA; /* Enable zAAP switching for Registered Natives and JVMTI callbacks by default in Java 9 and later. */
}
vm->extendedRuntimeFlags |= J9_EXTENDED_RUNTIME_OSR_SAFE_POINT; /* Enable OSR safe point by default */
vm->extendedRuntimeFlags |= (UDATA)J9_EXTENDED_RUNTIME_ENABLE_HCR; /* Enable HCR by default */
vm->extendedRuntimeFlags2 |= J9_EXTENDED_RUNTIME2_ENABLE_UTF_CACHE; /* Enable UTF cache by default */

{
IDATA noDynamicAgentLoading = FIND_AND_CONSUME_VMARG(EXACT_MATCH, VMOPT_XXNOENABLEDYNAMICAGENTLOADING, NULL);
IDATA dynamicAgentLoading = FIND_AND_CONSUME_VMARG(EXACT_MATCH, VMOPT_XXENABLEDYNAMICAGENTLOADING, NULL);
if (noDynamicAgentLoading > dynamicAgentLoading) {
vm->runtimeFlags &= ~J9_RUNTIME_ALLOW_DYNAMIC_AGENT;
vm->extendedRuntimeFlags &= ~(J9_EXTENDED_RUNTIME_OSR_SAFE_POINT | J9_EXTENDED_RUNTIME_ENABLE_HCR);
} else {
vm->runtimeFlags |= J9_RUNTIME_ALLOW_DYNAMIC_AGENT;
vm->extendedRuntimeFlags |= (J9_EXTENDED_RUNTIME_OSR_SAFE_POINT | J9_EXTENDED_RUNTIME_ENABLE_HCR);
}
Copy link
Member

Choose a reason for hiding this comment

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

A couple of comments:

  1. -XX:[+|-]EnableDynamicAgentLoading is a new option introduced since Java 9, and should not be applied to Java 8;
  2. 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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

@JasonFengJ9 JasonFengJ9 Apr 18, 2023

Choose a reason for hiding this comment

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

-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.

}

#if defined(J9VM_ARCH_X86) || defined(J9VM_ARCH_POWER) || defined(J9VM_ARCH_S390)
/* Enabled field watch by default on x86, Power, and S390 platforms */
vm->extendedRuntimeFlags |= J9_EXTENDED_RUNTIME_JIT_INLINE_WATCHES;
Expand Down