On Thu, 11 Feb 2021 13:28:49 GMT, Сергей Цыпанов <[email protected]> wrote:
> Originally was proposed by Zheka Kozlov here: > http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057192.html > > Just a tiny optimization: we can use for-i loop instead of > `Iterable.forEach()` which is relying on iterator. > > Simple benchmark demonstrates slight improvement: > > @State(Scope.Thread) > @BenchmarkMode(Mode.AverageTime) > @OutputTimeUnit(TimeUnit.NANOSECONDS) > public class NCopiesBenchmarks { > @Param({"10", "50", "100"}) > int size; > > private List<Object> list; > > @Setup > public void prepare() { > list = Collections.nCopies(size, new Object()); > } > > @Benchmark > public void forEach(Blackhole bh) { > list.forEach(bh::consume); > } > } > > > > before > > Benchmark (size) Mode Cnt Score > Error Units > NCopiesBenchmarks.forEach 10 avgt 50 40.737 ± > 1.854 ns/op > NCopiesBenchmarks.forEach:·gc.alloc.rate 10 avgt 50 0.001 ± > 0.001 MB/sec > NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 10 avgt 50 ≈ 10⁻⁴ > B/op > NCopiesBenchmarks.forEach:·gc.count 10 avgt 50 ≈ 0 > counts > NCopiesBenchmarks.forEach 50 avgt 50 213.324 ± > 3.784 ns/op > NCopiesBenchmarks.forEach:·gc.alloc.rate 50 avgt 50 0.001 ± > 0.001 MB/sec > NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 50 avgt 50 ≈ 10⁻³ > B/op > NCopiesBenchmarks.forEach:·gc.count 50 avgt 50 ≈ 0 > counts > NCopiesBenchmarks.forEach 100 avgt 50 443.171 ± > 17.919 ns/op > NCopiesBenchmarks.forEach:·gc.alloc.rate 100 avgt 50 0.001 ± > 0.001 MB/sec > NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100 avgt 50 0.001 ± > 0.001 B/op > NCopiesBenchmarks.forEach:·gc.count 100 avgt 50 ≈ 0 > counts > > after > > Benchmark (size) Mode Cnt Score > Error Units > NCopiesBenchmarks.forEach 10 avgt 50 36.838 ± > 0.065 ns/op > NCopiesBenchmarks.forEach:·gc.alloc.rate 10 avgt 50 0.001 ± > 0.001 MB/sec > NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 10 avgt 50 ≈ 10⁻⁴ > B/op > NCopiesBenchmarks.forEach:·gc.count 10 avgt 50 ≈ 0 > counts > NCopiesBenchmarks.forEach 50 avgt 50 191.173 ± > 0.570 ns/op > NCopiesBenchmarks.forEach:·gc.alloc.rate 50 avgt 50 0.001 ± > 0.001 MB/sec > NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 50 avgt 50 ≈ 10⁻⁴ > B/op > NCopiesBenchmarks.forEach:·gc.count 50 avgt 50 ≈ 0 > counts > NCopiesBenchmarks.forEach 100 avgt 50 376.675 ± > 2.476 ns/op > NCopiesBenchmarks.forEach:·gc.alloc.rate 100 avgt 50 0.001 ± > 0.001 MB/sec > NCopiesBenchmarks.forEach:·gc.alloc.rate.norm 100 avgt 50 0.001 ± > 0.001 B/op > NCopiesBenchmarks.forEach:·gc.count 100 avgt 50 ≈ 0 > counts 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>() { ------------- Changes requested by martin (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2524
