On Tue, 12 Mar 2024 10:12:18 GMT, Viktor Klang <vkl...@openjdk.org> wrote:
>> Please review this patch that:
>> 1. Implemented `forEach` to optimize for 1 or 2 element collections.
>> 2. Implemented `spliterator` to optimize for a single element.
>>
>> The default implementations for multiple-element immutable collections are
>> fine as-is, specializing implementation doesn't provide much benefit.
>
> src/java.base/share/classes/java/util/ImmutableCollections.java line 926:
>
>> 924: if (!REVERSE && e1 != EMPTY) {
>> 925: action.accept((E) e1);
>> 926: }
>
> I'm curious to know how the following alternative would fare:
>
> Suggestion:
>
> if (e1 != EMPTY) {
> action.accept(REVERSE ? (E)e1 : (E)e0); // implicit null check
> action.accept(REVERSE ? (E)e0 : (E)e1);
> } else {
> action.accept(e0); // Implicit null check
> }
I used this:
if (e1 == EMPTY) {
action.accept(e0); // Implicit null check
} else {
action.accept(REVERSE ? (E)e1 : e0); // implicit null check
action.accept(REVERSE ? e0 : (E)e1);
}
The results are about the same as the current patch.
With the alternative:
Benchmark Mode Cnt Score Error Units
ImmutableColls.forEachOverList thrpt 15 367.651 ± 10.730 ops/us
ImmutableColls.forEachOverSet thrpt 15 84.397 ± 12.235 ops/us
ImmutableColls.getOrDefault thrpt 15 243.874 ± 1.891 ops/us
ImmutableColls.iterateOverList thrpt 15 149.525 ± 3.652 ops/us
ImmutableColls.iterateOverSet thrpt 15 60.202 ± 5.053 ops/us
Current patch:
Benchmark Mode Cnt Score Error Units
ImmutableColls.forEachOverList thrpt 15 364.515 ± 8.076 ops/us
ImmutableColls.forEachOverSet thrpt 15 86.012 ± 2.420 ops/us
ImmutableColls.getOrDefault thrpt 15 243.723 ± 1.071 ops/us
ImmutableColls.iterateOverList thrpt 15 149.392 ± 5.328 ops/us
ImmutableColls.iterateOverSet thrpt 15 63.579 ± 4.671 ops/us
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15834#discussion_r1522201572
