`acmp` for value object might be expensive: it may need substitutability test, 
which is done through a Java call. That is the last resort when everything 
failed, that is: we are in the presence of two value objects of the same class, 
not statically known or speculated (profile absent or polluted). While a lot of 
the acmp logic is trying to improve acmp at compile-time, this change proposes 
a runtime check to take a fast path.

The idea is that if the object is small enough, we can read all the fields at 
once with a single 8-byte load, we exclude the non-interesting bits with a 
mask, and we can simply compare the masked value from both objects.

The first step happens at class loading. When the class is loaded, we check 
whether all the fields can be read at once, from which offset, and the mask to 
filter out useless bits. Useless bits may have two sources:
- padding between fields
- header: this happens because we cannot necessarily start reading at 
`payload_offset`, the payload might be smaller than a long, and we could 
overread. On the other hand, the header is always long enough (never less than 
8 bytes), so we may read from somewhere in the header to make sure we don't go 
further than the payload. This piece of header must be filtered out.

This strategy may not apply if the object is too big, or if it contains an oop. 
Loading oop without precaution, concurrently with the GC is not a good idea. 
This limitation is rather harmless: this fast path is mostly made not to pay 
the price of a complicated `acmp` for magically migrated classes (such as 
`Integer`), which are small and don't contain oops.

There is an interesting corner case: the mask can be 0 for an empty value 
class. We use a negative offset to signal that the fast path doesn't apply, 
since it is not a valid value for the offset: we shouldn't load before the 
object.

The second step is to prepare the runtime check. In `do_acmp`, when we used to 
prepare the Java call, now we have a parallel fast path. The condition uses the 
fact that we already have the class node available (used for checking operands 
have the same types). We can get the fast acmp offset from the class, check if 
it looks valid (that is non-negative), and if so, take the fast path that will 
load the mask, do the two loads and masking, and compare the given values. If 
the class of the operand is known precisely at parsing, we don't emit the fast 
path since the call will be nicely intrinsified.

The third step is to prevent the fast path to interfere with the faster path. 
When the class is not known at parsing-time, but after some optimizations it 
becomes precise, we already emitted the fast path for acmp while we now can 
intrinsify the substitutability test. That means, not only we will still pay 
the price of the fast path test, but also, we might not even take the 
intrinsified substitutability test (if the fast path applies). Thus, comes a 
hack. It's not very nice, but it's not too bad. When we are about to 
intrinsify, we check around if we can find the fast path test. This test is 
rather specific because nothing else needs the address of the fast acmp offset. 
If we find such a test, we simply replace it a constant, not to take the fast 
path. This works because C2 is unable to fold the load of the fast acmp 
offset/mask, even when the class is a constant, and that the check was 
carefully crafted to be already in ideal form. Since the check is quite 
precise, we don't risk to
  mess with an unrelated `If`. It is also not too bad if this test is not 
eliminated: we might end up taking the fast path instead of the faster path.

This is not very nice, and not a definitive version. A way forward is probably 
through some macro node that gather all the resources needed to generate the 
acmp IR, and that can pick when is the best moment.

But... does it work? It was successfully tested on 
`tier1,tier2,tier3,tier4,hs-comp-stress,valhalla-comp-stress`, and also with 
`+UseCompactObjectHeaders` (because one can be worried about loads).

But how well does it perform? In average, some benchmarks show improvements, 
and other are not affected. I've also wrote a microbench:

with fast path:

Mode  Cnt  Score   Error  Units
FastPath.big_mix                      avgt   15  1.487 ± 0.039  ns/op
FastPath.custom_multi_field           avgt   15  1.172 ± 0.021  ns/op
FastPath.custom_single_field          avgt   15  1.217 ± 0.034  ns/op
FastPath.empty_objects                avgt   15  0.632 ± 0.013  ns/op
FastPath.empty_objects_heterogeneous  avgt   15  0.896 ± 0.037  ns/op
FastPath.heterogeneous_eq             avgt   15  1.217 ± 0.040  ns/op
FastPath.heterogeneous_eq_neq         avgt   15  1.309 ± 0.039  ns/op
FastPath.heterogeneous_neq            avgt   15  1.191 ± 0.026  ns/op
FastPath.homogeneous_eq               avgt   15  0.739 ± 0.021  ns/op
FastPath.homogeneous_eq_neq           avgt   15  0.872 ± 0.021  ns/op
FastPath.homogeneous_neq              avgt   15  0.867 ± 0.014  ns/op
FastPath.identity_objects             avgt   15  0.462 ± 0.011  ns/op
FastPath.numeric_classes              avgt   15  1.207 ± 0.034  ns/op
FastPath.too_big                      avgt   15  4.733 ± 0.131  ns/op
FastPath.with_oops                    avgt   15  2.668 ± 0.082  ns/op

without:

Benchmark                             Mode  Cnt  Score   Error  Units
FastPath.big_mix                      avgt   15  2.274 ± 0.076  ns/op
FastPath.custom_multi_field           avgt   15  4.722 ± 0.112  ns/op
FastPath.custom_single_field          avgt   15  3.928 ± 0.118  ns/op
FastPath.empty_objects                avgt   15  0.630 ± 0.012  ns/op
FastPath.empty_objects_heterogeneous  avgt   15  1.651 ± 0.052  ns/op
FastPath.heterogeneous_eq             avgt   15  6.952 ± 0.163  ns/op
FastPath.heterogeneous_eq_neq         avgt   15  5.557 ± 0.179  ns/op
FastPath.heterogeneous_neq            avgt   15  4.298 ± 0.150  ns/op
FastPath.homogeneous_eq               avgt   15  0.752 ± 0.019  ns/op
FastPath.homogeneous_eq_neq           avgt   15  0.869 ± 0.016  ns/op
FastPath.homogeneous_neq              avgt   15  0.874 ± 0.019  ns/op
FastPath.identity_objects             avgt   15  0.463 ± 0.014  ns/op
FastPath.numeric_classes              avgt   15  3.751 ± 0.050  ns/op
FastPath.too_big                      avgt   15  4.704 ± 0.148  ns/op
FastPath.with_oops                    avgt   15  2.455 ± 0.097  ns/op


We can see a few cases. For instance:
- `too_big` or `with_oops` are cases where the fast path doesn't apply. So, on 
top of taking always the slow path, we pay at runtime the cost of the test.
- all the `homogeneous_*`, here profiling does wonders: the type is speculated, 
and we are taking the intrinsified test. The fast path is not generated (or 
removed).
- `heterogeneous_*`, here profiling is polluted, but all the classes can use 
the fast path, the win is quite huge
- `big_mix` has everything (identity class, value classes with and without fast 
path and null), the fast path is generated, but not always taken. The win is 
still there.

The worst case of this change is when the `acmp` involves value objects that 
are never candidate to the fast path. If it becomes problematic, I think we 
could consider profiling that, in the same way we profile the occurrences of 
`null` or of identity classes. If we have a profile that shows that none of the 
classes seen before would be eligible for the fast path, we could skip it. If 
the profile shows that fast path is possible, it wouldn't need a lot of fast 
path-compatible value objects to make it overall worth it. I'm not convinced 
this level of details make sense at the moment: most of the value classes are 
the automatically migrated onces and most of them are fast path-compatible.

Thanks,
Marc

---------
- [x] I confirm that I make this contribution in accordance with the [OpenJDK 
Interim AI Policy](https://openjdk.org/legal/ai).

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

Commit messages:
 - nl at eof
 - Complete InlineKlass::Members::print_on
 - Revert bad indent
 - nl at eof
 - nl at eof
 - Outdated comment
 - Fewer magic constants
 - Remove printing
 - Oopsi
 - Flagless test
 - ... and 10 more: https://git.openjdk.org/valhalla/compare/8b5c1adf...1d9722c8

Changes: https://git.openjdk.org/valhalla/pull/2353/files
  Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=2353&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8381472
  Stats: 1474 lines in 15 files changed: 1460 ins; 0 del; 14 mod
  Patch: https://git.openjdk.org/valhalla/pull/2353.diff
  Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2353/head:pull/2353

PR: https://git.openjdk.org/valhalla/pull/2353

Reply via email to