On Mon, 18 Dec 2023 13:42:35 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.
Generally, this is a good idea. A few comments:
1. Arrays are compared by reference rather than element-wise, so they
should/could be at the beginning similar to enums, not at the end.
2. You are sorting the array passed to the bootstrap method. This might be
observable to callers, potentially causing problems, especially when e.g.
creating the method handle for equals first, and then for toString with the
same array.
3. What's the test situation here? Is there enough coverage for things like
that already?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/17143#issuecomment-1862179369
