RFR(XS): 8193509: Test dynamic path to retrieve active processor count.

[Lindenmaier, Goetz]

Thanks, Martin!

Best regards,
  Goetz.

> -----Original Message-----
> From: Doerr, Martin
> Sent: Freitag, 15. Dezember 2017 16:58
> To: David Holmes <david.holmes at oracle.com>; Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com>; hotspot-runtime-dev at openjdk.java.net
> Subject: RE: RFR(XS): 8193509: Test dynamic path to retrieve active processor
> count.
> 
> Hi Götz,
> 
> thanks for providing this test which runs through the CPU_ALLOC code path.
> Looks good.
> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> bounces at openjdk.java.net] On Behalf Of David Holmes
> Sent: Donnerstag, 14. Dezember 2017 23:55
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
> dev at openjdk.java.net
> Subject: Re: RFR(XS): 8193509: Test dynamic path to retrieve active processor
> count.
> 
> My testing passed. So just needing second review.
> 
> Also for now I suggest ensuring the "FixVersion" is not set when doing
> the push, as the correct version for JDK 11 has not yet been set in the
> configuration info used by hgupdater. Otherwise if you correctly set
> FixVersion to 11, hgupdater will create a 10 backport; if you set it to
> 10 and hgupdater gets fixed before you push, then you will get a
> backport for 11. So leave it blank or just wait until we know hgupdater
> is doing the right thing.
> 
> Thanks,
> David
> 
> On 15/12/2017 7:12 AM, David Holmes wrote:
> > Hi Goetz,
> >
> > On 14/12/2017 11:01 PM, Lindenmaier, Goetz wrote:
> >> Hi David,
> >>
> >> I implemented the test to detect iff the VM is run on an older OS
> >> than it was compiled on which lacks the CPU_ALLOC support.
> >> Basically that's what I wrote into the @summary :)
> >
> > So do you plan on running the test on such older OS? And on such an OS
> > will you ever find >1024 processors and so try to really take the path
> > that test shows is not usable ?
> >
> >> I ran the test with jdk 9 on ppc64, ppc64le, s390, x86_64,
> >> SLES 11, 12; RHEL 6, 7 (not all combinations, though).
> >> Tonight it will run with jdk/hs and a similar coverage.  (Actually, the
> >> test has been in the test suite for the other platforms, too, but
> >> is skipped there, so skipping is tested, too.)
> >>
> >> New webrev with renamed test:
> >> http://cr.openjdk.java.net/~goetz/wr17/8193509-
> activeProcTest/webrev.02/
> >
> > Okay. I'll apply it locally and run on our systems just for a sanity
> > check. Meanwhile a second reviewer is needed.
> >
> > Thanks,
> > David
> >
> >>
> >> Best regards,
> >>   Goetz.
> >>
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: David Holmes [mailto:david.holmes at oracle.com]
> >>> Sent: Donnerstag, 14. Dezember 2017 13:40
> >>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
> runtime-
> >>> dev at openjdk.java.net
> >>> Subject: Re: RFR(XS): 8193509: Test dynamic path to retrieve active
> >>> processor
> >>> count.
> >>>
> >>> Hi Goetz,
> >>>
> >>> On 14/12/2017 10:06 PM, Lindenmaier, Goetz wrote:
> >>>> Hi,
> >>>>
> >>>> active_processor_count() in os_linux.cpp has two paths, one is only
> >>>> excercised if the VM runs on a machine with more than 1024
> processors.
> >>>> Flag UseCpuAllocPath allows to force this path. Add a test with
> >>>> this flag to excercise this code.
> >>>
> >>> Not sure I see the value in having a test for this. Where would this
> >>> test run such that it would show us there is some problem that needs
> >>> fixing? The real test of that code path is on a machine with > 1024
> >>> processors.
> >>>
> >>> That aside please reformat the
> >>>
> >>>     29  * @summary If #processors > 1024 os_linux.cpp uses special
> >>> coding. Excercise this by forcing usage of this coding. If this fails,
> >>> this VM was either compiled on a platform which does not define
> >>> CPU_ALLOC, or it is executed on a platform that does not support it.
> >>>
> >>> to use multiple lines. Though a simple:
> >>>
> >>> @summary  Test the UseCpuAllocPath code path
> >>>
> >>> would suffice IMHO.
> >>>
> >>> And the name of the test should more accurately be
> TestUseCpuAllocPath.
> >>>
> >>>> Please review this change. I please need a sponsor. (Do I need one
> with
> >>> the new repo setup?)
> >>>> http://cr.openjdk.java.net/~goetz/wr17/8193509-
> >>> activeProcTest/webrev.01/
> >>>
> >>> The need for a sponsor is not related to the repo setup, but the need
> >>> for adequate cross-platform testing. If the set of platforms you've
> >>> tested on includes all the platforms Oracle would test on, then a
> >>> sponsor would not add anything to the equation. But your RFR needs to
> >>> include what testing you have done.
> >>>
> >>> Thanks,
> >>> David
> >>>
> >>>> Best regards,
> >>>>     Goetz.
> >>>>