Review Request: JMC-5893: Commercial features check removed in JDK 11

Klara Ward klara.ward at oracle.com
Wed Jun 20 12:19:00 UTC 2018


New webrev looks like it will work. Ship it.

The name for the class HotSpot23CommercialFeaturesService comes from
HotSpot version 23, which is what is included in JDK 7u4 where Flight
Recorder was first released.
The code could probably benefit from being rewritten to be easier to
understand, but IMHO that could be done in a new bug.

Regards,
Klara

On 2018-06-20 09:06, Sharath Ballal wrote:
>
> Hi Klara,
>
>  
>
> Ø  * Spelling error in FlightRecorderServiceV2.java, should be "Flight
> Recorder" instead of "Flighter Recorder"
> Corrected it.
>
> Ø  * (The class HotSpot23CommercialFeaturesService should probably be
> called something else, but that could be seen as out of scope for this
> fix.)
> I can open a bug for it.  But why should the name be changed ?  What
> is the significance of 23 in the filename ?
>
> Ø  *Maybe  JavaVersionSupport.JFR_NOT_COMMERCIAL field should point to
> a new "JDK_11_EA" instead.*
>
> I have changed JDK_11 to JDK_11_EA and JFR_NOT_COMMERCIAL to JDK_11_EA
> and passed earlyAccess flag as true.  With this the fix works for 11
> developer build and EA build.
>
>  
>
> I again tested 11 and 10 builds with +/-UnlockCommercialFeatures and
> +/-FlightRecorder flags both open and Oracle jdks.  I also ran the
> ‘verify’ tests and they are passing.
>
>  
>
> Updated Webrev :
> http://cr.openjdk.java.net/~sballal/JMC-5893/webrev.03/
> <http://cr.openjdk.java.net/%7Esballal/JMC-5893/webrev.03/>
>
>  
>
> Thanks,
>
> Sharath
>
>  
>
>  
>
> *From:*Klara Ward
> *Sent:* Tuesday, June 12, 2018 9:26 PM
> *To:* Sharath Ballal
> *Cc:* jmc-dev at openjdk.java.net; Erik Gahlin; Marcus Hirt
> *Subject:* Re: Review Request: JMC-5893: Commercial features check
> removed in JDK 11
>
>  
>
> Looks like the version 11-ea is not considered to be greater of equal
> than 11, which makes this fail right now.
> *Maybe  JavaVersionSupport.JFR_NOT_COMMERCIAL field should point to a
> new "JDK_11_EA" instead. *This will make the code behave incorrectly
> for  really early versions of JDK 11, but those are not very
> interesting anyway.
>
> (Changing the check in JavaVersion.isGreaterOrEqualThan to not
> includes earlyAccess seems like a more risky, and perhaps incorrect fix.)
>
>
> I only tested JDK 11 without any flags,
> any other configurations of JDK versions, and
> +/-UnlockCommercialFeatures and +/-FlightRecorder flags are untested
> by me.
>
> // Klara
>
<snip>



More information about the jmc-dev mailing list