RFR: JDK-8180487: HotSpotResolvedJavaMethod#setNotInlineable() should be renamed to represent actual behavior

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: JDK-8180487: HotSpotResolvedJavaMethod#setNotInlineable() should be renamed to represent actual behavior

Yasumasa Suenaga-4
Hi all,

This review request is related to [1].

JBS: https://bugs.openjdk.java.net/browse/JDK-8180487
Patch: http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.00/

Could you review it?

I cannot access JPRT.
So I need a sponsor.


Thanks,

Yasumasa


[1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-May/026218.html
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180487: HotSpotResolvedJavaMethod#setNotInlineable() should be renamed to represent actual behavior

Doug Simon @ Oracle
The code changes look good. However, the javadoc still describes the functionality as a query:

     /**
      * Determines if {@code method} should not be inlined or compiled.
      */

where as it's really a setter. That is, the comment should be:

     /**
      * Sets flags on {@code method} indicating that it should never be inlined or compiled by the VM.
      */

-Doug

> On 18 May 2017, at 05:18, Yasumasa Suenaga <[hidden email]> wrote:
>
> Hi all,
>
> This review request is related to [1].
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8180487
> Patch: http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.00/
>
> Could you review it?
>
> I cannot access JPRT.
> So I need a sponsor.
>
>
> Thanks,
>
> Yasumasa
>
>
> [1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-May/026218.html

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180487: HotSpotResolvedJavaMethod#setNotInlineable() should be renamed to represent actual behavior

Yasumasa Suenaga-4
Hi Doug,

Thank you for your comment.

I uploaded new webrev.
Could you check again?

   http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.01/


Yasumasa


On 2017/05/18 18:11, Doug Simon wrote:

> The code changes look good. However, the javadoc still describes the functionality as a query:
>
>      /**
>       * Determines if {@code method} should not be inlined or compiled.
>       */
>
> where as it's really a setter. That is, the comment should be:
>
>      /**
>       * Sets flags on {@code method} indicating that it should never be inlined or compiled by the VM.
>       */
>
> -Doug
>
>> On 18 May 2017, at 05:18, Yasumasa Suenaga <[hidden email]> wrote:
>>
>> Hi all,
>>
>> This review request is related to [1].
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8180487
>> Patch: http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.00/
>>
>> Could you review it?
>>
>> I cannot access JPRT.
>> So I need a sponsor.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> [1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-May/026218.html
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180487: HotSpotResolvedJavaMethod#setNotInlineable() should be renamed to represent actual behavior

Doug Simon @ Oracle
Thanks for the update - the changes look good to me.

-Doug

> On 18 May 2017, at 14:12, Yasumasa Suenaga <[hidden email]> wrote:
>
> Hi Doug,
>
> Thank you for your comment.
>
> I uploaded new webrev.
> Could you check again?
>
>  http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.01/
>
>
> Yasumasa
>
>
> On 2017/05/18 18:11, Doug Simon wrote:
>> The code changes look good. However, the javadoc still describes the functionality as a query:
>>
>>     /**
>>      * Determines if {@code method} should not be inlined or compiled.
>>      */
>>
>> where as it's really a setter. That is, the comment should be:
>>
>>     /**
>>      * Sets flags on {@code method} indicating that it should never be inlined or compiled by the VM.
>>      */
>>
>> -Doug
>>
>>> On 18 May 2017, at 05:18, Yasumasa Suenaga <[hidden email]> wrote:
>>>
>>> Hi all,
>>>
>>> This review request is related to [1].
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8180487
>>> Patch: http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.00/
>>>
>>> Could you review it?
>>>
>>> I cannot access JPRT.
>>> So I need a sponsor.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-May/026218.html
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180487: HotSpotResolvedJavaMethod#setNotInlineable() should be renamed to represent actual behavior

Yasumasa Suenaga-4
Thanks Doug!

BTW, could you be a sponsor?


Yasumasa


On 2017/05/18 21:27, Doug Simon wrote:

> Thanks for the update - the changes look good to me.
>
> -Doug
>
>> On 18 May 2017, at 14:12, Yasumasa Suenaga <[hidden email]> wrote:
>>
>> Hi Doug,
>>
>> Thank you for your comment.
>>
>> I uploaded new webrev.
>> Could you check again?
>>
>>  http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.01/
>>
>>
>> Yasumasa
>>
>>
>> On 2017/05/18 18:11, Doug Simon wrote:
>>> The code changes look good. However, the javadoc still describes the functionality as a query:
>>>
>>>     /**
>>>      * Determines if {@code method} should not be inlined or compiled.
>>>      */
>>>
>>> where as it's really a setter. That is, the comment should be:
>>>
>>>     /**
>>>      * Sets flags on {@code method} indicating that it should never be inlined or compiled by the VM.
>>>      */
>>>
>>> -Doug
>>>
>>>> On 18 May 2017, at 05:18, Yasumasa Suenaga <[hidden email]> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> This review request is related to [1].
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8180487
>>>> Patch: http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.00/
>>>>
>>>> Could you review it?
>>>>
>>>> I cannot access JPRT.
>>>> So I need a sponsor.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-May/026218.html
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180487: HotSpotResolvedJavaMethod#setNotInlineable() should be renamed to represent actual behavior

Doug Simon @ Oracle

> On 18 May 2017, at 14:28, Yasumasa Suenaga <[hidden email]> wrote:
>
> Thanks Doug!
>
> BTW, could you be a sponsor?


I'd prefer it if one of the compiler devs could do it. Vladimir, can you take care of this?

-Doug

> On 2017/05/18 21:27, Doug Simon wrote:
>> Thanks for the update - the changes look good to me.
>>
>> -Doug
>>
>>> On 18 May 2017, at 14:12, Yasumasa Suenaga <[hidden email]> wrote:
>>>
>>> Hi Doug,
>>>
>>> Thank you for your comment.
>>>
>>> I uploaded new webrev.
>>> Could you check again?
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.01/
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> On 2017/05/18 18:11, Doug Simon wrote:
>>>> The code changes look good. However, the javadoc still describes the functionality as a query:
>>>>
>>>>    /**
>>>>     * Determines if {@code method} should not be inlined or compiled.
>>>>     */
>>>>
>>>> where as it's really a setter. That is, the comment should be:
>>>>
>>>>    /**
>>>>     * Sets flags on {@code method} indicating that it should never be inlined or compiled by the VM.
>>>>     */
>>>>
>>>> -Doug
>>>>
>>>>> On 18 May 2017, at 05:18, Yasumasa Suenaga <[hidden email]> wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> This review request is related to [1].
>>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8180487
>>>>> Patch: http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.00/
>>>>>
>>>>> Could you review it?
>>>>>
>>>>> I cannot access JPRT.
>>>>> So I need a sponsor.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-May/026218.html
>>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180487: HotSpotResolvedJavaMethod#setNotInlineable() should be renamed to represent actual behavior

Vladimir Kozlov
In JPRT queue.

Vladimir

On 5/18/17 5:41 AM, Doug Simon wrote:

>
>> On 18 May 2017, at 14:28, Yasumasa Suenaga <[hidden email]> wrote:
>>
>> Thanks Doug!
>>
>> BTW, could you be a sponsor?
>
>
> I'd prefer it if one of the compiler devs could do it. Vladimir, can you take care of this?
>
> -Doug
>
>> On 2017/05/18 21:27, Doug Simon wrote:
>>> Thanks for the update - the changes look good to me.
>>>
>>> -Doug
>>>
>>>> On 18 May 2017, at 14:12, Yasumasa Suenaga <[hidden email]> wrote:
>>>>
>>>> Hi Doug,
>>>>
>>>> Thank you for your comment.
>>>>
>>>> I uploaded new webrev.
>>>> Could you check again?
>>>>
>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.01/
>>>>
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2017/05/18 18:11, Doug Simon wrote:
>>>>> The code changes look good. However, the javadoc still describes the functionality as a query:
>>>>>
>>>>>    /**
>>>>>     * Determines if {@code method} should not be inlined or compiled.
>>>>>     */
>>>>>
>>>>> where as it's really a setter. That is, the comment should be:
>>>>>
>>>>>    /**
>>>>>     * Sets flags on {@code method} indicating that it should never be inlined or compiled by the VM.
>>>>>     */
>>>>>
>>>>> -Doug
>>>>>
>>>>>> On 18 May 2017, at 05:18, Yasumasa Suenaga <[hidden email]> wrote:
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> This review request is related to [1].
>>>>>>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8180487
>>>>>> Patch: http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.00/
>>>>>>
>>>>>> Could you review it?
>>>>>>
>>>>>> I cannot access JPRT.
>>>>>> So I need a sponsor.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-May/026218.html
>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: JDK-8180487: HotSpotResolvedJavaMethod#setNotInlineable() should be renamed to represent actual behavior

Ekaterina Pavlova
Hi Vladimir,

this change broke
 src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotGraalMBeanTest.java which is ported from Graal.

The proper place would be to fix this test in Graal, but Graal doesn't use jdk10.
We can also fix this test as part of Graal updates but I am not sure this is good solution.

Any other thoughts on this?

thanks,
-katya


On 5/18/17 4:39 PM, Vladimir Kozlov wrote:

> In JPRT queue.
>
> Vladimir
>
> On 5/18/17 5:41 AM, Doug Simon wrote:
>>
>>> On 18 May 2017, at 14:28, Yasumasa Suenaga <[hidden email]> wrote:
>>>
>>> Thanks Doug!
>>>
>>> BTW, could you be a sponsor?
>>
>>
>> I'd prefer it if one of the compiler devs could do it. Vladimir, can you take care of this?
>>
>> -Doug
>>
>>> On 2017/05/18 21:27, Doug Simon wrote:
>>>> Thanks for the update - the changes look good to me.
>>>>
>>>> -Doug
>>>>
>>>>> On 18 May 2017, at 14:12, Yasumasa Suenaga <[hidden email]> wrote:
>>>>>
>>>>> Hi Doug,
>>>>>
>>>>> Thank you for your comment.
>>>>>
>>>>> I uploaded new webrev.
>>>>> Could you check again?
>>>>>
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.01/
>>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2017/05/18 18:11, Doug Simon wrote:
>>>>>> The code changes look good. However, the javadoc still describes the functionality as a query:
>>>>>>
>>>>>>    /**
>>>>>>     * Determines if {@code method} should not be inlined or compiled.
>>>>>>     */
>>>>>>
>>>>>> where as it's really a setter. That is, the comment should be:
>>>>>>
>>>>>>    /**
>>>>>>     * Sets flags on {@code method} indicating that it should never be inlined or compiled by the VM.
>>>>>>     */
>>>>>>
>>>>>> -Doug
>>>>>>
>>>>>>> On 18 May 2017, at 05:18, Yasumasa Suenaga <[hidden email]> wrote:
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> This review request is related to [1].
>>>>>>>
>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8180487
>>>>>>> Patch: http://cr.openjdk.java.net/~ysuenaga/JDK-8180487/webrev.00/
>>>>>>>
>>>>>>> Could you review it?
>>>>>>>
>>>>>>> I cannot access JPRT.
>>>>>>> So I need a sponsor.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-May/026218.html
>>>>>>
>>>>
>>