On Wed, 11 Feb 2026 15:28:56 GMT, jengebr <[email protected]> wrote: >> # HashMap.putAll() optimizations: Eliminating Megamorphic Call Site >> Bottlenecks >> >> ## Summary >> >> This PR addresses performance bottlenecks in `HashMap.putMapEntries()` by >> implementing direct optimizations for specific input types: `j.u.HashMap` >> and `j.u.Collections$UnmodifiableMap`. The optimizations target >> `HashMap(Map)` constructor and `putAll()` operations based on the real-world >> megamorphic behavior identified in >> [JDK-8368292](https://bugs.openjdk.org/browse/JDK-8368292), delivering >> significant performance improvements when multiple `Map` subtypes are used. >> >> ## Problem Context >> >> ### Megamorphic Call Site Overhead in Map Iteration >> `HashMap.putMapEntries()` currently uses a generic approach that suffers >> from megamorphic call site overhead when applications perform bulk creation >> or population of HashMaps from various source map types: >> >> 1. `m.entrySet()` becomes megamorphic across different map implementations >> 2. `entrySet().iterator()` creates different iterator types >> 3. `entry.getKey()` and `entry.getValue()` calls vary by map type >> 4. Individual `putVal()` calls for each entry >> >> When the source is `Collections$UnmodifiableMap`, the problem is compounded >> by megamorphic wrappers around the already-megamorphic iteration methods. In >> cases where the unwrapped map is also a HashMap, both the wrapper overhead >> and the iteration overhead can be eliminated with a single optimization. >> >> ## Optimized Methods >> >> ### HashMap >> - **`putMapEntries(Map<? extends K, ? extends V> m, boolean evict)`**: Added >> fast paths for UnmodifiableMap unwrapping and HashMap-to-HashMap copying >> - **`putMapEntries(HashMap<? extends K, ? extends V> src, boolean evict)`**: >> copies HashMap-to-HashMap via direct Node processing. Avoids polymorphic >> issues and eliminates redundant calls to HashMap.hash(). >> >> ## Implementation Details >> >> ### HashMap-to-HashMap Fast Path >> Eliminates megamorphic iteration by targeting internal Node structure - and >> also reuses the pre-calculated hash code, thus avoiding megamorphic calls to >> Object.hashCode() and the sometimes-expensive recalculation (depending on >> key type). This also eliminates direct reads from the key, thus reducing >> the set of objects accessed. >> >> ### UnmodifiableMap Unwrapping >> Detects UnmodifiableMap instances and accesses the underlying map directly >> via the `m` field, eliminating wrapper-induced megamorphic call sites. >> UnmodifiableMap visibility changed from `private` to package-private to >> enable this... > > jengebr has updated the pull request incrementally with one additional commit > since the last revision: > > Addressing CR feedback (formatting + safe casts)
Hi, I finally got some time to dig into this. Fundamentally, the performance analysis seems sound. The remedy of peeling off a couple special cases for copying from HashMap also seems sensible. Based on some studies I've seen, HashMap is the second-most used collection, trailing only ArrayList. There are many times when it's necessary to make defensive copies of collections, so optimizing HashMap's copy construction path makes sense. Also, we've been advocating for a long time (with varied success) that objects protect encapsulation by wrapping internal collections with an unmodifiable wrapper, so handling that case too makes a lot of sense. I agree with using a specific class check for `HashMap.class` (as opposed to `instanceof HashMap`). I have two topics for discussion: 1) how `putMapEntries(Map)` should handle the logic to determine the special case; and 2) how the fast path loop should be implemented. I'll comment on those near the respective pieces of code, or add to existing comment threads. Note that I'll be taking off for the long weekend; I'll return midweek next week. Again, sorry for the long delay on this. But we should be able to move forward. ------------- PR Comment: https://git.openjdk.org/jdk/pull/28243#issuecomment-4184243777
