@HotSpotIntrinsicCandidate and native prefixes

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

@HotSpotIntrinsicCandidate and native prefixes

Michael Rasmussen
Hi

If you set a native prefix using SetNativeMethodPrefix, in order to wrap
native methods. If those methods are annotated with @HotSpotIntrinsicCandidate
you get a warning when running.

For instance for Thread::isInterrupted:
Compiler intrinsic is defined for method
[java.lang.Thread.isInterrupted(Z)Z], but the method is not annotated with
@HotSpotIntrinsicCandidate. Method will not be inlined.
Method [java.lang.Thread.$prefix$isInterrupted(Z)Z] is annotated with
@HotSpotIntrinsicCandidate, but no compiler intrinsic is defined for the
method.

Shouldn't the native prefix be taken into account for this?

Kind regards

Michael Rasmussen
JRebel, ZeroTurnaround
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: @HotSpotIntrinsicCandidate and native prefixes

serguei.spitsyn@oracle.com
I've added the seviceability-dev mailing list as the
SetNativeMethodPrefix() is in the JVM TI interface.

Thanks,
Serguei


On 3/14/17 06:24, Michael Rasmussen wrote:

> Hi
>
> If you set a native prefix using SetNativeMethodPrefix, in order to wrap
> native methods. If those methods are annotated with @HotSpotIntrinsicCandidate
> you get a warning when running.
>
> For instance for Thread::isInterrupted:
> Compiler intrinsic is defined for method
> [java.lang.Thread.isInterrupted(Z)Z], but the method is not annotated with
> @HotSpotIntrinsicCandidate. Method will not be inlined.
> Method [java.lang.Thread.$prefix$isInterrupted(Z)Z] is annotated with
> @HotSpotIntrinsicCandidate, but no compiler intrinsic is defined for the
> method.
>
> Shouldn't the native prefix be taken into account for this?
>
> Kind regards
>
> Michael Rasmussen
> JRebel, ZeroTurnaround

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

Re: @HotSpotIntrinsicCandidate and native prefixes

Vladimir Ivanov
Nice catch, Michael! @HotSpotIntrinsicCandidate starts to pay off :-)

So, if a native method w/ an intrinsic is instrumented, it won't be
intrinsified anymore. Looking into the code, it looks like it worked
that way from the very beginning.

Filed an RFE:
   https://bugs.openjdk.java.net/browse/JDK-8176771

Do you have any problems when the JVM issues such warnings?

Best regards,
Vladimir Ivanov

> On 3/14/17 06:24, Michael Rasmussen wrote:
>> Hi
>>
>> If you set a native prefix using SetNativeMethodPrefix, in order to wrap
>> native methods. If those methods are annotated with
>> @HotSpotIntrinsicCandidate
>> you get a warning when running.
>>
>> For instance for Thread::isInterrupted:
>> Compiler intrinsic is defined for method
>> [java.lang.Thread.isInterrupted(Z)Z], but the method is not annotated
>> with
>> @HotSpotIntrinsicCandidate. Method will not be inlined.
>> Method [java.lang.Thread.$prefix$isInterrupted(Z)Z] is annotated with
>> @HotSpotIntrinsicCandidate, but no compiler intrinsic is defined for the
>> method.
>>
>> Shouldn't the native prefix be taken into account for this?
>>
>> Kind regards
>>
>> Michael Rasmussen
>> JRebel, ZeroTurnaround
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: @HotSpotIntrinsicCandidate and native prefixes

Michael Rasmussen
On 14 March 2017 at 20:30, Vladimir Ivanov <[hidden email]> wrote:
> Do you have any problems when the JVM issues such warnings?

We only just recently resumed our integration with JDK9 (was waiting
for the API (especially jigsaw) to stabilize), so I don't yet know the
full extend of these warnings, or if they will cause some actual
problems as well.

The warnings seems more of a nuisance than a problem, as we do add a
native-prefix to a lot of native methods, there are bounds to be some
hotspot intrinsic among them.
But for now, I'll just add it to the list of warnings that can be
ignored, most noticeably  the "JavaLaunchHelper is implemented in both
... " warning on macOS, when launching with an agent.

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

Re: @HotSpotIntrinsicCandidate and native prefixes

Vladimir Ivanov
Just in case it will cause problems for you, there's a workaround: you
can prune @HotSpotIntrinsicCandidate during instrumentation.

Best regards,
Vladimir Ivanov

On 3/14/17 9:45 PM, Michael Rasmussen wrote:

> On 14 March 2017 at 20:30, Vladimir Ivanov <[hidden email]> wrote:
>> Do you have any problems when the JVM issues such warnings?
>
> We only just recently resumed our integration with JDK9 (was waiting
> for the API (especially jigsaw) to stabilize), so I don't yet know the
> full extend of these warnings, or if they will cause some actual
> problems as well.
>
> The warnings seems more of a nuisance than a problem, as we do add a
> native-prefix to a lot of native methods, there are bounds to be some
> hotspot intrinsic among them.
> But for now, I'll just add it to the list of warnings that can be
> ignored, most noticeably  the "JavaLaunchHelper is implemented in both
> ... " warning on macOS, when launching with an agent.
>
> /Michael
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: @HotSpotIntrinsicCandidate and native prefixes

Michael Rasmussen
On 14 March 2017 at 20:48, Vladimir Ivanov <[hidden email]> wrote:
> Just in case it will cause problems for you, there's a workaround: you can
> prune @HotSpotIntrinsicCandidate during instrumentation.

Wouldn't that just cause the 2nd warning to disappear, but the first
one would remain?

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

Re: @HotSpotIntrinsicCandidate and native prefixes

Vladimir Ivanov
>> Just in case it will cause problems for you, there's a workaround: you can
>> prune @HotSpotIntrinsicCandidate during instrumentation.
>
> Wouldn't that just cause the 2nd warning to disappear, but the first
> one would remain?

Yes, you are right. Then the only option is:
   -XX:+UnlockDiagnosticVMOptions -XX:-CheckIntrinsics

Best regards,
Vladimir Ivanov
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: @HotSpotIntrinsicCandidate and native prefixes

Michael Rasmussen
On Mar 14, 2017 23:03, "Vladimir Ivanov" <[hidden email]>
wrote:

Yes, you are right. Then the only option is:
  -XX:+UnlockDiagnosticVMOptions -XX:-CheckIntrinsics

Best regards,
Vladimir Ivanov


Thinking about this a bit further, are these messages going to be enabled
by default when jdk9 is released?

These kind of messages are clearly only for the benefit of hotspot
developers, and has zero value to end users, who wouldn't know anything
about this annotation or the implications of it.

For the average user, heck you have to be a pretty advanced user to even
know what intrinsic means, getting warnings like this, I can only think it
would do a lot more harm than good, thinking that their installation is
broken or something.

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

Re: @HotSpotIntrinsicCandidate and native prefixes

Volker Simonis
On Thu, Mar 23, 2017 at 10:30 AM, Michael Rasmussen
<[hidden email]> wrote:

> On Mar 14, 2017 23:03, "Vladimir Ivanov" <[hidden email]>
> wrote:
>
> Yes, you are right. Then the only option is:
>   -XX:+UnlockDiagnosticVMOptions -XX:-CheckIntrinsics
>
> Best regards,
> Vladimir Ivanov
>
>
> Thinking about this a bit further, are these messages going to be enabled
> by default when jdk9 is released?
>
> These kind of messages are clearly only for the benefit of hotspot
> developers, and has zero value to end users, who wouldn't know anything
> about this annotation or the implications of it.
>
> For the average user, heck you have to be a pretty advanced user to even
> know what intrinsic means, getting warnings like this, I can only think it
> would do a lot more harm than good, thinking that their installation is
> broken or something.
>

I think this issue is somehow a counterpart of and related to
"JDK-8013579: Intrinsics for Java methods don't take class
redefinition into account" :)

So if you transform a non-native method which is subject to
intrinsification you should be aware that your transformations will be
lost, once the method will be intrinsified!

That said, I agree that the warning message is hard to understand for
the average Java developer but I think it has a value. As Vladimir
pointed out, if you wrap a native method, which would be intrinisfied
otherwise, through class transformation / SetNativeMethodPrefix, that
method will not be intrinsified any more. So if you do the
transformation for profiling for example, you'd profile a different
thing.

I actually somehow missed "8131326: Enable CheckIntrinsics in all
types of builds" which enabled CheckIntrinsics in product builds by
default (before it was only enabled in debug builds). Not sure what
was the rational behind this change (CC'ed Zoltan who did the change)
but I'm pretty sure that the effects of SetNativeMethodPrefix haven't
been taken into account when that change was made. In the end it paid
off, otherwise you wouldn't have found this issue :)

Regards,
Volker

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

Re: @HotSpotIntrinsicCandidate and native prefixes

Zoltán Majó
Hi Volker,


On 03/23/2017 12:43 PM, Volker Simonis wrote:

> [...]
>
>
> I actually somehow missed "8131326: Enable CheckIntrinsics in all
> types of builds" which enabled CheckIntrinsics in product builds by
> default (before it was only enabled in debug builds). Not sure what
> was the rational behind this change (CC'ed Zoltan who did the change)
> but I'm pretty sure that the effects of SetNativeMethodPrefix haven't
> been taken into account when that change was made. In the end it paid
> off, otherwise you wouldn't have found this issue :)

I mentioned the following in the RFR for 8131326:

"Problem: The CheckIntrinsics flag added by JDK-8076112 is currently
enabled only in debug builds. As a result, users of product builds might
easier oversee potential mismatches between VM-level and classfile-level
intrinsics."

http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2015-July/018425.html

I don't recall any other reason for doing this change, although somebody
might have asked me in a private message to do it (maybe with
more/different justification).

Best regards,


Zoltan

>
> Regards,
> Volker
>
>> /Michael

Loading...