Hi Jiangli,
Ok, I'll wait for the next webrev version.
Thanks,
Serguei
On 7/8/16 18:04, Jiangli Zhou wrote:
Hi Serguei,
Thank you for looking into the changes. With Ioi and Karen’s suggestion for
dynamically attached agent and -Xshare:on, I’m reworking the changes. I’m
planning to remove the check below and do that in a later place. I’ll post the
new webrev next week.
Thanks,
Jiangli
On Jul 8, 2016, at 12:59 PM, [email protected] wrote:
Hi Jiangli,
src/share/vm/memory/metaspace.cpp
3177 if (UseSharedSpaces) {
3178 FileMapInfo* mapinfo = new FileMapInfo();
3179
3180 if (JvmtiExport::should_post_class_file_load_hook()) {
3181 // Currently CDS does not support JVMTI CFHL when loading shared class.
3182 // If JvmtiExport::should_post_class_file_load_hook is already enabled,
3183 // just disable UseSharedSpaces.
3184 FileMapInfo::fail_continue("Tool agent requires sharing to be disabled.");
3185 } else {
I understood, the FileMapInfo::fail_continue() will fail if the
RequireSharedSpaces is enabled which is expected.
Should the mapinfo be deallocated in the case FileMapInfo::fail_continue() is
called?
Also, there is a comment below.
On 7/7/16 18:56, Ioi Lam wrote:
(Adding [email protected])
Hi Jiangli,
I think the two blocks of
if (RequireSharedSpaces) {
tty->print_cr("An error has occurred while loading shared class.");
tty->print_cr("Tool agent requires sharing to be disabled.");
vm_exit(1);
}
should be removed.
If the agent is specified in the command line, the JVM start would be aborted.
This is already handled by your changes to src/share/vm/memory/metaspace.cpp
If the agent is attached later, after the VM has started, I think we should not
quit the VM. Consider this scenario -- a production server could be running for
a long time without issues using -Xshare:on, then when the user attaches a
diagnostic agent the JVM suddenly quits. I think it's better to cause the
agent attachment to fail with something like
jvmtiError
JvmtiEnv::SetEventNotificationMode(jvmtiEventMode mode, jvmtiEvent event_type,
jthread event_thread, ...) {
...
if (event_type == JVMTI_EVENT_CLASS_FILE_LOAD_HOOK && enabled) {
+ if (RequireSharedSpaces && !universe::is_bootstraping()) {
+ tty->print_cr("Tool agent that requires JVMTI_EVENT_CLASS_FILE_LOAD_HOOK
cannot be dynamically loaded after JVM has started");
+ return JVMTI_ERROR_ILLEGAL_ARGUMENT;
+ }
record_class_file_load_hook_enabled();
}
...
}
This is a good concern and suggestion.
We may need more sophisticated handling for this.
Sorry, I did not notice it earlier.
There is a capability can_generate_all_class_hook_events.
We already have this in the prims/jvmtiManageCapabilities.cpp:
// if the capability sets are initialized in the onload phase then // it
happens before class data sharing (CDS) is initialized. If it // turns out that
CDS gets disabled then we must adjust the always // capabilities. To ensure a
consistent view of the capabililties // anything we add here should already be
in the onload set. void
JvmtiManageCapabilities::recompute_always_capabilities() { if
(!UseSharedSpaces) { jvmtiCapabilities jc = always_capabilities;
jc.can_generate_all_class_hook_events = 1; always_capabilities = jc; } } If
enabled can_generate_all_class_hook_events sets the
flagJvmtiExport::can_modify_any_class() in jvmtiManageCapabilities.cpp:
JvmtiExport::set_can_modify_any_class( avail.can_generate_breakpoint_events ||
avail.can_generate_all_class_hook_events); The flag
JvmtiExport::can_modify_any_class() is used here:
char* FileMapInfo::map_region(int i) { . . . // If a tool agent is in use
(debugging enabled), we must map the address space RW if
(JvmtiExport::can_modify_any_class() || JvmtiExport::can_walk_any_space()) {
si->_read_only = false; }
So, I'm thinking if we could just skip the CFLH events for the shared classes
if the capability can_generate_all_class_hook_events is not enabled and
continue posting the events for non-shared classes. Then we do not need to
tweak the SetEventNotificationMode() implementation. Otherwise, the fragment
above may need to be updated too. To summarize, I understood that the current
approach/plan for JDK 9 is: - Disable the CDS if detected early that the CFLH
events are enabled - Skip the CFLH events for shared classes if CFLH is
enabled late Is this right? Is it the same as in the JDK 8? Thanks, Serguei
Thanks - Ioi On 7/7/16 6:08 PM, Jiangli Zhou wrote:
Please review the following webrev that disables CDS when
JvmtiExport::should_post_class_file_load_hook() is enabled at runtime. webrev:
http://cr.openjdk.java.net/~jiangli/8141341/webrev.00/ bug: JDK-8141341
<https://bugs.openjdk.java.net/browse/JDK-8141341> With -Xshare:on If -Xshare:on is
used and JvmtiExport::should_post_class_file_load_hook() is required, the VM now reports
"Tool agent requires sharing to be disabled” and terminates. This is the same
behavior as jdk8. With -Xshare:auto The change in meatspace.cpp is to detect the case
where JvmtiExport::should_post_class_file_load_hook() is enabled very early during JVM
initialization. In that case, we disable CDS entirely, including all shared classes,
symbols, and Strings objects. The change in systemDictionary.cpp is to detect the cases
where JvmtiExport::should_post_class_file_load_hook() is enabled late, which include agent
dynamically attached at runtime. In those cases, JVM does not allow loading classes from
the shared archive. The shared symbols and Strings can still be used. Thanks, Jiangli
