[GitHub] sdedic commented on a change in pull request #844: Enforce public packages and friends for dependencies on bootstrap mod…

[GitBox <gi...@apache.org>]

sdedic commented on a change in pull request #844: Enforce public packages and friends for dependencies on bootstrap mod…
URL: https://github.com/apache/incubator-netbeans/pull/844#discussion_r215532665
 
 

 ##########
 File path: platform/o.n.bootstrap/src/org/netbeans/ModuleManager.java
 ##########
 @@ -906,11 +931,71 @@ void refineModulePath(Module m, List<File> path) {
             path.addAll(allJars);
         }
     }
-    
-    /** Use by OneModuleClassLoader to communicate with the ModuleInstaller re. masking. */
+
+    /** Use by OneModuleClassLoader to communicate with the ModuleInstaller re. masking. 
+     * @deprecated Use {@link #shouldDelegateResource(org.netbeans.Module, org.netbeans.Module, java.lang.String, java.lang.ClassLoader)}.
+     */
+    @Deprecated
     public boolean shouldDelegateResource(Module m, Module parent, String pkg) {
+        return shouldDelegateResource(m, parent, pkg, null);
+    }
+    
+    /**
+     * Determines if module `m' should delegate loading resources from package `p' to the
+     * `parent'. The parent is identified either by module specification (parent) or by a classloader
+     * which should load the package. For system or bootstrap classes, `parent' may be {@code null}, since
+     * boostrap classloaders load more modules together.
+     * <p/>
+     * If <b>both</b> `parent' and `ldr' are {@code null}, access to system/application classpath will be checked.
+     * 
+     * @param m module that attempts to load resources.
+     * @param parent parent classloader which may eventually load the resource, could be {@code null} to indicate bootstrap or system class
+     * @param pkg package (folder) with the resource
+     * @param ldr the classloader which should load the resource; may be {@code null}
+     * @return true, if the loading should be delegated to the classloader
+     * @since 
+     */
+    public boolean shouldDelegateResource(Module m, Module parent, String pkg, ClassLoader ldr) {
         // Cf. #19621:
-        Module.PackageExport[] exports = (parent == null) ? null : parent.getPublicPackages();
+        Module.PackageExport[] exports;
+        if (parent != null) {
+            exports = parent.getPublicPackages();
+        } else if (ldr != null) {
+            Collection<Module> loaderMods = null;
+            synchronized (this) {
 
 Review comment:
   I should have measured. I made some patches, measuring # of calls and time spend using `nanoTime`. Absolute times below are in microseconds.
   
   I compared the extra time needed for checks to the total time spent in this `shouldDelegate` function; during startup there are ~677 (bare platform) or ~892 (java cluster enabled) invocations, with 21% - 30% time spent in the new access checks. The total time spent in the new checks on startup is ~10kusec = 10msec. 
   
   I also printed the counters during shutdown: start pristine IDE, load java project (+ enable java cluster), let the scan to finish, open editor. Numbers for the whole IDE run:
   ```
   Number of bootstrap accesses: 3333
   Extra time for bootstrap restrictions: 19065
   ```
   (times in usec)
   
   I guess I can throw away the patches as the time impact seems negligible.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists