JDK-8069149: jdk.internal.pref.Perf is unaware of the VM option UsePerfData

[David Holmes]

Hi Gary,

Meta-question partially raised in the the recent bug report comments: do 
we care about this any more? Does anything need to disable UsePerfData? 
Can we not deprecate it in 11 then obsolete in 12? That would be a lot 
simpler.

That aside ...

It's hard to see how all the different paths in these related API's 
stitch together - especially when you have to consider the API being 
used on the current VM or a different one. I'm not at all clear what the 
control flow is, so it is hard to see exactly how the initialization 
issue is being handled. As I wrote in the initial report it is unclear 
exactly which parts of the "perf" subsystem are impacted by the value of 
UsePerfData.

Can you summarize the role of the is_initialized check versus a direct 
UsePerfData check on the VM side i.e. which should be checked when? I 
immediately see an issue that Perf_Detach is a no-op when !UsePerfdata, 
yet Perf_Attach doesn't examine the flag at all! But perhaps we never 
reach those methods because of a higher-level call filtering it out ??

More below ...

On 1/06/2018 5:09 AM, Gary Adams wrote:
> A patch was done previously to prevent an error when -XX:-UsePerfData
> is used, but this bug was filed to make the setting more visible in the
> jdk.internal.perf package.
> 
>     Webrev: http://cr.openjdk.java.net/~gadams/8069149/webrev.00/
>     Issue: https://bugs.openjdk.java.net/browse/JDK-8069149
> 
> I have tried a few initial tests using:
> 
>     make run-tests \
>           TEST_OPTS=VM_OPTIONS=-XX:-UsePerfData \
>           TEST=open/test/jdk/sun/management/jmxremote
> 
> While I'm tracking down one test timeout,  I'd like to get some
> feedback on the approach used here :
> 
>     - the basic mechanism propagates the "is initialized" state up to 
> Perf.java;

Within PerfMemory UsePerfData and is_usable() are used for guarding 
execution. Perhaps that is what needs to be propagated up?

>        does the state need to be exposed to users getPerf(), or is it 
> sufficient
>        to provide exceptions at this level.

Unclear what the control flow is here. I don't see that Perf triggers 
initialization of the perf subsystem, so what guarantees initialization 
should have happened by the time the Perf class is initialized ??

>     - an existing use of IOException was used in the case of attach 
> failures;
>        added IOExceptions to the createXXX methods if the memory was not 
> initialized

On the VM side can you at least add a message to the IOException so that 
the user can tell why it was thrown.

>     - presuming a CSR would be needed to cover the IOExceptions

It's not a public API so no CSR should be needed.

>     - it appears that jdk.internal.perf has very limited usage,
>        is it still needed/used (?)

I can't answer that part, but I don't think UsePerfData is needed any more.

Thanks,
David

> Any feedback appreciated.
>