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

[Christian Thalinger]

> On Jan 6, 2016, at 8:04 AM, Doug Simon <doug.simon at oracle.com> wrote:
> 
>> 
>> 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).

Yes.

> 
> 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”);

I’ve seen that one too.

> 
> 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.

Yes, let’s remove the legacy property.

https://bugs.openjdk.java.net/browse/JDK-8146820

> 
> -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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160111/9593fa5f/attachment-0001.html>