On Tue, 14 Apr 2026 05:17:04 GMT, Stuart Marks <[email protected]> wrote:
>> @ExE-Boss thank you very much!! :) > > @jengebr > > Thanks for the updates. > >> Unwrapping UnmodifiableMap for non-HashMap cases such as >> UnmodifiableMap(TreeMap) improves their performance by 40%, according to the >> benchmark data in the current PR description. This was a side effect of >> fixing my target case and not my primary goal. Do we want to lose that in >> favor of code proximity? I can live with any outcome here. > > Hm, it's hard to believe that unwrapping the TreeMap gives a 40% > improvement... that's presumably from bypassing megamorphic calls to the > unmodifiable wrapper's iterator. But TreeMap is generally so slow that I'd > have expected that the actual iteration of the TreeMap to swamp the wrapper > calls. > > A bit more concerning to me is that in the earlier code (which modified the > input parameter) I had missed the implication that the unmodifiable-wrapped > TreeMap (and other maps) would get unwrapped. On the one hand, it's nice that > this would end up running faster. On the other hand, it's an illustration of > the "quantum superposition" that arises from modifying the method parameter. > Since you're OK leaving the TreeMap optimization on the table, I'm OK with > this too, and we can continue with this PR focusing on the HashMap and > wrapped-HashMap special case optimization. > >> This makes me squirm: >> > Also, we've been advocating for a long time (with varied success) that >> > objects protect encapsulation by wrapping internal collections with an >> > unmodifiable wrapper > >> Unmodifiable* methods are globally megamorphic, so adding these wrappers not >> only eliminates the sole workaround to JDK-8368292, but also creates cases >> where the callsite of Unmodifiable* may be megamorphic and the called method >> is trivial followed by yet another megamorphic call. Also, one of our >> standard ways to reduce garbage collection is to eliminate the allocation of >> trivial wrappers such as these. This advice is clearly negative for >> performance. I acknowledge the non-performance benefits. > > To be clear, I'm not requesting wrapping things in unmodifiable wrappers in > this PR. In addition, this is old advice; the unmodifiable wrappers have been > in the collections framework since its introduction in JDK 1.2. > > I think this is the usual tradeoff between object design and performance > considerations. Sometimes it makes sense to have a hierarchy of classes with > different subclasses providing custom behavior via overrides. This can lead > to megamorphic dispatch, and if it's a performance problem, then maybe there > are cases where it makes sense to ... @stuart-marks I can't find your latest MOAT comment in the UI, so I'll respond here: sorry about that, it was entirely my error. I believe `testMap()` matches your expectations now. Apologies! ------------- PR Comment: https://git.openjdk.org/jdk/pull/28243#issuecomment-4329049551
