RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

Eric Caspole-2
Hi,
Please review this very small change to add more info to
+LogTouchedMethods including the touched count and whether the method
has loops. This extra info has been handy while doing experiments to
create the most efficient AOT library for quick startup.

http://cr.openjdk.java.net/~ecaspole/JDK-8186154/webrev/

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

Passed JPRT hotspot tests.

Thanks,
Eric
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

Vladimir Kozlov
Good.

Thanks,
Vladimir

On 8/11/17 12:10 PM, Eric Caspole wrote:

> Hi,
> Please review this very small change to add more info to
> +LogTouchedMethods including the touched count and whether the method
> has loops. This extra info has been handy while doing experiments to
> create the most efficient AOT library for quick startup.
>
> http://cr.openjdk.java.net/~ecaspole/JDK-8186154/webrev/
>
> https://bugs.openjdk.java.net/browse/JDK-8186154
>
> Passed JPRT hotspot tests.
>
> Thanks,
> Eric
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

Milan Mimica-2
In reply to this post by Eric Caspole-2
Hi

I believe each VM option should have a unique description:

 diagnostic(bool, PrintTouchedMethodsAtExit, false,                          \
           "Print all methods that have been ever touched in runtime")       \

      \+  diagnostic(bool, PrintTouchedMethodCount, false,
             \+          "Print all methods that have been ever
touched in runtime")       \+


pet, 11. kol 2017. u 21:11 Eric Caspole <[hidden email]> napisao
je:

> Hi,
> Please review this very small change to add more info to
> +LogTouchedMethods including the touched count and whether the method
> has loops. This extra info has been handy while doing experiments to
> create the most efficient AOT library for quick startup.
>
> http://cr.openjdk.java.net/~ecaspole/JDK-8186154/webrev/
>
> https://bugs.openjdk.java.net/browse/JDK-8186154
>
> Passed JPRT hotspot tests.
>
> Thanks,
> Eric
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

Ioi Lam
In reply to this post by Eric Caspole-2
Hi Eric,

I am not sure if _touch_count can accurately represent 'how often is
this method used'. In the interpreter, Method::log_touched is called
only once, when building the Method::method_counters()

#define GET_METHOD_COUNTERS(res)    \
   res = METHOD->method_counters();  \
   if (res == NULL) {                \
     CALL_VM(res = InterpreterRuntime::build_method_counters(THREAD,
METHOD), handle_exception); \
   }

In addition, whenever the compiler references a method (when compiling a
method, or when inlining a method into another, etc), it calls
Method::log_touched

ciMethod::ciMethod(const methodHandle& h_m, ciInstanceKlass* holder) :
   ciMetadata(h_m()),
   _holder(holder)
{
   assert(h_m() != NULL, "no null method");

   if (LogTouchedMethods) {
     h_m()->log_touched(Thread::current());
   }

So, a high count of _touch_count is more correlated to how often this
method is seen by the compiler.

Thanks

- Ioi



On 8/11/17 12:10 PM, Eric Caspole wrote:

> Hi,
> Please review this very small change to add more info to
> +LogTouchedMethods including the touched count and whether the method
> has loops. This extra info has been handy while doing experiments to
> create the most efficient AOT library for quick startup.
>
> http://cr.openjdk.java.net/~ecaspole/JDK-8186154/webrev/
>
> https://bugs.openjdk.java.net/browse/JDK-8186154
>
> Passed JPRT hotspot tests.
>
> Thanks,
> Eric

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

coleen.phillimore
In reply to this post by Eric Caspole-2

Hi,

I don't think you should add another option to print the count.  You
should do it unconditionally.  Since it's a diagnostic option, we don't
have to support parsers of the output.
I was going to type that you should use UL for this output instead, but
that's a bigger task.  It would be nice if the compiler options used UL
and didn't have these Print options anymore.  In any case, please don't
add another Print* option.

thanks,
Coleen

On 8/11/17 3:10 PM, Eric Caspole wrote:

> Hi,
> Please review this very small change to add more info to
> +LogTouchedMethods including the touched count and whether the method
> has loops. This extra info has been handy while doing experiments to
> create the most efficient AOT library for quick startup.
>
> http://cr.openjdk.java.net/~ecaspole/JDK-8186154/webrev/
>
> https://bugs.openjdk.java.net/browse/JDK-8186154
>
> Passed JPRT hotspot tests.
>
> Thanks,
> Eric

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

Eric Caspole-2
In reply to this post by Milan Mimica-2
Good catch.
Eric


On 08/12/2017 05:15 AM, Milan Mimica wrote:

> Hi
>
> I believe each VM option should have a unique description:
>
>   diagnostic(bool, PrintTouchedMethodsAtExit, false,                          \
>             "Print all methods that have been ever touched in runtime")       \
>
>        \+  diagnostic(bool, PrintTouchedMethodCount, false,
>               \+          "Print all methods that have been ever
> touched in runtime")       \+
>
>
> pet, 11. kol 2017. u 21:11 Eric Caspole <[hidden email]> napisao
> je:
>
>> Hi,
>> Please review this very small change to add more info to
>> +LogTouchedMethods including the touched count and whether the method
>> has loops. This extra info has been handy while doing experiments to
>> create the most efficient AOT library for quick startup.
>>
>> http://cr.openjdk.java.net/~ecaspole/JDK-8186154/webrev/
>>
>> https://bugs.openjdk.java.net/browse/JDK-8186154
>>
>> Passed JPRT hotspot tests.
>>
>> Thanks,
>> Eric
>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

Vladimir Kozlov
In reply to this post by coleen.phillimore
On 8/14/17 5:07 AM, [hidden email] wrote:
>
> Hi,
>
> I don't think you should add another option to print the count.  You
> should do it unconditionally.  Since it's a diagnostic option, we don't
> have to support parsers of the output.

Not true. Please, don't complicate PrintTouchedMethodsAtExit output
because AOT have to parse it ;) as Eric explained.

If you want an other output use -Xlog format.

Vladimir

> I was going to type that you should use UL for this output instead, but
> that's a bigger task.  It would be nice if the compiler options used UL
> and didn't have these Print options anymore.  In any case, please don't
> add another Print* option.
>
> thanks,
> Coleen
>
> On 8/11/17 3:10 PM, Eric Caspole wrote:
>> Hi,
>> Please review this very small change to add more info to
>> +LogTouchedMethods including the touched count and whether the method
>> has loops. This extra info has been handy while doing experiments to
>> create the most efficient AOT library for quick startup.
>>
>> http://cr.openjdk.java.net/~ecaspole/JDK-8186154/webrev/
>>
>> https://bugs.openjdk.java.net/browse/JDK-8186154
>>
>> Passed JPRT hotspot tests.
>>
>> Thanks,
>> Eric
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

coleen.phillimore


On 8/14/17 11:07 AM, Vladimir Kozlov wrote:

> On 8/14/17 5:07 AM, [hidden email] wrote:
>>
>> Hi,
>>
>> I don't think you should add another option to print the count. You
>> should do it unconditionally.  Since it's a diagnostic option, we
>> don't have to support parsers of the output.
>
> Not true. Please, don't complicate PrintTouchedMethodsAtExit output
> because AOT have to parse it ;) as Eric explained.

I didn't see that.  Can we also fix AOT to parse and/or ignore the
count?  I thought the purpose was for AOT to get the count to provide a
better list to precompile.

thanks,
Coleen

>
> If you want an other output use -Xlog format.
>
> Vladimir
>
>> I was going to type that you should use UL for this output instead,
>> but that's a bigger task.  It would be nice if the compiler options
>> used UL and didn't have these Print options anymore.  In any case,
>> please don't add another Print* option.
>>
>> thanks,
>> Coleen
>>
>> On 8/11/17 3:10 PM, Eric Caspole wrote:
>>> Hi,
>>> Please review this very small change to add more info to
>>> +LogTouchedMethods including the touched count and whether the
>>> method has loops. This extra info has been handy while doing
>>> experiments to create the most efficient AOT library for quick startup.
>>>
>>> http://cr.openjdk.java.net/~ecaspole/JDK-8186154/webrev/
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8186154
>>>
>>> Passed JPRT hotspot tests.
>>>
>>> Thanks,
>>> Eric
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS): JDK-8186154 : Extra info with +LogTouchedMethods

Eric Caspole-2
In reply to this post by Ioi Lam
Hi Ioi,
Yes this is not a cheap invocation counter. I think the count is useful
because it is related to how often the methods are considered for
inlining etc, so the higher counts are methods that affect the hot paths
in a way I admit I don't completely understand yet. That's why I want
this stat :)

Eric


On 08/12/2017 09:42 PM, Ioi Lam wrote:

> Hi Eric,
>
> I am not sure if _touch_count can accurately represent 'how often is
> this method used'. In the interpreter, Method::log_touched is called
> only once, when building the Method::method_counters()
>
> #define GET_METHOD_COUNTERS(res)    \
>    res = METHOD->method_counters();  \
>    if (res == NULL) {                \
>      CALL_VM(res = InterpreterRuntime::build_method_counters(THREAD,
> METHOD), handle_exception); \
>    }
>
> In addition, whenever the compiler references a method (when compiling a
> method, or when inlining a method into another, etc), it calls
> Method::log_touched
>
> ciMethod::ciMethod(const methodHandle& h_m, ciInstanceKlass* holder) :
>    ciMetadata(h_m()),
>    _holder(holder)
> {
>    assert(h_m() != NULL, "no null method");
>
>    if (LogTouchedMethods) {
>      h_m()->log_touched(Thread::current());
>    }
>
> So, a high count of _touch_count is more correlated to how often this
> method is seen by the compiler.
>
> Thanks
>
> - Ioi
>
>
>
> On 8/11/17 12:10 PM, Eric Caspole wrote:
>> Hi,
>> Please review this very small change to add more info to
>> +LogTouchedMethods including the touched count and whether the method
>> has loops. This extra info has been handy while doing experiments to
>> create the most efficient AOT library for quick startup.
>>
>> http://cr.openjdk.java.net/~ecaspole/JDK-8186154/webrev/
>>
>> https://bugs.openjdk.java.net/browse/JDK-8186154
>>
>> Passed JPRT hotspot tests.
>>
>> Thanks,
>> Eric
>
Loading...