RFR: JDK-8287061: Support for rematerializing scalar replaced objects participating in allocation merges [v14]

[Cesar Soares Lucas]

On Mon, 5 Jun 2023 19:26:59 GMT, Cesar Soares Lucas <cslucas at openjdk.org> wrote:

>> src/hotspot/share/code/debugInfo.cpp line 301:
>> 
>>> 299: void ObjectMergeValue::print_detailed(outputStream* st) const {
>>> 300:   st->print("merge: ID=%d", _id);
>>> 301: #ifndef PRODUCT
>> 
>> Can you post a sample of the output, please?
>> 
>> Why is it limited to non-product builds? It's valuable irrespective of build flavor.
>> 
>> As I see in `ObjectValue::print_on` and `ScopeDesc::print_on`, you mix `print_on` with `print_fields_on`. Any particular reason for that? You could add `is_object_merge` case in ObjectValue::print_on` instead and extend `ObjectValue::print_fields_on` to cover `ObjectMergeValue` case. I find it hard to reason about `ObjectValue::print_on` vs `ObjectMergeValue::print_on` since it's a non-virtual method. 
>> 
>>  
>> 
>> Also, formatting is broken.
>
> I added a few samples below and there are a few more here: https://gist.github.com/JohnTortugo/913523947e08157def6cfebafa7d5daa
> 
> Sample 1:
> 
> 
> Compiled method (c2)     415   24             TestTrapAfterMerge::test (57 bytes)
>  total in heap  [0x00007f7b4d03da90,0x00007f7b4d03de18] = 904
>  relocation     [0x00007f7b4d03dc00,0x00007f7b4d03dc18] = 24
>  main code      [0x00007f7b4d03dc20,0x00007f7b4d03dcb8] = 152
>  stub code      [0x00007f7b4d03dcb8,0x00007f7b4d03dcd0] = 24
>  oops           [0x00007f7b4d03dcd0,0x00007f7b4d03dce0] = 16
>  metadata       [0x00007f7b4d03dce0,0x00007f7b4d03dce8] = 8
>  scopes data    [0x00007f7b4d03dce8,0x00007f7b4d03dd50] = 104
>  scopes pcs     [0x00007f7b4d03dd50,0x00007f7b4d03de10] = 192
>  dependencies   [0x00007f7b4d03de10,0x00007f7b4d03de18] = 8
> scopes:
> ScopeDesc(pc=0x00007f7b4d03dc3a offset=1a):
>    TestTrapAfterMerge::test at -1 (line 3)
> ScopeDesc(pc=0x00007f7b4d03dc41 offset=21):
>    TestTrapAfterMerge::test at 11 (line 5)
> ScopeDesc(pc=0x00007f7b4d03dc44 offset=24):
>    TestTrapAfterMerge::test at 51 (line 12)
> ScopeDesc(pc=0x00007f7b4d03dc4a offset=2a):
>    TestTrapAfterMerge::test at 46 (line 8)
> ScopeDesc(pc=0x00007f7b4d03dc52 offset=32):
>    TestTrapAfterMerge::test at 37 (line 9)
> ScopeDesc(pc=0x00007f7b4d03dc57 offset=37):
>    TestTrapAfterMerge::test at 43 (line 8)
> ScopeDesc(pc=0x00007f7b4d03dc61 offset=41):
>    TestTrapAfterMerge::test at 46 (line 8)  reexecute=true
>    Locals
>     - l0: empty
>     - l1: empty
>     - l2: reg rbx [6],int
>     - l3: empty
>     - l4: merge: ID=26
>     - l5: reg r11 [22],int
>    Objects
>     - 0: merge: ID=26, selector="reg r10 [20],int", merge_pointer="nullptr", candidate objs=[27, 28]
>     - 1: obj: ID=27, is_root=0, N.Fields=1, klass: Point 
> Fields: reg r8 [16],int
>     - 2: obj: ID=28, is_root=0, N.Fields=1, klass: Point 
> Fields: reg rcx [2],int
> ScopeDesc(pc=0x00007f7b4d03dc63 offset=43):
>    TestTrapAfterMerge::test at 46 (line 8)
> ScopeDesc(pc=0x00007f7b4d03dc6c offset=4c):
>    TestTrapAfterMerge::test at 34 (line 8)
> ScopeDesc(pc=0x00007f7b4d03dc71 offset=51):
>    TestTrapAfterMerge::test at 55 (line 12)
> 
> 
> - Sample2:
> 
> 
> Compiled method (c2)     443   24             TestManys::test (41 bytes)
>  total in heap  [0x00007f35e9155b90,0x00007f35e9155e78] = 744
>  relocation     [0x00007f35e9155d00,0x00007f35e9155d18] = 24
>  main code      [0x00007f35e9155d20,0x00007f35e9155d88] = 104
>  stub code      [0x00007f35e9155d88,0x00007f35e9155da0] = 24
>  oops           [0x00007f35e9155da0,0x00007f35e9155db0] =...

> Why is it limited to non-product builds? It's valuable irrespective of build flavor.

This is because `print_on` in `AnyObj` is only defined in non-product builds. I based implementation of `ObjectMergeValue::print_on` on `ObjectValue::print_on`. In `ObjectValue::print_on` fields aren't printed in product builds.

> Any particular reason for that? You could add is_object_merge case in ObjectValue::print_oninstead and extendObjectValue::print_fields_onto coverObjectMergeValue case.

I'll do that then.

> Also, formatting is broken.

Can you please share an example? If you mean the tabs on lines 303/304/306/307 I added those because I thought would make the code easier to read, but if you want I can definitely remove that.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12897#discussion_r1218523643