RFR: 8290833: Remove ConstantPoolCache::walk_entries_for_initialization() [v3]

[Coleen Phillimore]

On Mon, 8 Aug 2022 21:48:34 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()`.
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use a separate _saved_cpcache_entries_table

I think this might need some more comments.

src/hotspot/share/classfile/systemDictionaryShared.cpp line 525:

> 523:   if (cpc != NULL) {
> 524:     remove_saved_cpcache_entries_locked(cpc);
> 525:   }

Why did you need the separate table?

src/hotspot/share/classfile/systemDictionaryShared.cpp line 1348:

> 1346:   assert_lock_strong(DumpTimeTable_lock);
> 1347:   ConstantPoolCacheEntry** p = _saved_cpcache_entries_table->get(cpc);
> 1348:   if (p) {

Should be: p != nullptr
and might as well return nullptr below.

src/hotspot/share/oops/cpCache.cpp line 701:

> 699:   Arguments::assert_is_dumping_archive();
> 700:   ConstantPoolCache* orig_cpc = ArchiveBuilder::current()->get_src_obj(this);
> 701:   ConstantPoolCacheEntry* saved = SystemDictionaryShared::get_saved_cpcache_entries_locked(orig_cpc);

What does this do?  get_src_obj looks like it just returns the address with a cast to the type T.

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

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