On Mon, 4 Oct 2021 16:51:27 GMT, Martin Buchholz <[email protected]> wrote:
>> Сергей Цыпанов has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains four additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into ncopies
>> - 8274715: Add NCopiesBenchmarks.java
>> - 8274715: Revert some irrelevant changes
>> - 8274715: Implement forEach in Collections.CopiesList
>
> Core collection classes should have optimized versions of forEach, so this is
> a good change in principle. Although CopiesList.forEach is unlikely to be
> performance critical.
>
> I implemented many similar optimizations for core collection classes in past
> years.
> Many of them are benchmarked in
> test/jdk/java/util/Collection/IteratorMicroBenchmark.java
> That was written pre-JMH.
> I see a JMH benchmark was written, but it is not part of the commit.
>
> There are a number of unrelated changes in this commit that look like they
> were suggested by a lint-like tool. Such changes are good, but they belong
> in a separate cleanup commit applied across large portions of the jdk sources.
>
> I would not use "var" here - more readable with concrete types.
>
> Similarly, I prefer not using diamond for
> return new Enumeration<T>() {
@Martin-Buchholz thanks for review! I've reverted irrelevant changes and added
the benchmark.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2524
