RFR: 8303882: Refactor some iterators in jdk.compiler [v3]
Maurizio Cimadamore
mcimadamore at openjdk.org
Mon Mar 13 10:52:34 UTC 2023
On Mon, 13 Mar 2023 09:55:02 GMT, Pavel Rappo <prappo at openjdk.org> wrote:
>> Please review this refactoring to iterators in jdk.compiler. The refactoring delegates more to collections framework, while retaining the performance characteristics of bespoke iterators. The refactoring also adds inline comments, `@Override` annotations, and fixes some trivial bugs (as can be seen in the added test, which is slightly augmented from that suggested by Jan Lahoda here https://github.com/openjdk/jdk/pull/12904#discussion_r1129777660).
>>
>> I'll add a comment here once I have benchmarked the change formally.
>
> Pavel Rappo has updated the pull request incrementally with four additional commits since the last revision:
>
> - Merge remote-tracking branch 'myjdk/8303882' into 8303882
> - Squeeze a few extra drops of perf
> - Add a recursive test
> - Try squeeze more performance
Looks good - some minor (optional) comments on the test
test/langtools/tools/javac/util/IteratorsTest.java line 112:
> 110: assertAndResetMaxCalls(c, 0);
> 111: assertAndResetMaxCalls(test1, 1, 1);
> 112: assertAndResetMaxCalls(test2, 0, 0);
Should the test continue, and check that we end up with calls to the second iterator?
test/langtools/tools/javac/util/IteratorsTest.java line 163:
> 161:
> 162: public TestConverter(Function<I, Iterator<O>> delegate) {
> 163: this.delegate = delegate;
This parameter doesn't really seem to be used in the test?
test/langtools/tools/javac/util/IteratorsTest.java line 172:
> 170: }
> 171: }
> 172: }
Missing newline here
-------------
Marked as reviewed by mcimadamore (Reviewer).
PR: https://git.openjdk.org/jdk/pull/12949
More information about the compiler-dev
mailing list