RFR: 8290833: Remove ConstantPoolCache::walk_entries_for_initialization()

[Coleen Phillimore]

On Thu, 4 Aug 2022 23:22:25 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> **Background:**
> 
> `ConstantPoolCache::walk_entries_for_initialization()` is called by `ConstantPoolCache::remove_unshareable_info()` to restore the CpCache to the state immediately after the class has been rewritten by the Rewriter (which happens during the class linking phase). In most part, this means the `ConstantPoolCacheEntry`'s need to be zeroed. However, the `_f2` fields of some of the entries are initialized to be non-zero by the Rewriter and must be preserved.
> 
> The reason `walk_entries_for_initialization()` exists is that after `Rewriter::rewrite()` has finished, some information about what is stored inside the CpCache is discarded (e.g., `Rewriter::_invokedynamic_references_map`). As a result, we cannot easily determine which entries has a `_f2` field that need to be preserved. We must walk all the bytecodes in all the methods of this class to recompute this information.
> 
> This is awkward and time consuming. It also needs to be updated if the `Rewriter` ever changes.
> 
> Also, for future optimizations, we may need to pre-resolve a subset of the CpCache entries during CDS dump time. Trying to make that work alongside `walk_entries_for_initialization()` seems too complicated.
> 
> **Fix:**
> 
> Store a copy of the CpCache after rewritting. Use this to revert the CpCache's state inside  `ConstantPoolCache::remove_unshareable_info()`.

This looks like a nice improvement, not having to figure out how to reinitialize the cpCache properly.

src/hotspot/share/cds/dumpTimeClassInfo.cpp line 80:

> 78:   // The saved cp entries array is not modified by the ArchiveBuilder, so there's
> 79:   // no need to make a deep copy. It will be freed when the holder class is removed.
> 80:   _saved_cpcache_entries = src._saved_cpcache_entries;

If you're copying these entries, should you make src._saved_cpcache_entries null so you don't double delete them?  Or just for safety in case someone adds deleting it to the destructor later.

src/hotspot/share/cds/dumpTimeClassInfo.hpp line 132:

> 130:   GrowableArray<DTLoaderConstraint>*   _loader_constraints;
> 131:   GrowableArray<int>*                  _enum_klass_static_fields;
> 132:   ConstantPoolCacheEntry*       _saved_cpcache_entries;

can you align with enum_klass_static_fields?

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

Marked as reviewed by coleenp (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9759