RFR: 8289943: Simplify some object allocation merges [v6]

[Cesar Soares]

On Fri, 2 Sep 2022 16:01:15 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> @vnkozlov - Thank you for clarifying that. I've been playing with lifting the restriction and I actually found a corner case:
>> 
>> 
>> public static Class test(boolean c1, boolean c2, boolean c3, int x, int y, int w, int z) {
>>     Animal s = new Dog(x, y, z);
>> 
>>     if (c1) {
>>         s = new Cat("Fisker");
>>     }
>> 
>>     Unloaded u = new Unloaded(); // assumes this is converted to a uncommon_trap(unloaded, reinterpret)
>> 
>>     return s.getClass();
>> }
>> 
>> 
>> It seems that when merging allocations of different subtypes I'll need to add a special `Phi` node merging the `Klass` of the input allocations and assign the output of that `Phi` to `SafepointScalarReplacedNode`. If I don't do that, the method above will return Animal.class instead of `Dog.class` or `Cat.class`. I'm wondering if I'll actually have to do the same for the Header/Mark word of the input allocations.
>
> @JohnTortugo Yes, you would need to construct Phi when you replace RAM with `SafePointScalarObjectNode`. Hmm, may be you would need to construct Phi in other cases too (getClass intrinsic).
> 
> Add cases when class is loaded from argument for allocation: `Unsafe.allocateInstance()` and `Object.clone()` to test class Load instead of simple constant on some paths of such Phi.
> 
> Why you need Phi for mark word? For identity check (hashCode/identityHashCode intrinsics)?

Hi again @vnkozlov, I ended up giving up on adding support for merging allocations of different subclasses. I think the current version of the patch already covers some reasonable ground and it seems that adding support for merging allocations of subclasses will require more widespread changes. 

Also, I wasn't able to find a case where the patch fail due to an incorrect/missing Mark word. FYI we have been testing the current patch with all JTREG tests, TCK tests, SPECJBB, Renaissance, DaCapo, and some customer services. I currently don't see any failure.

I also added a few JMH tests to show up what kind of improvements we can expect with the proposed changes:


Benchmark                                                         Mode  Cnt         Score        Error  Units

AllocationsMerge.IfElseInLoop                                     avgt    5  93,839,709.289 ± 16846405.548  ns/op
AllocationsMerge.MergeAndIterative                                avgt    5     160,988.682 ±    57389.599  ns/op
AllocationsMerge.NestedObjectsObject                              avgt    5     311,523.642 ±    59622.703  ns/op
AllocationsMerge.SimpleMerge                                      avgt    5     142,398.057 ±    57010.144  ns/op
AllocationsMerge.TrapAfterMerge                                   avgt    5     333,061.738 ±    25645.896  ns/op

AllocationsMerge.WithAllocationsMergeEnabled.IfElseInLoop         avgt    5  11,964,798.596 ±  1661843.496  ns/op
AllocationsMerge.WithAllocationsMergeEnabled.MergeAndIterative    avgt    5      33,118.428 ±     2995.224  ns/op
AllocationsMerge.WithAllocationsMergeEnabled.NestedObjectsObject  avgt    5     155,808.370 ±    30379.710  ns/op
AllocationsMerge.WithAllocationsMergeEnabled.SimpleMerge          avgt    5      31,266.188 ±    13476.634  ns/op
AllocationsMerge.WithAllocationsMergeEnabled.TrapAfterMerge       avgt    5     208,694.038 ±    18667.023  ns/op


Please let me know if there is something else that I can do to make it easier to review the proposed changes.

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

PR: https://git.openjdk.org/jdk/pull/9073