RFR: 8315850: Improve AbstractMap anonymous Iterator classes

[Rémi Forax]

On Thu, 7 Sep 2023 18:05:51 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> Hello,
>> In Java, sharing code may have a runtime cost (or not) depending on the shape of the code, because the VM may have to speculate on the class of some value at runtime.
>> Here, in hasNext() and remove(), the VM has to find the class of the field 'i' at runtime.
>> 
>> Iterators are performance sentive (they are used in for loop) and for that they need the escape analysis to work.
>> If the iterator code is polymorphic, so part of the code may be unknown so the escape analysis will consider that the iterator escape. So usually, it's a good practice to not share the iterator code.
>> 
>> Also the field should be package private (or even maybe private) but not protected. Protected is very rare in Java because usually classes that share implementation details are in the same package (or in the same nestmate nest).
>
> @forax Note that HashMap uses a similar subclassing idiom for the iterators of the keySet, values, and entrySet collections in order to share the iterator logic:
> 
> https://github.com/openjdk/jdk/blob/jdk-21%2B35/src/java.base/share/classes/java/util/HashMap.java#L1626

Hi @stuart-marks ,
for HashMap, all the fields of HashIterator have only one implementation, in this patch, the field of AbstractIterator is typed Iterator (so multiple implementations at runtime). This is not the same code shape.

I think the best is to write a simple to test that iterate over both the keySet() and the values() of an implementation of AbstractMap that only defines size() and get() and use -XX:+PrintInlining to see if the VM reports a profile pollution for the code before and after this change.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15615#issuecomment-1710632697