JDK-8207795 : Change VM.initializeFromArchive() to return a boolean
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 12
  • Priority: P3
  • Status: Closed
  • Resolution: Won't Fix
  • Submitted: 2018-07-18
  • Updated: 2019-05-02
  • Resolved: 2018-11-22
Related Reports
Relates :  
JDK-8202035 - Archive the set of ModuleDescriptor and ModuleReference objects for observable system modules with unnamed initial module
Relates :  
JDK-8207381 - Provide internal Java API for archiving object sub-graph reachable from static field
Relates :  
JDK-8207263 - Store the Configuration for system modules into CDS archive
Description
Currently VM.initializeFromArchive() has no return value. 

  public static native void initializeFromArchive(Class<?> c);

It might be useful to change it to return a boolean:
  True: if initialization from the archive is successful
  False: Not initialized from the archive (archived values are not mapped in the current process; there is an error during initialization from the archive; etc)

Suggested by Claes.
Comments
[~redestad] confirmed that he's okay with closing the REF. Thanks, Claes!
22-11-2018
Here's a proposed change for this RFE: http://cr.openjdk.java.net/~ccheung/8207795/webrev.00/ As indicated by Ioi and Jiangli above, there isn't much benefit of changing the API to return a boolean. Perhaps this should be closed as WNF?
21-11-2018
Then changing VM.initializeFromArchive() to return a boolean makes the API confusing to use. It cannot be used when an archive class has multiple archived fields. You can use it with when there's only a single archive field, but you don't save any code (just replacing field == null with VM.initializeFromArchive() != false). It can potentially lead to bugs in the future when a second archive field is added.
21-11-2018
I realized null value can happen to other archived entry fields as well in certain cases. The Integer cache is one of the example. If we eliminate an entry field at dump time without recording it, then the return value ('true' or 'false') of VM.initializeFromArchive() can only be used safely for a klass with a single entry field. It is not safe if a klass has more than one entry fields. Here is the reason: A klass has multiple archiving entry fields and one of them is null. We eliminate the entry field with the value is null at dump time, and it's not recorded in the subgraph info record. At runtime, VM.initializeFromArchive() returns with 'true', if all recorded entry fields are successfully installed and their values are not null (that would always be the case since we have eliminated the null entry field at dump time already). For any other static fields (including the eliminated entry field) in the klass, they would still contain the default values (0 for primitive type and null for reference type). If the java code relies on VM.initializeFromArchive() return value and doesn't do null check on all the archived fields in this case, it might run into unexpected issue if it assumes all archived fields should contain non-null value. For a klass with single archiving entry field, if the entry field value is null, eliminating it would result 'false' being returned by VM.initializeFromArchive(). So that's a safe case.
21-11-2018
The rules of what can or cannot be null seems to be too complicated. It's not clear whether null means "failed to archive" or "I want this to be null at run time" How about we always required an archived value to be non-null. If you want to store a null value, use an instance field. .E.g., FROM: final class ArchivedModuleGraph { private static String archivedMainModule; private static SystemModules archivedSystemModules; private static ModuleFinder archivedModuleFinder; private static Configuration archivedConfiguration; ... return archivedMainModule; TO final class ArchivedModuleGraph { private static ArchivedModuleGraph archivedInfo; private String archivedMainModule; private SystemModules archivedSystemModules; private ModuleFinder archivedModuleFinder; private Configuration archivedConfiguration; ... return archivedInfo.archivedMainModule;
19-11-2018
At subgraph archiving time, we need to check the entry field values. Only the ArchivedModuleGraph.archivedMainModule field is allowed to contain null value. For all other entry fields, null values are not allowed and a fatal error is reported. If ArchivedModuleGraph.archivedMainModule is null, it's skipped and not archived. At runtime, more checks should be added when initialization the entry fields using the archived data. HeapShared::initialize_from_archived_subgraph() needs to report error by returning false for following cases: - Runtime resolved klass is different from the recorded one in subgraph_object_klasses (check already exists). - Class initialization failure/exception for any of the subgraph_object_klasses (check already exists). - No entry_field_records. - An entry field in the entry_field_records has recorded null value. With the above, the java code can be changed to only check the return value of VM.initializeFromArchive(). All the existing null checks for the archived fields in the java code can be removed. This will make the java code cleaner and the runtime is slightly more efficient. Here is the patch (without the return value part): http://cr.openjdk.java.net/~jiangli/8207795_null_value_check/webrev/
19-11-2018