JDK10/RFR(L): 8172231: SPARC ISA/CPU feature detection is broken/insufficient (on Solaris).

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

JDK10/RFR(L): 8172231: SPARC ISA/CPU feature detection is broken/insufficient (on Solaris).

Patric Hedlin

Dear all,

I would like to ask for help to review the following change/update:

Issue:    https://bugs.openjdk.java.net/browse/JDK-8172231

Webrev:   http://cr.openjdk.java.net/~neliasso/8172231/



8172231: SPARC ISA/CPU feature detection is broken/insufficient (on Solaris).

    Updating SPARC feature/capability detection (incorporating changes from Martin Walsh).
    More complete set of features as provided by 'getisax(2)' interface, propagated via JVMCI.
    More robust hardware probing for additional features (up to Core S4).
    Removing support for old, pre Niagara, hardware.
    Removing support for old, pre 11.1, Solaris.

    Changed behaviour:
    Changing SPARC setup for AllocatePrefetchLines and AllocateInstancePrefetchLines
    such that they will (still) be doubled when cache-line size is small (32 bytes),
    but more moderately increased on new/contemporary hardware (inc >= 50%).
    Changing to default instruction fetch alignment based on derived caps. instead
    of relying on default/configuration values.

    The above changes also subsumes:
    8035146: assert(is_T_family(features) == is_niagara(features), "Niagara should be T series") is incorrect
    8054979: Remove unnecessary defines in SPARC's VM_Version::platform_features


Rationale:

    Current hardware detection on Solaris/SPARC is not up to date with the "latest" (here,
    meaning commercially available server solutions, i.e. T7/M7). To facilitate improved
    use of the new hardware features provided (by Core S3&S4) these capabilities need to
    be recognised by the JVM.

    NOTE: This update is limited to Core S3&S4, i.e. not including Core S5. Proper Core S5
    support will be added when regular testing and benchmarking resources are available,
    i.e. regular testing need to include M8 hardware.


Caveat:

    This update will introduce some redundancies into the code base, features and definitions
    currently not used, as well as a (small) number of FIXMEs, addressed by subsequent bug or
    feature updates/patches. Fujitsu HW is treated very conservatively.


Testing:

    Mostly tested on JDK9 (RBT/hotspot/comp). Only local testing on JDK10 (jtreg/hotspot).


Benchmarking:

    Benchmark reports from a limited set of runs can be found at:
http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.jbb05
http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.jvm08
http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.octane.plus
    (Limited availability of M7 hardware prevents complete suites/runs.)


Best regards,
Patric

Reply | Threaded
Open this post in threaded view
|

Re: JDK10/RFR(L): 8172231: SPARC ISA/CPU feature detection is broken/insufficient (on Solaris).

Stefan Anzinger
Hi Patric,

I had a look and tested this change with jvmci/graal on Solaris. It
compiles and runs without any error. Looks good to me.

Stefan



On 04/28/2017 01:48 PM, Patric Hedlin wrote:

> Dear all,
>
> I would like to ask for help to review the following change/update:
>
> Issue:    https://bugs.openjdk.java.net/browse/JDK-8172231
>
> Webrev:   http://cr.openjdk.java.net/~neliasso/8172231/
>
>
>
> 8172231: SPARC ISA/CPU feature detection is broken/insufficient (on
> Solaris).
>
>     Updating SPARC feature/capability detection (incorporating changes
> from Martin Walsh).
>     More complete set of features as provided by 'getisax(2)' interface,
> propagated via JVMCI.
>     More robust hardware probing for additional features (up to Core S4).
>     Removing support for old, pre Niagara, hardware.
>     Removing support for old, pre 11.1, Solaris.
>
>     Changed behaviour:
>     Changing SPARC setup for AllocatePrefetchLines and
> AllocateInstancePrefetchLines
>     such that they will (still) be doubled when cache-line size is small
> (32 bytes),
>     but more moderately increased on new/contemporary hardware (inc >= 50%).
>     Changing to default instruction fetch alignment based on derived
> caps. instead
>     of relying on default/configuration values.
>
>     The above changes also subsumes:
>     8035146: assert(is_T_family(features) == is_niagara(features),
> "Niagara should be T series") is incorrect
>     8054979: Remove unnecessary defines in SPARC's
> VM_Version::platform_features
>
>
> Rationale:
>
>     Current hardware detection on Solaris/SPARC is not up to date with
> the "latest" (here,
>     meaning commercially available server solutions, i.e. T7/M7). To
> facilitate improved
>     use of the new hardware features provided (by Core S3&S4) these
> capabilities need to
>     be recognised by the JVM.
>
>     NOTE: This update is limited to Core S3&S4, i.e. not including Core
> S5. Proper Core S5
>     support will be added when regular testing and benchmarking
> resources are available,
>     i.e. regular testing need to include M8 hardware.
>
>
> Caveat:
>
>     This update will introduce some redundancies into the code base,
> features and definitions
>     currently not used, as well as a (small) number of FIXMEs, addressed
> by subsequent bug or
>     feature updates/patches. Fujitsu HW is treated very conservatively.
>
>
> Testing:
>
>     Mostly tested on JDK9 (RBT/hotspot/comp). Only local testing on
> JDK10 (jtreg/hotspot).
>
>
> Benchmarking:
>
>     Benchmark reports from a limited set of runs can be found at:
>
>     http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.jbb05
>
>     http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.jvm08
>
>     http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.octane.plus
>
>
>     (Limited availability of M7 hardware prevents complete suites/runs.)
>
>
> Best regards,
> Patric
>
Reply | Threaded
Open this post in threaded view
|

Re: JDK10/RFR(L): 8172231: SPARC ISA/CPU feature detection is broken/insufficient (on Solaris).

David Holmes
In reply to this post by Patric Hedlin
Hi Patric,

I have read the below and looked through the proposed changes. While I
can't validate the details (as I am not familiar with chip capabilities)
the overall approach looks good and I prefer the capability-based tests
to the "family" based tests.

As you note there are a few fixme's and follow ups to do, but one
suggestion I have is to remove the UseV8InstrsOnly flag. It doesn't make
sense to me to keep this if we will abort on V8 anyway. The nature of
the flag, in an unsupported environment, precludes it from following our
more usual deprecation process, and it is a develop flag anyway.

I do have concerns about how this may work on Fujitsu, but hopefully
there is plenty of bake-time in JDK 10 to shake out any issue.

Thanks,
David

On 28/04/2017 11:48 PM, Patric Hedlin wrote:

> Dear all,
>
> I would like to ask for help to review the following change/update:
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8172231
>
> Webrev: http://cr.openjdk.java.net/~neliasso/8172231/
>
>
>
> 8172231: SPARC ISA/CPU feature detection is broken/insufficient (on
> Solaris).
>
>     Updating SPARC feature/capability detection (incorporating changes
> from Martin Walsh).
>     More complete set of features as provided by 'getisax(2)' interface,
> propagated via JVMCI.
>     More robust hardware probing for additional features (up to Core S4).
>     Removing support for old, pre Niagara, hardware.
>     Removing support for old, pre 11.1, Solaris.
>
>     Changed behaviour:
>     Changing SPARC setup for AllocatePrefetchLines and
> AllocateInstancePrefetchLines
>     such that they will (still) be doubled when cache-line size is small
> (32 bytes),
>     but more moderately increased on new/contemporary hardware (inc >=
> 50%).
>     Changing to default instruction fetch alignment based on derived
> caps. instead
>     of relying on default/configuration values.
>
>     The above changes also subsumes:
>     8035146: assert(is_T_family(features) == is_niagara(features),
> "Niagara should be T series") is incorrect
>     8054979: Remove unnecessary defines in SPARC's
> VM_Version::platform_features
>
>
> Rationale:
>
>     Current hardware detection on Solaris/SPARC is not up to date with
> the "latest" (here,
>     meaning commercially available server solutions, i.e. T7/M7). To
> facilitate improved
>     use of the new hardware features provided (by Core S3&S4) these
> capabilities need to
>     be recognised by the JVM.
>
>     NOTE: This update is limited to Core S3&S4, i.e. not including Core
> S5. Proper Core S5
>     support will be added when regular testing and benchmarking
> resources are available,
>     i.e. regular testing need to include M8 hardware.
>
>
> Caveat:
>
>     This update will introduce some redundancies into the code base,
> features and definitions
>     currently not used, as well as a (small) number of FIXMEs, addressed
> by subsequent bug or
>     feature updates/patches. Fujitsu HW is treated very conservatively.
>
>
> Testing:
>
>     Mostly tested on JDK9 (RBT/hotspot/comp). Only local testing on
> JDK10 (jtreg/hotspot).
>
>
> Benchmarking:
>
>     Benchmark reports from a limited set of runs can be found at:
>
>
> http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.jbb05
>
>
>
> http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.jvm08
>
>
>
> http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.octane.plus
>
>
>
>     (Limited availability of M7 hardware prevents complete suites/runs.)
>
>
> Best regards,
> Patric
>
Reply | Threaded
Open this post in threaded view
|

Re: JDK10/RFR(L): 8172231: SPARC ISA/CPU feature detection is broken/insufficient (on Solaris).

Vladimir Kozlov
In reply to this post by Patric Hedlin
Hi Patric

Very nice work.

Did you measure a performance improvement when you increase allocation prefetch values on new S4?

Since you removed call to kstat you can remove kstat.h include and link to libkstat in makefile:

http://hg.openjdk.java.net/jdk10/hs/file/35273d1dff83/common/autoconf/flags.m4#l1290

Thanks,
Vladimir

On 4/28/17 6:48 AM, Patric Hedlin wrote:

> Dear all,
>
> I would like to ask for help to review the following change/update:
>
> Issue:    https://bugs.openjdk.java.net/browse/JDK-8172231
>
> Webrev:   http://cr.openjdk.java.net/~neliasso/8172231/
>
>
>
> 8172231: SPARC ISA/CPU feature detection is broken/insufficient (on Solaris).
>
>     Updating SPARC feature/capability detection (incorporating changes from Martin Walsh).
>     More complete set of features as provided by 'getisax(2)' interface, propagated via JVMCI.
>     More robust hardware probing for additional features (up to Core S4).
>     Removing support for old, pre Niagara, hardware.
>     Removing support for old, pre 11.1, Solaris.
>
>     Changed behaviour:
>     Changing SPARC setup for AllocatePrefetchLines and AllocateInstancePrefetchLines
>     such that they will (still) be doubled when cache-line size is small (32 bytes),
>     but more moderately increased on new/contemporary hardware (inc >= 50%).
>     Changing to default instruction fetch alignment based on derived caps. instead
>     of relying on default/configuration values.
>
>     The above changes also subsumes:
>     8035146: assert(is_T_family(features) == is_niagara(features), "Niagara should be T series") is incorrect
>     8054979: Remove unnecessary defines in SPARC's VM_Version::platform_features
>
>
> Rationale:
>
>     Current hardware detection on Solaris/SPARC is not up to date with the "latest" (here,
>     meaning commercially available server solutions, i.e. T7/M7). To facilitate improved
>     use of the new hardware features provided (by Core S3&S4) these capabilities need to
>     be recognised by the JVM.
>
>     NOTE: This update is limited to Core S3&S4, i.e. not including Core S5. Proper Core S5
>     support will be added when regular testing and benchmarking resources are available,
>     i.e. regular testing need to include M8 hardware.
>
>
> Caveat:
>
>     This update will introduce some redundancies into the code base, features and definitions
>     currently not used, as well as a (small) number of FIXMEs, addressed by subsequent bug or
>     feature updates/patches. Fujitsu HW is treated very conservatively.
>
>
> Testing:
>
>     Mostly tested on JDK9 (RBT/hotspot/comp). Only local testing on JDK10 (jtreg/hotspot).
>
>
> Benchmarking:
>
>     Benchmark reports from a limited set of runs can be found at:
>
>     http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.jbb05
>     http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.jvm08
>     http://aurora.se.oracle.com/performance/reporting/report/patric.hedlin.TvM.octane.plus
>
>     (Limited availability of M7 hardware prevents complete suites/runs.)
>
>
> Best regards,
> Patric
>