On Fri, 8 Mar 2024 23:26:05 GMT, Andy Goryachev <[email protected]> wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Optimize performance of OpenAddressed Set just in case it is ever used
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/FixedCapacitySet.java
> line 207:
>
>> 205:
>> 206: for (T element : c) {
>> 207: modified |= add(element);
>
> is there a reason we are overriding the superclass with essentially the same
> implementation (and this implementation executes OR on every element, while
> the superclass one is more efficient).
> not that it's important in the case of Single...
You are right, I hadn't noticed this, probably when I was splitting the sets in
different capacities, but the above implementation is indeed the same as in
`AbstractCollection#addAll`. I will remove it.
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/FixedCapacitySet.java
> line 570:
>
>> 568: T element = elements[i];
>> 569:
>> 570: h += element == null ? 0 : element.hashCode();
>
> - is there a reason not to use the standard hashCode pattern of `result = 31
> * result + (element == null ? 0 : element.hashCode());`?
> - should we also mix in the class hash code?
This can't be changed, see other comment, but in short, `Set`s have a specified
implementation for `hashCode` to make them interoperable.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1518483390
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1518483590
