On Fri, 8 Mar 2024 23:26:05 GMT, Andy Goryachev <ango...@openjdk.org> 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