RFR: 8265129: Add intrinsic support for JVM.getClassId [v6]

[Denghui Dong]

On Wed, 19 May 2021 15:28:23 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Hi Vladimir,
>> 
>> Thanks for your comment.
>> 
>> Yes, the native implementation for `getClassIdNonIntrinsic`/`getClassId` is located in `jfrTraceId.cpp#L178` just as you said, more specifically, there are two path, one(JfrTraceId::load) for normal class and one(load_primitive) for primitive class (includeing void.class).
>> 
>> My pseudo-code(the comment of `LibraryCallKit::inline_native_classID`) is consistent with the implementation of these two paths.
>> 
>> And in the normal class implementation path, there are fast path and slow path(see JfrTraceIdLoadBarrier::load), only some comparison and shift operations are needed to obtain the class ID in the fast path, and that's where I think intrinsic can bring performance improvements, I saw about 20x improvement from my microbenchmark.
>> 
>> Judging from the current JFR implementation, there are already some events that need to rely on this API, such as `ExceptionThrownEvent` and `ErrorThrownEvent` use `thrownClass` to record the type of exception, and I also noticed that there is a new PR(https://github.com/openjdk/jdk/pull/4101) to add `FinalizerEvent` which include a field named `finalizedClass` to record the type information. Therefore, I have reason to believe that this API will be frequently used during the JFR activation process.
>> 
>> As far as the current implementation is concerned, it is indeed a bit complicated, I think some simplifications can be made, for example, only the fast path for the normal class is retained, and other paths are directly implemented by calling the native function. What do you think?
>> 
>> @egahlin @mgronlun
>> And I hope JFR's folks could give some suggestions on this PR:)  
>> 
>> Best,
>> Denghui
>
> Hi @D-D-H,
> 
> Can you show pseudo code for your fast/slow path suggestion?
> What is most frequent (performance critical) path is used?
> If it simplify code I am for it.
> 
> I am not sure about implementing it in C1 since in default mode methods will be compiled by C2. So for C1 to call native implementation could be fine and we can remove code there. Or you can file followup RFE to fix it in C1 later.

Hi @vnkozlov ,

Thank you for your reply, I am not sure if the pseudo-code I describe below is what you expect, if not, please feel free to point it out.

The native impletation of JVM.getClassId is `jfr_class_id`, which directly call `JfrTraceId::load` and the parameter `raw` is false.

And there are two paths (one for normal class, one for primitive class) in `JfrTraceId::load`, the pseudo-code is as follows:

(a) For the normal classes path:


  epoch = JfrTraceIdEpoch::_epoch_state ? 2 : 1
  if (oop->klass->trace_id & ((epoch << META_SHIFT) | epoch)) != epoch) {

        // here is the slow path, and only occur when this klass is first recorded
        // or the epoch of JFR shifts(this won't happen frequently)

        SET_USED_THIS_EPOCH
        enqueue klass (a function call)
        if (!signaled) // load_acquire, see JfrSignal::signal
          signaled = true // release_store
  }

  id = oop->klass->trace_id >> TRACE_ID_SHIFT

In short, the fast path will not enter the if branch, and only uses some simple instructions.


(b) For the primitive classes path:

   if oop->array_klass != null
     id = (oop->array_klass->trace_id >> TRACE_ID_SHIFT) + 1 // primitive class path
   else
     id = LAST_TYPE_ID + 1 // void class path, LAST_TYPE_ID is constant

   if (!signaled) // same as before
     signaled = true

This path is clear, there is no complex operation.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3470