On 7/8/16 12:59, [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?
I realized, it is not correct. Sorry for the confusion. The above
contradicts with the statement from Jiangli: > 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. So, we want to disallow loading classes
from the shared archive if the CFLH events are enabled late. I'm still
in process of reviewing. Thanks, Serguei
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
