RFR: 8335922: Incorrect @Stable usage of LambdaForm$Name.index [v2]

Aleksey Shipilev shade at openjdk.org
Mon Jul 15 15:32:52 UTC 2024


On Mon, 15 Jul 2024 14:09:22 GMT, Chen Liang <liach at openjdk.org> wrote:

>> The `@Stable` on the `index` field is incorrect, as stable only avoids inlining `0`. Solution is to use bit flip on the actual index (and rename the field to `flippedIndex`), so we use -1 to -256 (mapping to 0 to 255) and 0 the default value is used as an unset indicator.
>
> Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   We have sufficient space in short, use +1 offset

Looks reasonable to me, with nits.

src/java.base/share/classes/java/lang/invoke/LambdaForm.java line 1400:

> 1398:             if (offsetIndex != i + 1) {
> 1399:                 if (offsetIndex != 0)  return false;
> 1400:                 offsetIndex = (short) (i + 1);

Can we pull `(short) (i + 1);` into a local variable, given that we need it on all paths?

src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java line 778:

> 776:         var newParameters = new TreeMap<Name, Integer>(new Comparator<>() {
> 777:             public int compare(Name n1, Name n2) {
> 778:                 return n1.index() - n2.index();

We don't have to call the method here and do the translation to "proper" index here, or do we?

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20178#pullrequestreview-2177881698
PR Review Comment: https://git.openjdk.org/jdk/pull/20178#discussion_r1677980731
PR Review Comment: https://git.openjdk.org/jdk/pull/20178#discussion_r1677926814


More information about the core-libs-dev mailing list