RFR: 8303755: Clean up around JavacElements.getAllMembers [v3]
Pavel Rappo
prappo at openjdk.org
Wed Mar 8 10:18:28 UTC 2023
On Wed, 8 Mar 2023 08:35:27 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Respond to feedback
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/util/Iterators.java line 68:
>
>> 66: @Override
>> 67: public O next() {
>> 68: if (!hasNext()) {
>
> I was looking into the history, and the reason for the current code is JDK-8167070 (performance). So, I'd suggest to try how this new code performs. The pre-JDK-8167070 did two calls to nested Iterator's hasNext for each call to hasNext (so for deeper hierarchies, the number of invocations grew very fast), while the code here only does one, so it quite probably may be OK.
>
> I wonder if it is time to write a (unit) test for this class?
When I saw a _custom_ empty iterator, my first reaction was to ask myself why it was needed when Collections.emptyIterator had been long available. So yes, I also saw JDK-8167070.
I think, I was careful enough to preserve the good behavior and remove the bad behavior as well as unneeded code. When developing this PR, I used throwaway unit tests to satisfy myself that the iterator behaves as expected. I'm not sure that we need to integrate those tests. But if we do, then please suggest how it should look like, infrastructure-wise that is.
-------------
PR: https://git.openjdk.org/jdk/pull/12904
More information about the compiler-dev
mailing list