[lworld] RFR: 8331006: [lworld] Support of null markers for nullable flat fields [v2]

[Frederic Parain]

On Thu, 25 Apr 2024 10:19:02 GMT, David Simms <dsimms at openjdk.org> wrote:

>> Frederic Parain has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fixing issues spotted during reviews
>
> test/hotspot/jtreg/runtime/valhalla/inlinetypes/NullableFlatFieldTest.java line 29:
> 
>> 27:  * @modules java.base/jdk.internal.vm.annotation
>> 28:  * @enablePreview
>> 29:  * @run main/othervm -XX:+EnableNullableFieldFlattening -XX:+PrintInlineLayout NullableFlatFieldTest
> 
> `PrintInlineLayout` isn't a "product flag" is it ? Should the test be debug only, or should we make `PrintInlineLayout` a product flag ? I'd consider the later for EA release.

`PrintFieldLayout` and `PrintInlineLayout` are now product diagnostic flags.

> test/hotspot/jtreg/runtime/valhalla/inlinetypes/field_layout/NullMarkersTest.java line 276:
> 
>> 274:     OutputAnalyzer out = new OutputAnalyzer(pb.start());
>> 275: 
>> 276:     Asserts.assertEquals(out.getExitValue(), 0, "Something went wrong while running the tests");
> 
> Speeds up problem solving if the output and error streams for processes is printed...there is the same issue with `PrintFieldLayout` not being product flag

The output now contains more information to speed-up investigations. In most cases, the output is too big to be saved by jtreg with a normal configuration, but added information should indicate in which class an error has been detected (last part of the log is saved by jtreg). 
A better solution would be to not print the whole output, but just the layouts of the classes involved in the fail check. But this would require significant work and this patch is already big. I'd prefer to push that in another RFE.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1078#discussion_r1583306227
PR Review Comment: https://git.openjdk.org/valhalla/pull/1078#discussion_r1583304944