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