Review request for JM-6544: Add support to parse Java internal version of Zulu JVM

[Florian David]

Thanks for your feedback, I understand the pain of having to maintain
different vendors version in the upstream project. As Henrik wrote, having
a long-term solution for this would involve a longer discussion.

Meanwhile, we can still fix this issue with the solution mentioned above
and having a correct error message reported to the user, providing a better
understanding than just a NPE.

Here is the updated patch:
JIRA: https://bugs.openjdk.java.net/browse/JMC-6544
Webrev: http://cr.openjdk.java.net/~fdavid/JMC-6544/webrev.1/

Thanks,
Florian



On Mon, Aug 5, 2019 at 6:44 PM Henrik Dafgård <hdafgard at gmail.com> wrote:

> Hi all,
>
> I think that this is a more involved discussion that should definitely be
> had. Originally we only built JMC to support known JDKs built by Oracle
> where we could be assumed to know enough details about the features of each
> version to depend on only version numbers to decide whether to check for
> certain behaviours in the rules. This is in e.g. the String Deduplication
> rule and the Code Cache rule, they're already depending on vendor, albeit
> implicitly.
>
> To fix this properly we could perhaps build some support in the core JMC
> APIs for checking Java Vendor and allowing downstream implementations to
> react to different vendors, rather than just to which JDK version is used.
> We would then have to replace all references to RulesToolkit.getJavaVersion
> used for determining runtime features with both checks for vendor and
> version.
>
> Regardless, we should still fix this issue and after looking through usages
> of the JavaVersion methods I've found that most of them check for a
> potential null return value and only two don't (and they both have implicit
> vendor dependencies). So the best fix is probably to add null checks to the
> FlightRecordingSupportRule:113 with a N/A result and the
> CodeCachePageUI:294.
>
>
> Cheers,
> Henrik Dafgård
>
>
> On Mon, 5 Aug 2019 at 10:46, Mario Torre <neugens at redhat.com> wrote:
>
> > On 05/08/2019 10:31, Florian David wrote:
> > > I have been told that the patch was removed from the previous mail.
> > > Please find a web review instead:
> > >
> > > JIRA: https://bugs.openjdk.java.net/browse/JMC-6544
> > > Webrev: http://cr.openjdk.java.net/~fdavid/JMC-6544/webrev.0/
> >
> > Hi Florian,
> >
> > To be honest, I already expressed concerns about this
> > FlightRecordingSupportRule, I don't think it's a good idea to make JMC
> > know every single java vendor out there in the world.
> >
> > I would rather prefer such patches are either internal to vendors and
> > not exposed in the upstream project, but more generally we should rework
> > this mechanisms in order to be vendor-agnostic rather than add more
> > special cases.
> >
> > Cheers,
> > Mario
> >
> > > On Sun, Aug 4, 2019 at 6:12 PM Florian David <
> > florian.david at datadoghq.com>
> > > wrote:
> > >
> > >> Hi,
> > >>
> > >>
> > >> Please find attached a patch to solve an error that arise when parsing
> > >> Zulu internal version [0]. I also added a test with a representative
> > set of
> > >> Java internal versions of the Zulu JVM, even if it's far from covering
> > >> every single case.
> > >>
> > >>
> > >> Regards,
> > >>
> > >> Florian
> > >>
> > >>
> > >> [0] https://bugs.openjdk.java.net/browse/JMC-6544
> > >>
> > >>
> > >>
> > >>
> >
> >
> > --
> > Mario Torre
> > Associate Manager, Software Engineering
> > Red Hat GmbH <https://www.redhat.com>
> > 9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898
> >
>