RFR: 8146001: Remove support for command line options from JVMCI

[Doug Simon]

> On 06 Jan 2016, at 18:54, Christian Thalinger <christian.thalinger at oracle.com> wrote:
> 
> I just noticed this code in HotSpotResolvedJavaMethodImpl:
> 
>     private static final String TraceMethodDataFilter = System.getProperty("jvmci.traceMethodDataFilter");
> 
> The only other direct usage of System.getProperty is:
> 
> hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotJVMCIRuntime.java
> 167:        if (Boolean.valueOf(System.getProperty("jvmci.printconfig"))) {
> 
> I think both of them should be using the same mechanism as introduced by this change.

I agree (assuming you mean the HotSpotJVMCIRuntime.getBooleanProperty mechanism).

There’s also:

hotspot/src/jdk.vm.ci/share/classes/jdk.vm.ci.inittimer/src/jdk/vm/ci/inittimer/InitTimer.java
70:    private static final boolean ENABLED = Boolean.getBoolean("jvmci.inittimer") || Boolean.getBoolean("jvmci.runtime.TimeInit");

But we will have to leave that as is given that HotSpotJVMCIRuntime is not visible from this code. We could also remove the (legacy) “jvmci.runtime.TimeInit” alias.

-Doug

> 
>> On Jan 4, 2016, at 12:47 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>> 
>>> 
>>> On Jan 4, 2016, at 12:31 PM, Doug Simon <doug.simon at oracle.com> wrote:
>>> 
>>>> 
>>>> On 04 Jan 2016, at 18:41, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>> 
>>>>> 
>>>>> On Jan 4, 2016, at 7:19 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>>> 
>>>>>> 
>>>>>> On Jan 4, 2016, at 7:16 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>>>> 
>>>>>>> 
>>>>>>> On Dec 22, 2015, at 4:50 AM, Doug Simon <doug.simon at oracle.com> wrote:
>>>>>>> 
>>>>>>> The effort of maintaining JVMCI across different JDK versions (including a potential backport to JDK7) is reduced by making JVMCI as small as possible. The support for command line options in JVMCI (based around the @Option annotation) is a good candidate for removal: 
>>>>>>> 
>>>>>>> 1. It’s almost entirely implemented on top of system properties and so can be made to work without VM support. 
>>>>>>> 2. JVMCI itself only currently uses 3 options which can be replaced with usage of sun.misc.VM.getSavedProperty(). The latter ensures application code can’t override JVMCI properties set on the command line.
>>>>>>> 
>>>>>>> This change removes the JVMCI command line option support.
>>>>>>> 
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8146001
>>>>>>> http://cr.openjdk.java.net/~dnsimon/8146001/
>>>>>> 
>>>>>> +    private static final boolean TrustFinalDefaultFields = HotSpotJVMCIRuntime.getBooleanProperty(TrustFinalDefaultFieldsProperty, true);
>>>>>> 
>>>>>> +    private static final boolean ImplicitStableValues = HotSpotJVMCIRuntime.getBooleanProperty("jvmci.ImplicitStableValues", true);
>>>>>> 
>>>>>> We should either use the jvmci. prefix or not.
>>>>> 
>>>>> Sorry, I was reading the patch wrong.  Of course both use the jvmci. prefix.
>>>> 
>>>> I think we should prefix the property name in getBooleanProperty:
>>>> 
>>>> +    public static boolean getBooleanProperty(String name, boolean def) {
>>>> +        String value = VM.getSavedProperty("jvmci." + name);
>>> 
>>> Ok, sounds reasonable.
>>> 
>>>> 
>>>> and I put UseProfilingInformation back:
>>>> 
>>>> diff -r 0fcfe4b07f7e src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java
>>>> --- a/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java	Tue Dec 29 18:30:51 2015 +0100
>>>> +++ b/src/jdk.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java	Mon Jan 04 07:40:46 2016 -1000
>>>> @@ -24,7 +24,6 @@ package jdk.vm.ci.hotspot;
>>>> 
>>>> import static jdk.vm.ci.hotspot.CompilerToVM.compilerToVM;
>>>> import static jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.runtime;
>>>> -import static jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod.Options.UseProfilingInformation;
>>>> import static jdk.vm.ci.hotspot.HotSpotVMConfig.config;
>>>> import static jdk.vm.ci.hotspot.UnsafeAccess.UNSAFE;
>>>> 
>>>> @@ -65,6 +64,11 @@ import jdk.vm.ci.meta.TriState;
>>>> final class HotSpotResolvedJavaMethodImpl extends HotSpotMethod implements HotSpotResolvedJavaMethod, HotSpotProxified, MetaspaceWrapperObject {
>>>> 
>>>>     /**
>>>> +     * Whether to use profiling information.
>>>> +     */
>>>> +    private static final boolean UseProfilingInformation = HotSpotJVMCIRuntime.getBooleanProperty("UseProfilingInformation", true);
>>>> +
>>>> +    /**
>>>>      * Reference to metaspace Method object.
>>>>      */
>>>>     private final long metaspaceMethod;
>>>> @@ -424,7 +428,7 @@ final class HotSpotResolvedJavaMethodImp
>>>>     public ProfilingInfo getProfilingInfo(boolean includeNormal, boolean includeOSR) {
>>>>         ProfilingInfo info;
>>>> 
>>>> -        if (UseProfilingInformation.getValue() && methodData == null) {
>>>> +        if (UseProfilingInformation && methodData == null) {
>>>>             long metaspaceMethodData = UNSAFE.getAddress(metaspaceMethod + config().methodDataOffset);
>>>>             if (metaspaceMethodData != 0) {
>>>>                 methodData = new HotSpotMethodData(metaspaceMethodData, this);
>>> 
>>> JVMCI should unconditionally return available profiling information. It's up to the compiler whether or not to use it. For example, this is now compilation local in Graal:
>>> 
>>> http://hg.openjdk.java.net/graal/graal-compiler/rev/f35e653aa876#l16.16
>> 
>> Oh, I missed that.  Yes, that works for us as well.  Thanks for pointing that out.
>> 
>>> 
>>> -Doug
>