[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8286490: JvmtiEventControllerPrivate::set_event_callbacks CLEARING_MASK computation is inco
From:       Serguei Spitsyn <sspitsyn () openjdk ! java ! net>
Date:       2022-05-25 0:50:56
Message-ID: t7s3zhFb2eeIozaETKxjgu5ObJfhTF7BZfo-u_ECGJM=.ba23cd6f-e7cd-45a8-b494-1d7a292dc459 () github ! com
[Download RAW message or body]

On Wed, 25 May 2022 00:20:48 GMT, Serguei Spitsyn <sspitsyn@openjdk.org> wrote:

> > It was a typo when the original CLEARING_MASK was initially introduced:
> > 
> > 
> > // Mask to clear normal event bits.
> > const jlong CLEARING_MASK = (1L >> (TOTAL_MIN_EVENT_TYPE_VAL - \
> > TOTAL_MIN_EVENT_TYPE_VAL)) - 1L; // Avoid cleaning extension event bits.
> > jlong enabled_bits = CLEARING_MASK & \
> > env->env_event_enable()->_event_callback_enabled.get_bits(); 
> > ``` 
> > 
> > The first TOTAL_MIN_EVENT_TYPE_VAL has to be JVMTI_MIN_EVENT_TYPE_VAL as below:
> > `  const jlong CLEARING_MASK = (1L >> (JVMTI_MIN_EVENT_TYPE_VAL - \
> > TOTAL_MIN_EVENT_TYPE_VAL)) - 1L;` 
> > Let me provide some background to understand how this mask is used.
> > 
> > Normal JVMTI event types are numbered from 50 to 88. There are two constants:
> > 
> > JVMTI_MIN_EVENT_TYPE_VAL = 50
> > JVMTI_MAX_EVENT_TYPE_VAL = 88
> > 
> > The extension event types are numbered from 47 to 49. There are also two \
> > constants: 
> > EXT_MIN_EVENT_TYPE_VAL = 47
> > EXT_MAX_EVENT_TYPE_VAL = 49
> > 
> > 
> > There are also two additional constants:
> > 
> > TOTAL_MIN_EVENT_TYPE_VAL = 47
> > TOTAL_MAX_EVENT_TYPE_VAL = 88
> > 
> > 
> > The `enabled_bits` are shifted left by 47, so that the 0-bit in `enabled_bits` is \
> > for first extension event type. And the first normal JVMTI event type is numbered \
> > 3 in the `enabled_bits` bit mask. The `CLEARING_MASK` is used to clear only \
> > normal JVMTI event types but keep the extension event bits (0-2) unchanged. 
> > Testing:
> > I'll run all JVMTI tests including nsk.jvmti tests and serviceability/jvmti tests \
> > including those added in Loom for virtual thread coverage.
> 
> Serguei Spitsyn has updated the pull request incrementally with one additional \
> commit since the last revision: 
> get rid of CLEARING_MASK in JVMTI set_event_callbacks

Thank you for review and good comments, Alex!

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

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


[prev in list] [next in list] [prev in thread] [next in thread]