On Mon, 4 Oct 2021 16:51:27 GMT, Martin Buchholz <mar...@openjdk.org> 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

Reply via email to