On Mon, 5 Jun 2023 21:10:22 GMT, Cesar Soares Lucas <[email protected]> wrote:
>> A couple of suggestions about the output:
>> * `merge`: it's clearer to call it `merge_obj`
>> * `obj` vs `merge` output: obj output is duplicated in ScopeDesc entries and
>> Objects sections; before it was a short version printed in
>> Locals/Expressions and all the details were included in Objects; I like to
>> see field locations in the short version, but including everything looks way
>> too much IMO;
>> * it makes sense to include selector and merge_pointer info in short
>> version, but `is_root` can be omitted
>
> Thanks @iwanowww . Does the output below look good to you? It prints
> ObjectValue in the same format as it was before this PR and only print
> details of the merge in the "Objects" section. Is there other output section
> that you think needs to be adjusted?
>
>
> Compiled method (c2) 436 24 TestMultiSFO::test (48 bytes)
> total in heap [0x00007f1df5155590,0x00007f1df5155850] = 704
> relocation [0x00007f1df5155700,0x00007f1df5155718] = 24
> main code [0x00007f1df5155720,0x00007f1df5155788] = 104
> stub code [0x00007f1df5155788,0x00007f1df51557a0] = 24
> oops [0x00007f1df51557a0,0x00007f1df51557b0] = 16
> metadata [0x00007f1df51557b0,0x00007f1df51557b8] = 8
> scopes data [0x00007f1df51557b8,0x00007f1df51557f8] = 64
> scopes pcs [0x00007f1df51557f8,0x00007f1df5155848] = 80
> dependencies [0x00007f1df5155848,0x00007f1df5155850] = 8
> scopes:
> ScopeDesc(pc=0x00007f1df515573a offset=1a):
> TestMultiSFO::test@-1 (line 12)
> ScopeDesc(pc=0x00007f1df515575c offset=3c):
> TestMultiSFO::test@28 (line 19)
> Locals
> - l0: empty
> - l1: empty
> - l2: empty
> - l3: merge_obj[14]
> - l4: obj[15]
>
> Objects
> - merge_obj[14], selector="reg rbp [10],int", merge_pointer="nullptr",
> candidate_objs=[15, 16]
> - obj[15], is_root=1, klass: TestMultiSFO$Point
> Fields: stack[12], stack[8]
> - obj[16], is_root=0, klass: TestMultiSFO$Point
> Fields: stack[8], stack[12]
Thanks, it looks much better now (except the position in Objects array is
missing).
It makes sense to mention `is_root` for merge_obj case even though it's always
equals to '1`. Also, make merge_pointer optional and omit it when its value is
null.
BTW instead of printing `is_root=0/1`, you can introduce a more compact
notation for and mark relevant lines with a single symbol:
- 0: R merge_obj[14], selector="reg rbp [10],int" candidates=[15, 16]
- 1: R obj[15], klass: TestMultiSFO$Point
Fields: stack[12], stack[8]
- 2: obj[16], klass: TestMultiSFO$Point
Fields: stack[8], stack[12]
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12897#discussion_r1218642386
