RFR: 8348426: Generate binary file for -XX:AOTMode=record -XX:AOTConfiguration=file [v7]

[Ashutosh Mehra]

On Thu, 20 Feb 2025 05:10:44 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Currently, with `java -XX:AOTMode=record -XX:AOTConfiguration=file ...`, a text file is written. The file contains the names of loaded classes, indices of resolved constant pools entries, etc, that are easily represented in text.
>> 
>> With the upcoming 2nd JEP of the Leyden project, [JDK-8325147](https://bugs.openjdk.org/browse/JDK-8325147) (Ahead-of-Time Method Profiling), the AOT config file needs to record complex data structures that are difficult to represent in text (we would need code for serializing hierarchical data structures to/from text). Also, a next step after [JDK-8325147](https://bugs.openjdk.org/browse/JDK-8325147) would be to support hidden classes that have no predictable names. Representing such classes with textual names would become another challenge.
>> 
>> To prepare for  [JDK-8325147](https://bugs.openjdk.org/browse/JDK-8325147), this PR writes the AOT configuration file in a **binary format** (essentially the same format as a CDS archive file). This allows arbitrary data associated with the cached classes to be processed and stored using the existing `MetaspaceClosure` API (which can recursively copy C++ objects). Such a change in the file format is allowed by [JEP 483](https://openjdk.org/jeps/483):
>> 
>>> the format of the configuration and cache files is not specified and is subject to change without notice.
>> 
>> **Notes for reviewers:**
>> 
>> - Although the new config file format is essentially the same as a CDS "static" archive, for sanity, we use a different magic number so that the config file cannot be accidentally used as a CDS archive. See new tests inside AOTFlags.java.
>> - After this PR, the CDS "static" archive can be dumped in three modes: "classic", "preimage", and "final". See new comments in cdsConfig.hpp.
>> - The main starting point of this PR is `CDSConfig::check_aot_flags()` - it checks the existence of `-XX:AOTConfiguration` and `-XX:AOTMode` to configure the JVM to dump the CDS "preimage" or "final" archives as necessary.
>> - Most of the other changes are checks for `CDSConfig::is_dumping_preimage_static_archive()` and `CDSConfig::is_dumping_final_static_archive()` to handle subtlle differences between the different dumping modes.
>> - I also updated the UL messages to use the new JEP 483 terminology ("AOT cache", "AOT configuration file",  etc) when JEP 483 options are specified.
>> 
>> **Misc Note**
>> - The changes in [CDS.java and RunTests.gmk](https://github.com/iklam/jdk/commit/0e77a35c25a968c7d931931bc108ccb...
>
> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
> 
>  - Merge branch 'master' into 8348426-binary-aot-config-file
>  - @ashu-mehra comments
>  - @calvinccheung comments
>  - Improved JTREG_AOT_JDK=true so we do not need to add test code into the JDK itself
>  - Improve error message when AOTMode=create has an incompatible classpath
>  - Fixed test cases @vnkozlov
>  - Update "make test JTREG_AOT_JDK=true ..." to work with binary AOT configuration
>  - Fixed test failures
>  - Added comments; fixed FIXMEs
>  - Added more test cases
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/00d4e4a9...21f140e7

src/hotspot/share/cds/cppVtables.cpp line 200:

> 198: //     _orig_cpp_vtptrs[ConstantPool_Kind] == ((intptr_t**)cp)[0]
> 199: //
> 200: // _archived_cpp_vtptrs is a map of all the vptprs used by classes in a preimage. E.g., for

Thanks for adding these comments. I think I now understand why we need `_archived_cpp_vtptrs`.
I am wondering if we really need to store this table in the preimage.
When the control enters `CppVtables::dumptime_init`, if we are dumping the final archive, then the `_index[kind].cloned_vtable()` would be pointing to the vtable in the preimage. So we can initialize the `_archived_cpp_vtptrs` at that time before `_index[kind]` is overwritten by the runtime vtable data.
Wouldn't that work?

Something like this:

```@@ -231,13 +231,15 @@ char* CppVtables::_vtables_serialized_base = nullptr;
 void CppVtables::dumptime_init(ArchiveBuilder* builder) {
   assert(CDSConfig::is_dumping_static_archive(), "cpp tables are only dumped into static archive");
 
-  CPP_VTABLE_TYPES_DO(ALLOCATE_AND_INITIALIZE_VTABLE);
-
-  if (!CDSConfig::is_dumping_final_static_archive()) {
+  // When dumping final archive, _index[kind] at this point is in the preimage.
+  // Store the vtable pointers present in the preimage  as _index[kind] will now be rewritten
+  // to point to the runtime vtable data.
+  if (CDSConfig::is_dumping_final_static_archive()) {
     for (int kind = 0; kind < _num_cloned_vtable_kinds; kind++) {
       _archived_cpp_vtptrs[kind] = _index[kind]->cloned_vtable();
     }
   }
+  CPP_VTABLE_TYPES_DO(ALLOCATE_AND_INITIALIZE_VTABLE);
 
   size_t cpp_tables_size = builder->rw_region()->top() - builder->rw_region()->base();
   builder->alloc_stats()->record_cpp_vtables((int)cpp_tables_size);
@@ -253,16 +255,6 @@ void CppVtables::serialize(SerializeClosure* soc) {
   if (soc->reading()) {
     CPP_VTABLE_TYPES_DO(INITIALIZE_VTABLE);
   }
-
-  if (soc->writing() && !CDSConfig::is_dumping_preimage_static_archive()) {
-    // This table is written only when creating the preimage. It will be used
-    // only when writing the final static archive.
-    memset(_archived_cpp_vtptrs, 0, sizeof(_archived_cpp_vtptrs));
-  }
-
-  for (int kind = 0; kind < _num_cloned_vtable_kinds; kind++) {
-    soc->do_ptr(&_archived_cpp_vtptrs[kind]);
-  }
 }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23484#discussion_r1966840116