RFR(S): 8241101: [s390] jtreg test failure after JDK-8238696: not conformant features string
Doerr, Martin
martin.doerr at sap.com
Wed Apr 1 13:42:58 UTC 2020
Hi Lutz,
ok. We will probably need the next update soon for z14.
Then, I think we should also remove the very old systems on which the JVM can't start at all because we already build with -march=z10 (which we may want to upgrade to z196).
In addition to that, "vectorinstr" also depends on the OS which is not reflected in the current version.
Best regards,
Martin
> -----Original Message-----
> From: Schmidt, Lutz <lutz.schmidt at sap.com>
> Sent: Mittwoch, 1. April 2020 14:50
> To: Doerr, Martin <martin.doerr at sap.com>; Langer, Christoph
> <christoph.langer at sap.com>; Thomas Stüfe <thomas.stuefe at gmail.com>
> Cc: hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(S): 8241101: [s390] jtreg test failure after JDK-8238696: not
> conformant features string
>
> Hi Martin,
>
> thanks for admitting the fix to be pushed.
>
> As I stated initially, I'm in favor of fixing root causes. In this particular case,
> however, I prefer to cure the symptoms only. I just don't have the time at
> the moment to dig deeper, understand all other tests calling
> CPUInfo.hasFeature(), and iterate until all tests complete OK. I will keep this
> in mind, though.
>
> Thanks,
> Lutz
>
> On 01.04.20, 12:04, "Doerr, Martin" <martin.doerr at sap.com> wrote:
>
> Hi Lutz,
>
> I'm ok with making the features lower case (even though it is not my
> preferred solution).
>
> But this test code deserves improvement! Maybe other people will run
> into the same issue and waste a lot of time figuring out why capital letters in
> feature names don't work. (Not necessarily part of your fix, but if you don't
> fix it who will?)
>
> Best regards,
> Martin
>
>
> > -----Original Message-----
> > From: Schmidt, Lutz <lutz.schmidt at sap.com>
> > Sent: Mittwoch, 1. April 2020 11:22
> > To: Langer, Christoph <christoph.langer at sap.com>; Doerr, Martin
> > <martin.doerr at sap.com>; Thomas Stüfe <thomas.stuefe at gmail.com>
> > Cc: hotspot-runtime-dev at openjdk.java.net
> > Subject: Re: RFR(S): 8241101: [s390] jtreg test failure after JDK-8238696:
> not
> > conformant features string
> >
> > Hi Christoph,
> > thank you for the review!
> >
> > @Martin Do you still have concerns/objections?
> >
> > Thanks,
> > Lutz
> >
> > On 01.04.20, 08:18, "Langer, Christoph" <christoph.langer at sap.com>
> wrote:
> >
> > Hi Lutz,
> >
> > ok, after having another, closer, look to the features string and its
> usage in
> > hotspot, I can confirm that it is only used in hs_err files and by the
> whitebox
> > API. Furthermore, I see that features happen to be lowercase on all
> > platforms. So, 390 was a bit out of order and it probably makes sense to
> > follow the other platforms. That means your fix is good ��
> >
> > Actually, I started to think that one should create a jtreg test to check
> > whether all features are lowercase, given that they are queried with
> > features.contains(feature.toLowerCase()). But I figured that that's what
> > CPUInfoTest implicitly does ��
> >
> > Best regards
> > Christoph
> >
> > > -----Original Message-----
> > > From: Schmidt, Lutz <lutz.schmidt at sap.com>
> > > Sent: Dienstag, 31. März 2020 22:14
> > > To: Langer, Christoph <christoph.langer at sap.com>; Doerr, Martin
> > > <martin.doerr at sap.com>; Thomas Stüfe
> <thomas.stuefe at gmail.com>
> > > Cc: hotspot-runtime-dev at openjdk.java.net
> > > Subject: Re: RFR(S): 8241101: [s390] jtreg test failure after JDK-
> 8238696:
> > not
> > > conformant features string
> > >
> > > Hi Christoph,
> > >
> > > I'm sorry but I strongly disagree. This feature string is solely used as
> > > supplemental information in the hs_err* file. At least I couldn't find
> any
> > > other usage. And for that use case, it simply doesn't matter if the
> string is
> > all
> > > lowercase.
> > >
> > > Best,
> > > Lutz
> > >
> > >
> > > On 31.03.20, 21:23, "Langer, Christoph" <christoph.langer at sap.com>
> > wrote:
> > >
> > > Hi Lutz,
> > >
> > > I also agree with Martin's idea of fixing this. I think it's better than
> > changing
> > > existing feature strings just for the sake of quiescing a test.
> > >
> > > I would even suggest to have a second Array of features in CPUInfo
> > that
> > > contains all values in lowercase to be queried by
> CPUInfo::hasFeature.
> > We
> > > should give it a try.
> > >
> > > Best regards
> > > Christoph
> > >
> > > > -----Original Message-----
> > > > From: hotspot-runtime-dev <hotspot-runtime-dev-
> > > > bounces at openjdk.java.net> On Behalf Of Schmidt, Lutz
> > > > Sent: Dienstag, 31. März 2020 15:59
> > > > To: Doerr, Martin <martin.doerr at sap.com>; Thomas Stüfe
> > > > <thomas.stuefe at gmail.com>
> > > > Cc: hotspot-runtime-dev at openjdk.java.net
> > > > Subject: [CAUTION] Re: RFR(S): 8241101: [s390] jtreg test failure
> after
> > > JDK-
> > > > 8238696: not conformant features string
> > > >
> > > > Hi Martin,
> > > > your extended check would heal the issue we saw with the
> original
> > > feature
> > > > string on s390. So far, so good. I can’t judge if other tests can live
> with
> > > that.
> > > > Regards,
> > > > Lutz
> > > >
> > > > From: "Doerr, Martin (martin.doerr at sap.com)"
> > > <martin.doerr at sap.com>
> > > > Date: Tuesday, 31. March 2020 at 15:54
> > > > To: Thomas Stüfe <thomas.stuefe at gmail.com>, Lutz Schmidt
> > > > <lutz.schmidt at sap.com>
> > > > Cc: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>, "hotspot-
> > runtime-
> > > > dev at openjdk.java.net" <hotspot-runtime-
> dev at openjdk.java.net>
> > > > Subject: RE: RFR(S): 8241101: [s390] jtreg test failure after JDK-
> > 8238696:
> > > not
> > > > conformant features string
> > > >
> > > > But what about
> > > > features.contains(feature) ||
> > features.contains(feature.toLowerCase())
> > > > ?
> > > >
> > > > Is it still problematic if we just support both, lower case and exact
> > match?
> > > >
> > > > Best regards,
> > > > Martin
> > > >
> > > >
> > > > From: Thomas Stüfe <thomas.stuefe at gmail.com>
> > > > Sent: Dienstag, 31. März 2020 15:51
> > > > To: Schmidt, Lutz <lutz.schmidt at sap.com>
> > > > Cc: Doerr, Martin <martin.doerr at sap.com>; Vladimir Ivanov
> > > > <vladimir.x.ivanov at oracle.com>; hotspot-runtime-
> > > dev at openjdk.java.net
> > > > Subject: Re: RFR(S): 8241101: [s390] jtreg test failure after JDK-
> > 8238696:
> > > not
> > > > conformant features string
> > > >
> > > > Okay. The change looks good to me.
> > > >
> > > > Cheers, Thomas
> > > >
> > > > On Tue, Mar 31, 2020 at 3:38 PM Schmidt, Lutz
> > > > <lutz.schmidt at sap.com<mailto:lutz.schmidt at sap.com>> wrote:
> > > > Thomas, Martin,
> > > >
> > > > thank you for looking into this.
> > > >
> > > > Generally, I'm very much in favor of the "fix the root cause"
> > approach. In
> > > this
> > > > particular case, however, I'm afraid of side effects and excessive
> > effort.
> > > > CPUInfo.hasFeature() is called in quite a few places, see the
> attached
> > > text
> > > > file for reference.
> > > >
> > > > My proposed change, on the other hand, is very constrained in
> > scope:
> > > just
> > > > s390, just a few string edits. The risk to break anything is
> extremely
> > low.
> > > > Therefore, I would like to stick with the proposed change.
> > > >
> > > > Thanks,
> > > > Lutz
> > > >
> > > >
> > > > On 31.03.20, 11:33, "Doerr, Martin"
> > > > <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
> wrote:
> > > >
> > > > Hi Thomas,
> > > >
> > > > I agree with that.
> > > >
> > > > test/lib/sun/hotspot/cpuinfo/CPUInfo.java uses
> > > > return features.contains(feature.toLowerCase());
> > > > in "hasFeature" which is not nicely designed.
> > > >
> > > > If it is required to lower the case (for some reason), I think it
> should
> > > better
> > > > use
> > > > features.contains(feature) ||
> > > features.contains(feature.toLowerCase())
> > > >
> > > > Or even better if we don't need toLowerCase at all and fix the
> > strings.
> > > >
> > > > Anyway, the 2nd part of the change (SHA, AES, ...) looks good
> to
> > me.
> > > >
> > > > Best regards,
> > > > Martin
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: hotspot-runtime-dev <hotspot-runtime-dev-
> > > > >
> > bounces at openjdk.java.net<mailto:bounces at openjdk.java.net>>
> > > On
> > > > Behalf Of Thomas Stüfe
> > > > > Sent: Dienstag, 31. März 2020 10:01
> > > > > To: Schmidt, Lutz
> > > > <lutz.schmidt at sap.com<mailto:lutz.schmidt at sap.com>>
> > > > > Cc: hotspot-runtime-dev at openjdk.java.net<mailto:hotspot-
> > > runtime-
> > > > dev at openjdk.java.net>
> > > > > Subject: Re: RFR(S): 8241101: [s390] jtreg test failure after
> JDK-
> > > 8238696:
> > > > not
> > > > > conformant features string
> > > > >
> > > > > HI Lutz,
> > > > >
> > > > > Should we not better adjust the jtreg test?
> > > > >
> > > > > Cheers, Thomas
> > > > >
> > > > > On Tue, Mar 31, 2020 at 9:22 AM Schmidt, Lutz
> > > > <lutz.schmidt at sap.com<mailto:lutz.schmidt at sap.com>>
> > > > > wrote:
> > > > >
> > > > > > Dear All,
> > > > > >
> > > > > > may I please request reviews for this fix, adjusting the CPU
> > features
> > > > > > string such that jtreg tests, in particular a newly introduced
> one,
> > > pass
> > > > OK.
> > > > > >
> > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8241101
> > > > > > Webrev:
> > https://cr.openjdk.java.net/~lucy/webrevs/8241101.00/
> > > > > >
> > > > > > Thank you!
> > > > > > Lutz
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > >
> > >
> > >
> >
> >
>
>
More information about the hotspot-runtime-dev
mailing list