RFR: 8308407: libjvm library not reproducibly comparable between vendors [v3]
Andrew John Hughes
andrew at openjdk.org
Fri May 19 16:02:53 UTC 2023
On Fri, 19 May 2023 13:05:41 GMT, Andrew Leonard <aleonard at openjdk.org> wrote:
>> Andrew Leonard has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit:
>>
>> 8308407: libjvm library not reproducibly comparable between vendors
>>
>> Signed-off-by: Andrew Leonard <anleonar at redhat.com>
>
> Re-based due to upstream build break:
> > > @andrew-m-leonard is 64 chars sufficient for the most common values found in the `vm_vendor` string? Some indication that this covers the major vendors would make it a clearer win.
> >
> >
> > `sizeof(VENDOR) < VENDOR_PADDING ? VENDOR_PADDING : sizeof(VENDOR)` means it will always be at least `VENDOR_PADDING` but may be longer. An earlier revision - see [adoptium/jdk#17](https://github.com/adoptium/jdk/pull/17) - did start to impose a 64 character limit, but the patch was revised to avoid this.
>
> If `sizeof(VENDOR)` on most distributions is >64 chars, then using 64 as a minimum provides no value. I have no idea what the typical length of this string is and I expect most reviewers only know the value their distribution uses. Confirming the common choices are less then 64 chars makes the reviewers lives easier as it affirms the value of the PR beyond its technical correctness.
I see your point. The default ("Oracle Corporation") and the ones I'm familiar with ("Red Hat, Inc.", "Adoptium") fit well within 64 characters, and this PR would make all three take up the same amount of space, easing comparisons between binaries from different vendors. I don't know if @andrew-m-leonard has studied more cases, but I believe a length over 64 characters is uncommon. It's also only an issue if comparing binaries with different vendor strings, and even then, it would be no different to the situation prior to this patch. So I do think this change makes common comparisons more viable and doesn't regress any existing ones.
As noted previously, the handling of lengths above 64 characters is more for potential backwards compatibility than because we've actually encountered any with a length that long in the wild i.e. we don't want to break something that was theoretically possible before.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14058#issuecomment-1554789705
More information about the hotspot-runtime-dev
mailing list