Hi Alex,
It looks good.
Just a couple of minor comments though.
109 log("onLoad = " + onLoadValue);
110 log("live= " + liveValue);
111 if (onLoadValue != liveValue || onLoadValue < 0) {
112 throw new RuntimeException("Inconsistent
can_generate_all_class_hook_events value");
113 }
It'd be nice to make more clear statement about why the test has been
failed.
A couple of things to cover:
- the capability is expected to be available in both phases,
but now the test would pass if both phases returned false
- an unexpected difference in returned capability in onload and live
phases
- what does it mean if the printed capability value is negative
Something like the following would work:
log("can_generate_all_class_hook_events capability values returned in
ONLOAD and LIVE phases:");
log("ONLOAD phase: " + (onLoadValue < 0 ? "Failed to read" :
onLoadValue);
log("LIVE phase: " + (liveValue < 0 ? "Failed to read" : liveValue);
if (onLoadValue != 1 || liveValue != 1) {
throw new RuntimeException("The can_generate_all_class_hook_events "
" expected to be available in ONLOAD and LIVE phases");
}
It is better to remove the line:
145 //log("CommandLine: " + params.stream().collect(Collectors.joining("
")));
Thanks,
Serguei
On 2/1/18 16:11, Alex Menkov wrote:
Hi Serguei,
Thanks for the feedback.
Updated fix:
http://cr.openjdk.java.net/~amenkov/can_generate_class_hook/webrev.02/
--alex
On 02/01/2018 11:00, serguei.spit...@oracle.com wrote:
One more attempt to send it with the correct to-list...
Sorry for the noise.
Hi Alex,
It looks good in general.
A couple of comments on the test.
We had a convention to avoid having test sub-folders with bug numbers.
Could you, please, rename it to some symbolic name?
Also we need the test to fail in the native agent when an error is
reported with the reportError().
It'd be great if some CDS/AppCDS experts from the Runtime team look
at the fix and test
so I've included Jiangli and Misha into the to-list.
Thanks,
Serguei
On 1/25/18 16:11, Alex Menkov wrote:
Hi all,
Please take a look at a fix for
JDK-8161605 : The '!UseSharedSpaces' check is not need in
JvmtiManageCapabilities::recompute_always_capabilities
The fix moves can_generate_all_class_hook_events capability from
"onload_capabilities" to "always_capabilities" and cleans up
recompute_always_capabilities method.
jira: https://bugs.openjdk.java.net/browse/JDK-8161605
webrev:
http://cr.openjdk.java.net/~amenkov/can_generate_class_hook/webrev/
--alex
