RFR (M): JDK-8159262: Walking PackageEntry Export and ModuleEntry Reads Must Occur Only When Neccessary And Wait Until ClassLoader's Aliveness Determined

[David Holmes]

On 23/06/2016 9:25 PM, Alan Bateman wrote:
>
>
> On 23/06/2016 12:05, David Holmes wrote:
>>
>> You said:
>>
>> "Module system initialization will initialize the built-in class
>> loaders, including the application class loader. The platform or
>> application class loaders won't load any classes in this phase of
>> course but they will be initialized (with the modules that they might
>> later load classes from). "
>>
>> What does that last part mean: "with the modules they might later load
>> classes from"? How have modules become bound to a classloader that may
>> not even get instantiated? If that class loader is never instantiated
>> then why do we need a runtime test that checks for the type of the
>> actual system class loader?
> The graph of modules that are defined to the runtime during startup are
> we call the "boot layer". They are statically mapped to the built-in
> class loaders where built-in means the boot, platform or application
> class loaders. There may be a custom system class loader that comes into
> existence later during the startup but it's just not interesting to the
> discussion here (except to know that it might be different to the
> application class loader in some niche configuration).

But you skipped the key part - why do we care about the built-in 
loaders. In this code:

+// If the module's loader, that a read edge is being established to, is
+// not the same loader as this module's and is not one of the 3 builtin
+// class loaders, then this module's reads list must be walked at GC
+// safepoint. Modules have the same life cycle as their defining class
+// loaders and should be removed if dead.
+void ModuleEntry::set_read_walk_required(ClassLoaderData* m_loader_data) {
+  assert_locked_or_safepoint(Module_lock);
+  if (!_must_walk_reads &&
+      loader_data() != m_loader_data &&
+      !m_loader_data->is_builtin_class_loader_data()) {
+    _must_walk_reads = true;
+    if (log_is_enabled(Trace, modules)) {
+      ResourceMark rm;
+      log_trace(modules)("ModuleEntry::set_read_walk_required(): module 
%s reads list must be walked",
+                         (name() != NULL) ? name()->as_C_string() : 
UNNAMED_MODULE);
+    }
+  }
+}

what is "is_builtin_class_loader_data" really asking? Is it just that 
this is a loader whose lifetime is matched to that of the VM? If so then 
it is asking the wrong question with respect to the system loader. If 
not, then what is it about the built-in system loader type that makes it 
different to some custom system loader?

Thanks,
David

(PS. Really calling it a night now :) )

> -Alan