Ok, I'll update the comment. I think I'll still mention and CDS and
maybe add AOT as examples.
thanks,
Chris
On 6/6/18 7:01 PM, David Holmes wrote:
Hi Chris,
Great sleuthing on this one and thanks for explaining things for me in
the bug report!
The fix you have addresses the problem observed with the "vm info"
string. One small request, can you update this comment in thread.cpp
please:
// We need this for ClassDataSharing - the initial vm.info property
is set
// with the default value of CDS "sharing" which may be reset through
// command line options.
reset_vm_info_property(CHECK_JNI_ERR);
As we actually need that for any piece of information that might be
used in the vm info string initially, that can subsequently change due
to option processing e.g. UseSharedSpaces and UseAOT. Suggestion:
// We need this to update the vm.info property in case any flags used
// to initially define it have been changed with command-line options.
reset_vm_info_property(CHECK_JNI_ERR);
Thanks!
---
That said I think we can simplify things somewhat now you discovered
the bug in reset_vm_info_property. If we simply update the native
vm_info property as soon as we've completed argument parsing, then we
will have the right value in place for copying to the Java level
during java.lang.System initialization. We don't need any of the
existing code in reset_vm_info because we copy the correct vm_info in
the first place. In short you should be able to fix this with simply:
jint parse_result = Arguments::parse(args);
if (parse_result != JNI_OK) return parse_result;
os::init_before_ergo();
jint ergo_result = Arguments::apply_ergo();
if (ergo_result != JNI_OK) return ergo_result;
+ const char *vm_info = VM_Version::vm_info_string();
+ Arguments::PropertyList_update_value(Arguments::system_properties(),
"java.vm.info", vm_info);
...
- // We need this for ClassDataSharing - the initial vm.info property
is set
- // with the default value of CDS "sharing" which may be reset through
- // command line options.
- reset_vm_info_property(CHECK_JNI_ERR);
and delete reset_vm_info_property.
Just a thought - feel free to defer - it's really a runtime issue
anyway. :)
Thanks,
David
On 7/06/2018 8:24 AM, Chris Plummer wrote:
Hello,
Please review the following:
https://bugs.openjdk.java.net/browse/JDK-8203329
http://cr.openjdk.java.net/~cjplummer/8203329/webrev.00/
The native version of the java.vm.info property was getting out of
sync with the java version. Details can be found here:
https://bugs.openjdk.java.net/browse/JDK-8203329?focusedCommentId=14185679&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14185679
thanks,
Chris
