8319343: Improve CDS module graph support for --add-modules option

Reviewed-by: alanb, iklam

src/hotspot/share/cds/cdsConfig.cpp

@@ -236,7 +236,9 @@ void CDSConfig::init_shared_archive_paths() {
 }
 
 void CDSConfig::check_internal_module_property(const char* key, const char* value) {
-  if (Arguments::is_internal_module_property(key) && !Arguments::is_module_path_property(key)) {
+  if (Arguments::is_internal_module_property(key) &&
+      !Arguments::is_module_path_property(key) &&
+      !Arguments::is_add_modules_property(key)) {
     stop_using_optimized_module_handling();
     log_info(cds)("optimized module handling: disabled due to incompatible property: %s=%s", key, value);
   }

src/hotspot/share/cds/filemap.cpp

@@ -934,7 +934,7 @@ void FileMapInfo::extract_module_paths(const char* runtime_path, GrowableArray<c
     ClassLoaderExt::extract_jar_files_from_path(name, module_paths);
   }
   // module paths are stored in sorted order in the CDS archive.
-  module_paths->sort(ClassLoaderExt::compare_module_path_by_name);
+  module_paths->sort(ClassLoaderExt::compare_module_names);
 }
 
 bool FileMapInfo::check_module_paths() {

src/hotspot/share/cds/metaspaceShared.cpp

@@ -403,6 +403,7 @@ void MetaspaceShared::serialize(SerializeClosure* soc) {
   soc->do_tag(--tag);
 
   CDS_JAVA_HEAP_ONLY(Modules::serialize(soc);)
+  CDS_JAVA_HEAP_ONLY(Modules::serialize_addmods_names(soc);)
   CDS_JAVA_HEAP_ONLY(ClassLoaderDataShared::serialize(soc);)
 
   LambdaFormInvokers::serialize(soc);
@@ -502,6 +503,8 @@ char* VM_PopulateDumpSharedSpace::dump_read_only_tables() {
   LambdaFormInvokers::dump_static_archive_invokers();
   // Write module name into archive
   CDS_JAVA_HEAP_ONLY(Modules::dump_main_module_name();)
+  // Write module names from --add-modules into archive
+  CDS_JAVA_HEAP_ONLY(Modules::dump_addmods_names();)
   // Write the other data to the output array.
   DumpRegion* ro_region = ArchiveBuilder::current()->ro_region();
   char* start = ro_region->top();

src/hotspot/share/classfile/classLoaderExt.cpp

@@ -90,7 +90,7 @@ void ClassLoaderExt::setup_app_search_path(JavaThread* current) {
   os::free(app_class_path);
 }
 
-int ClassLoaderExt::compare_module_path_by_name(const char** p1, const char** p2) {
+int ClassLoaderExt::compare_module_names(const char** p1, const char** p2) {
   return strcmp(*p1, *p2);
 }
 
@@ -121,7 +121,7 @@ void ClassLoaderExt::process_module_table(JavaThread* current, ModuleEntryTable*
 
   // Sort the module paths before storing into CDS archive for simpler
   // checking at runtime.
-  module_paths->sort(compare_module_path_by_name);
+  module_paths->sort(compare_module_names);
 
   for (int i = 0; i < module_paths->length(); i++) {
     ClassLoader::setup_module_search_path(current, module_paths->at(i));

src/hotspot/share/classfile/classLoaderExt.hpp

@@ -72,7 +72,7 @@ class ClassLoaderExt: public ClassLoader { // AllStatic
   static void setup_search_paths(JavaThread* current);
   static void setup_module_paths(JavaThread* current);
   static void extract_jar_files_from_path(const char* path, GrowableArray<const char*>* module_paths);
-  static int compare_module_path_by_name(const char** p1, const char** p2);
+  static int compare_module_names(const char** p1, const char** p2);
 
   static char* read_manifest(JavaThread* current, ClassPathEntry* entry, jint *manifest_size) {
     // Remove all the new-line continuations (which wrap long lines at 72 characters, see

src/hotspot/share/classfile/modules.cpp

@@ -30,6 +30,7 @@
 #include "classfile/classLoader.hpp"
 #include "classfile/classLoaderData.inline.hpp"
 #include "classfile/classLoaderDataShared.hpp"
+#include "classfile/classLoaderExt.hpp"
 #include "classfile/javaAssertions.hpp"
 #include "classfile/javaClasses.hpp"
 #include "classfile/javaClasses.inline.hpp"
@@ -560,6 +561,7 @@ void Modules::verify_archived_modules() {
 }
 
 char* Modules::_archived_main_module_name = nullptr;
+char* Modules::_archived_addmods_names = nullptr;
 
 void Modules::dump_main_module_name() {
   const char* module_name = Arguments::get_property("jdk.module.main");
@@ -600,6 +602,100 @@ void Modules::serialize(SerializeClosure* soc) {
   }
 }
 
+void Modules::dump_addmods_names() {
+  unsigned int count = Arguments::addmods_count();
+  const char* addmods_names = get_addmods_names_as_sorted_string();
+  if (addmods_names != nullptr) {
+    _archived_addmods_names = ArchiveBuilder::current()->ro_strdup(addmods_names);
+  }
+  ArchivePtrMarker::mark_pointer(&_archived_addmods_names);
+}
+
+void Modules::serialize_addmods_names(SerializeClosure* soc) {
+  soc->do_ptr(&_archived_addmods_names);
+  if (soc->reading()) {
+    bool disable = false;
+    if (_archived_addmods_names[0] != '\0') {
+      if (Arguments::addmods_count() == 0) {
+        log_info(cds)("--add-modules module name(s) found in archive but not specified during runtime: %s",
+            _archived_addmods_names);
+        disable = true;
+      } else {
+        const char* addmods_names = get_addmods_names_as_sorted_string();
+        if (strcmp((const char*)_archived_addmods_names, addmods_names) != 0) {
+          log_info(cds)("Mismatched --add-modules module name(s).");
+          log_info(cds)("  dump time: %s runtime: %s", _archived_addmods_names, addmods_names);
+          disable = true;
+        }
+      }
+    } else {
+      if (Arguments::addmods_count() > 0) {
+        log_info(cds)("--add-modules module name(s) specified during runtime but not found in archive: %s",
+                      get_addmods_names_as_sorted_string());
+        disable = true;
+      }
+    }
+    if (disable) {
+      log_info(cds)("Disabling optimized module handling");
+      CDSConfig::stop_using_optimized_module_handling();
+    }
+    log_info(cds)("optimized module handling: %s", CDSConfig::is_using_optimized_module_handling() ? "enabled" : "disabled");
+    log_info(cds)("full module graph: %s", CDSConfig::is_using_full_module_graph() ? "enabled" : "disabled");
+  }
+}
+
+const char* Modules::get_addmods_names_as_sorted_string() {
+  ResourceMark rm;
+  const int max_digits = 3;
+  const int extra_symbols_count = 2; // includes '.', '\0'
+  size_t prop_len = strlen("jdk.module.addmods") + max_digits + extra_symbols_count;
+  char* prop_name = resource_allocate_bytes(prop_len);
+  GrowableArray<const char*> list;
+  for (unsigned int i = 0; i < Arguments::addmods_count(); i++) {
+    jio_snprintf(prop_name, prop_len, "jdk.module.addmods.%d", i);
+    const char* prop_value = Arguments::get_property(prop_name);
+    char* p = resource_allocate_bytes(strlen(prop_value) + 1);
+    strcpy(p, prop_value);
+    while (*p == ',') p++; // skip leading commas
+    while (*p) {
+      char* next = strchr(p, ',');
+      if (next == nullptr) {
+        // no more commas, p is the last element
+        list.append(p);
+        break;
+      } else {
+        *next = 0;
+        list.append(p);
+        p = next + 1;
+      }
+    }
+  }
+
+  // Example:
+  // --add-modules=java.compiler --add-modules=java.base,java.base,,
+  //
+  // list[0] = "java.compiler"
+  // list[1] = "java.base"
+  // list[2] = "java.base"
+  // list[3] = ""
+  // list[4] = ""
+  list.sort(ClassLoaderExt::compare_module_names);
+
+  const char* prefix = "";
+  stringStream st;
+  const char* last_string = ""; // This also filters out all empty strings
+  for (int i = 0; i < list.length(); i++) {
+    const char* m = list.at(i);
+    if (strcmp(m, last_string) != 0) { // filter out duplicates
+      st.print("%s%s", prefix, m);
+      last_string = m;
+      prefix = "\n";
+    }
+  }
+
+  return (const char*)os::strdup(st.as_string()); // Example: "java.base,java.compiler"
+}
+
 void Modules::define_archived_modules(Handle h_platform_loader, Handle h_system_loader, TRAPS) {
   assert(CDSConfig::is_using_full_module_graph(), "must be");
 

src/hotspot/share/classfile/modules.hpp

@@ -61,9 +61,13 @@ class Modules : AllStatic {
   static void verify_archived_modules() NOT_CDS_JAVA_HEAP_RETURN;
   static void dump_main_module_name() NOT_CDS_JAVA_HEAP_RETURN;
   static void serialize(SerializeClosure* soc) NOT_CDS_JAVA_HEAP_RETURN;
+  static void dump_addmods_names() NOT_CDS_JAVA_HEAP_RETURN;
+  static void serialize_addmods_names(SerializeClosure* soc) NOT_CDS_JAVA_HEAP_RETURN;
+  static const char* get_addmods_names_as_sorted_string() NOT_CDS_JAVA_HEAP_RETURN_(nullptr);
 
 #if INCLUDE_CDS_JAVA_HEAP
   static char* _archived_main_module_name;
+  static char* _archived_addmods_names;
 #endif
 
   // Provides the java.lang.Module for the unnamed module defined

src/hotspot/share/runtime/arguments.cpp

@@ -85,6 +85,7 @@ char** Arguments::_jvm_flags_array              = nullptr;
 int    Arguments::_num_jvm_flags                = 0;
 char** Arguments::_jvm_args_array               = nullptr;
 int    Arguments::_num_jvm_args                 = 0;
+unsigned int Arguments::_addmods_count          = 0;
 char*  Arguments::_java_command                 = nullptr;
 SystemProperty* Arguments::_system_properties   = nullptr;
 size_t Arguments::_conservative_max_heap_alignment = 0;
@@ -336,6 +337,10 @@ bool Arguments::is_internal_module_property(const char* property) {
   return false;
 }
 
+bool Arguments::is_add_modules_property(const char* key) {
+  return (strcmp(key, MODULE_PROPERTY_PREFIX ADDMODS) == 0);
+}
+
 // Return true if the key matches the --module-path property name ("jdk.module.path").
 bool Arguments::is_module_path_property(const char* key) {
   return (strcmp(key, MODULE_PROPERTY_PREFIX PATH) == 0);
@@ -1773,7 +1778,6 @@ bool Arguments::sun_java_launcher_is_altjvm() {
 unsigned int addreads_count = 0;
 unsigned int addexports_count = 0;
 unsigned int addopens_count = 0;
-unsigned int addmods_count = 0;
 unsigned int patch_mod_count = 0;
 unsigned int enable_native_access_count = 0;
 
@@ -1799,7 +1803,7 @@ bool Arguments::check_vm_args_consistency() {
     PropertyList_unique_add(&_system_properties, "jdk.internal.vm.ci.enabled", "true",
         AddProperty, UnwriteableProperty, InternalProperty);
     if (ClassLoader::is_module_observable("jdk.internal.vm.ci")) {
-      if (!create_numbered_module_property("jdk.module.addmods", "jdk.internal.vm.ci", addmods_count++)) {
+      if (!create_numbered_module_property("jdk.module.addmods", "jdk.internal.vm.ci", _addmods_count++)) {
         return false;
       }
     }
@@ -1808,7 +1812,7 @@ bool Arguments::check_vm_args_consistency() {
 
 #if INCLUDE_JFR
   if (status && (FlightRecorderOptions || StartFlightRecording)) {
-    if (!create_numbered_module_property("jdk.module.addmods", "jdk.jfr", addmods_count++)) {
+    if (!create_numbered_module_property("jdk.module.addmods", "jdk.jfr", _addmods_count++)) {
       return false;
     }
   }
@@ -2235,7 +2239,7 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
         return JNI_ENOMEM;
       }
     } else if (match_option(option, "--add-modules=", &tail)) {
-      if (!create_numbered_module_property("jdk.module.addmods", tail, addmods_count++)) {
+      if (!create_numbered_module_property("jdk.module.addmods", tail, _addmods_count++)) {
         return JNI_ENOMEM;
       }
     } else if (match_option(option, "--enable-native-access=", &tail)) {
@@ -2322,7 +2326,7 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
         FREE_C_HEAP_ARRAY(char, options);
 
         // java agents need module java.instrument
-        if (!create_numbered_module_property("jdk.module.addmods", "java.instrument", addmods_count++)) {
+        if (!create_numbered_module_property("jdk.module.addmods", "java.instrument", _addmods_count++)) {
           return JNI_ENOMEM;
         }
       }
@@ -2503,7 +2507,7 @@ jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_m
           return JNI_EINVAL;
         }
         // management agent in module jdk.management.agent
-        if (!create_numbered_module_property("jdk.module.addmods", "jdk.management.agent", addmods_count++)) {
+        if (!create_numbered_module_property("jdk.module.addmods", "jdk.management.agent", _addmods_count++)) {
           return JNI_ENOMEM;
         }
 #else

src/hotspot/share/runtime/arguments.hpp

@@ -196,6 +196,8 @@ class Arguments : AllStatic {
   static int    _num_jvm_args;
   // string containing all java command (class/jarfile name and app args)
   static char* _java_command;
+  // number of unique modules specified in the --add-modules option
+  static unsigned int _addmods_count;
 
   // Property list
   static SystemProperty* _system_properties;
@@ -461,6 +463,8 @@ class Arguments : AllStatic {
   static int  PropertyList_readable_count(SystemProperty* pl);
 
   static bool is_internal_module_property(const char* option);
+  static bool is_add_modules_property(const char* key);
+  static unsigned int addmods_count() { return  _addmods_count; }
   static bool is_module_path_property(const char* key);
 
   // Miscellaneous System property value getter and setters.

src/java.base/share/classes/jdk/internal/module/ArchivedModuleGraph.java

@@ -25,6 +25,7 @@
 package jdk.internal.module;
 
 import java.util.Objects;
+import java.util.Set;
 import java.util.function.Function;
 import java.lang.module.Configuration;
 import java.lang.module.ModuleFinder;
@@ -43,19 +44,22 @@ class ArchivedModuleGraph {
     private final Configuration configuration;
     private final Function<String, ClassLoader> classLoaderFunction;
     private final String mainModule;
+    private final Set<String> addModules;
 
     private ArchivedModuleGraph(boolean hasSplitPackages,
                                 boolean hasIncubatorModules,
                                 ModuleFinder finder,
                                 Configuration configuration,
                                 Function<String, ClassLoader> classLoaderFunction,
-                                String mainModule) {
+                                String mainModule,
+                                Set<String> addModules) {
         this.hasSplitPackages = hasSplitPackages;
         this.hasIncubatorModules = hasIncubatorModules;
         this.finder = finder;
         this.configuration = configuration;
         this.classLoaderFunction = classLoaderFunction;
         this.mainModule = mainModule;
+        this.addModules = addModules;
     }
 
     ModuleFinder finder() {
@@ -78,12 +82,24 @@ boolean hasIncubatorModules() {
         return hasIncubatorModules;
     }
 
+    static boolean sameAddModules(Set<String> addModules) {
+        if (archivedModuleGraph.addModules == null || addModules == null) {
+            return false;
+        }
+
+        if (archivedModuleGraph.addModules.size() != addModules.size()) {
+            return false;
+        }
+
+        return archivedModuleGraph.addModules.containsAll(addModules);
+    }
+
     /**
      * Returns the ArchivedModuleGraph for the given initial module.
      */
-    static ArchivedModuleGraph get(String mainModule) {
+    static ArchivedModuleGraph get(String mainModule, Set<String> addModules) {
         ArchivedModuleGraph graph = archivedModuleGraph;
-        if ((graph != null) && Objects.equals(graph.mainModule, mainModule)) {
+        if ((graph != null) && Objects.equals(graph.mainModule, mainModule) && sameAddModules(addModules)) {
             return graph;
         } else {
             return null;
@@ -98,13 +114,15 @@ static void archive(boolean hasSplitPackages,
                         ModuleFinder finder,
                         Configuration configuration,
                         Function<String, ClassLoader> classLoaderFunction,
-                        String mainModule) {
+                        String mainModule,
+                        Set<String> addModules) {
         archivedModuleGraph = new ArchivedModuleGraph(hasSplitPackages,
                                                       hasIncubatorModules,
                                                       finder,
                                                       configuration,
                                                       classLoaderFunction,
-                                                      mainModule);
+                                                      mainModule,
+                                                      addModules);
     }
 
     static {

src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java

@@ -141,7 +141,6 @@ public static ModuleFinder limitedFinder() {
     private static boolean canUseArchivedBootLayer() {
         return getProperty("jdk.module.upgrade.path") == null &&
                getProperty("jdk.module.patch.0") == null &&       // --patch-module
-               getProperty("jdk.module.addmods.0") == null  &&    // --add-modules
                getProperty("jdk.module.limitmods") == null &&     // --limit-modules
                getProperty("jdk.module.addreads.0") == null &&    // --add-reads
                getProperty("jdk.module.addexports.0") == null &&  // --add-exports
@@ -212,10 +211,9 @@ private static ModuleLayer boot2() {
         // If the java heap was archived at CDS dump time, and the environment
         // at dump time matches the current environment, then use the archived
         // system modules and finder.
-        ArchivedModuleGraph archivedModuleGraph = ArchivedModuleGraph.get(mainModule);
+        ArchivedModuleGraph archivedModuleGraph = ArchivedModuleGraph.get(mainModule, addModules);
         if (archivedModuleGraph != null
                 && !haveModulePath
-                && addModules.isEmpty()
                 && limitModules.isEmpty()
                 && !isPatched) {
             systemModuleFinder = archivedModuleGraph.finder();
@@ -466,7 +464,6 @@ private static ModuleLayer boot2() {
 
         if (CDS.isDumpingStaticArchive()
                 && !haveUpgradeModulePath
-                && addModules.isEmpty()
                 && allJrtOrModularJar(cf)) {
             assert !isPatched;
 
@@ -478,7 +475,8 @@ && allJrtOrModularJar(cf)) {
                                         systemModuleFinder,
                                         cf,
                                         clf,
-                                        mainModule);
+                                        mainModule,
+                                        addModules);
             if (!hasSplitPackages && !hasIncubatorModules) {
                 ArchivedBootLayer.archive(bootLayer);
             }