On Wed, 20 Dec 2023 10:11:58 GMT, Sergey Tsypanov <stsypa...@openjdk.org> wrote:
>> Currently if we create a record it's fields are compared in their
>> declaration order. This might be ineffective in cases when two objects have
>> "heavy" fields equals to each other, but different "lightweight" fields
>> (heavy and lightweight in terms of comparison) e.g. primitives, enums,
>> nullable/non-nulls etc.
>>
>> If we have declared a record like
>>
>> public record MyRecord(String field1, int field2) {}
>>
>> It's equals() looks like:
>>
>> public final equals(Ljava/lang/Object;)Z
>> L0
>> LINENUMBER 3 L0
>> ALOAD 0
>> ALOAD 1
>> INVOKEDYNAMIC
>> equals(Lcom/caspianone/openbanking/productservice/controller/MyRecord;Ljava/lang/Object;)Z
>> [
>> // handle kind 0x6 : INVOKESTATIC
>>
>> java/lang/runtime/ObjectMethods.bootstrap(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/TypeDescriptor;Ljava/lang/Class;Ljava/lang/String;[Ljava/lang/invoke/MethodHandle;)Ljava/lang/Object;
>> // arguments:
>> com.caspianone.openbanking.productservice.controller.MyRecord.class,
>> "field1;field2",
>> // handle kind 0x1 : GETFIELD
>>
>> com/caspianone/openbanking/productservice/controller/MyRecord.field1(Ljava/lang/String;),
>> // handle kind 0x1 : GETFIELD
>> com/caspianone/openbanking/productservice/controller/MyRecord.field2(I)
>> ]
>> IRETURN
>> L1
>> LOCALVARIABLE this
>> Lcom/caspianone/openbanking/productservice/controller/MyRecord; L0 L1 0
>> LOCALVARIABLE o Ljava/lang/Object; L0 L1 1
>> MAXSTACK = 2
>> MAXLOCALS = 2
>>
>> This can be improved by rearranging the comparison order of the fields
>> moving enums and primitives upper, and collections/arrays lower.
>
> Sergey Tsypanov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8322292: Tiny improvement
src/java.base/share/classes/java/lang/runtime/ObjectMethods.java line 227:
> 225: }
> 226: return rt1.isPrimitive() || rt1.isEnum() || rt1.isArray() ?
> 1 : Iterable.class.isAssignableFrom(rt1) ? -1 : 0;
> 227: });
The way this comparator is currently implemented, the `signum(c.compare(mh1,
mh2)) == -signum(c.compare(mh2, mh1))` assertion won’t hold, which will cause
the sorting to be unstable.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17143#discussion_r1434166820
