RFR: 8375561: Class NGGroup is in need of some cleanup [v2]
Christopher Schnick
duke at openjdk.org
Sat Jan 17 04:26:56 UTC 2026
On Fri, 16 Jan 2026 22:44:27 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:
>> Optimization: I don't know. Maybe, maybe not. I suspect the contract is to create the iterator.
>>
>> My main point - it is a completely unnecessary change, a change for the sake of change.
>
> An enhanced for-loop is syntactic sugar for:
>
> Iterator<E> it = list.iterator();
> while (it.hasNext()) {
> E e = it.next();
> ...
> }
>
>
> If you look at the bytecode, you'll see that there is an object allocation. However, if C2 can prove via escape analysis that the iterator doesn't escape, it can
> 1. inline the `ArrayList.iterator()` call into the calling method, which is just a `new ArrayList$Itr()` expression,
> 2. scalarize the object allocation into locals, effectively storing `ArrayList$Itr`'s fields in the stack frame.
>
> So yes, an enhanced for-loop can be optimized by the C2 compiler so that it doesn't allocate any memory. However, this is not a guarantee, and optimization may fail for a number of reasons:
> 1. The call site must be monomorphic, that means that the runtime profile of the call site only ever sees `ArrayList` here. If different concrete `List` classes are encountered at runtime, the call site becomes polymorphic or megamorphic and the method call can't be inlined.
> 2. The iterator can't be proven to not escape (stored somewhere, passed to a method, etc.)
> 3. The call-site method body is too large, so that inlining of `ArrayList.iterator()` doesn't happen.
>
> All in all, it seems likely that iterator optimization will happen here. For clarity of intent, I would still recommend an indexed for-loop if you want to make sure that no object allocations happen at runtime, and I would also think in this case you should not repeatedly call the `List.size()` method, but just call it once:
>
> for (int i = 0, max = list.size(); i < max; ++i) { // the intent here is obvious
> ...
> }
Thanks for the detailed explanation. That makes me feel better about using for each loops when possible in my code.
For this PR, I reverted the for loops to the original variant due to other feedback.
The implementation to call the size() method only once looks interesting. However I have never seen that in an actual codebase. Probably because the overhead for a normal size() call is quite small and seeing such a loop would, as Andy would say it, require more thinking for anyone having to look at it. But I like it, maybe I can make use of that somewhere
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2043#discussion_r2700623710
More information about the openjfx-dev
mailing list