[foreign] RFR 8218153: Read/Write of Value layout should honor declared endianness

Henry Jen henry.jen at oracle.com
Thu Feb 28 21:57:12 UTC 2019



> On Feb 28, 2019, at 1:23 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> Thanks, some comments, again on withEndianness:
> 
> 1) There's a missing override on Unresolved - this means that if the user calls withEndianness on Unresolved he'll get back a Layout, so you will lose type info
> 

Good point, I remove default method and add override for Unresolved and Padding.

> 2) Group::withEndianness is weird:
> 
>  if (elementLayout instanceof Value || elementLayout instanceof Group) {
> 
> With this you are basically covering the entire space, but Unresolved. I think it's better just to let virtual dispatch do its job - e.g. call withEndianness on the element. I wouldn't bother too much about minimizing the instances being created. If concrete benchmarks reveal an issue, we can fix it when the time comes. But in general layouts are only created once and then used multiple times; minimizing layout creation time is a second order issue, I think.
> 

Not in Group, but in Sequence. Like you said, it was trying to reduce instances creation. I remove the if.

> 3) Same for Sequence
> 
> 4) Address - should it ever have endianness? Yes. This is a deliberate choice; address layouts are more than just machine addresses - they will also be used to encode references to other parts of a message protocol schema - so in that case endianness could be relevant (as size).
> 

I updated webrev[1], or you can see the diff[2] from previous.

> The rest looks ok.
> 

Any comment on the jextract patch, good to go or we prefer to modify the annotation of each master file?

Cheers,
Henry

[1] http://cr.openjdk.java.net/~henryjen/panama/8218153/4/webrev/
[2] http://cr.openjdk.java.net/~henryjen/panama/8218153/4/diff-from-3


More information about the panama-dev mailing list