RFR: 8308869: C2: use profile data in subtype checks when profile has more than one class [v5]

[Tobias Hartmann]

On Thu, 15 Jun 2023 13:08:36 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> In this simple micro benchmark:
>> 
>> https://github.com/franz1981/java-puzzles/blob/main/src/main/java/red/hat/puzzles/polymorphism/RequireNonNullCheckcastScalability.java#L70
>> 
>> Performance drops sharply with polluted profile:
>> 
>> 
>> Benchmark                                         (typePollution)   Mode  Cnt     Score    Error   Units
>> RequireNonNullCheckcastScalability.isDuplicated1            false  thrpt   10  1453.372 ± 24.919  ops/us
>> 
>> 
>> to:
>> 
>> 
>> Benchmark                                         (typePollution)   Mode  Cnt   Score   Error   Units
>> RequireNonNullCheckcastScalability.isDuplicated1             true  thrpt   10  28.579 ± 2.280  ops/us
>> 
>> 
>> The test has 2 type checks to 2 different interfaces so caching with
>> `secondary_super_cache` doesn't help.
>> 
>> The micro-benchmark only uses 2 different concrete classes
>> (`DuplicatedContext` and `NonDuplicatedContext`) and they are recorded
>> in profile data at the type checks. But c2 only take advantage of
>> profile data at type checks if they report a single class.
>> 
>> What I propose is that the full blown type check expanded in
>> `Phase::gen_subtype_check()` takes advantage of profile data. So in
>> the case of the micro benchmark, before checking the
>> `secondary_super_cache`, generated code checks whether the object
>> being type checked is a `DuplicatedContext` or a
>> `NonDuplicatedContext`.
>> 
>> This works fairly well on this micro benchmark:
>> 
>> 
>> Benchmark                                         (typePollution)   Mode  Cnt    Score    Error   Units
>> RequireNonNullCheckcastScalability.isDuplicated1             true  thrpt   10  871.224 ± 20.750  ops/us
>> 
>> 
>> It also scales much better if there are multiple threads running the
>> same test (`secondary_super_cache` doesn't scale well: see
>> JDK-8180450).
>> 
>> Now if the micro-benchmark is changed according to the comment:
>> 
>> https://github.com/franz1981/java-puzzles/blob/d2d60af3d0dfe7a2567807395138edcb1d1c24f5/src/main/java/red/hat/puzzles/polymorphism/RequireNonNullCheckcastScalability.java#L62
>> 
>> so the type check hits in the `secondary_super_cache`, the current
>> code performs much better:
>> 
>> 
>> Benchmark                                         (typePollution)   Mode  Cnt    Score    Error   Units
>> RequireNonNullCheckcastScalability.isDuplicated1             true  thrpt   10  871.224 ± 20.750  ops/us
>> 
>> 
>> but leveraging profiling as explained above performs even better:
>> 
>> 
>> Benchmark                   ...
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   whitespaces

`testlibrary_tests/ir_framework/tests/TestIRMatching.java` still fails: `Failed IR Rules (21) of Methods (8)`:


1) Method "public void ir_framework.tests.Traps.classCheck()" - [Failed IR rules: 3]:
   * @IR rule 1: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeatureOr={}, applyIfCPUFeature={}, counts={}, failOn={"_#TRAP#_"}, applyIfAnd={}, applyIfOr={}, applyIfNot={})"
     > Phase "PrintIdeal":
       - failOn: Graph contains forbidden nodes:
         * Constraint 1: "(\\d+(\\s){2}(CallStaticJava.*)+(\\s){2}===.*uncommon_trap.*reason)"
           - Matched forbidden nodes (2):
             * 39  CallStaticJava  === 33 6 7 8 9 (38 1 1 10 29 ) [[ 40 ]] # Static uncommon_trap(reason
             * 60  CallStaticJava  === 245 6 7 8 9 (59 1 1 10 24 ) [[ 61 ]] # Static uncommon_trap(reason
   * @IR rule 2: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeatureOr={}, applyIfCPUFeature={}, counts={}, failOn={"_#CLASS_CHECK_TRAP#_"}, applyIfAnd={}, applyIfOr={}, applyIfNot={})"
     > Phase "PrintIdeal":
       - failOn: Graph contains forbidden nodes:
         * Constraint 1: "(\\d+(\\s){2}(CallStaticJava.*)+(\\s){2}===.*uncommon_trap.*class_check)"
           - Matched forbidden node:
             * 60  CallStaticJava  === 245 6 7 8 9 (59 1 1 10 24 ) [[ 61 ]] # Static uncommon_trap(reason='class_check
   * @IR rule 3: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeatureOr={}, applyIfCPUFeature={}, counts={}, failOn={"_#NULL_CHECK_TRAP#_"}, applyIfAnd={}, applyIfOr={}, applyIfNot={})"
     > Phase "PrintIdeal":
       - failOn: Graph contains forbidden nodes:
         * Constraint 1: "(\\d+(\\s){2}(CallStaticJava.*)+(\\s){2}===.*uncommon_trap.*null_check)"
           - Matched forbidden node:
             * 39  CallStaticJava  === 33 6 7 8 9 (38 1 1 10 29 ) [[ 40 ]] # Static uncommon_trap(reason='null_check

[...]

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14375#issuecomment-1597036069