Merge pull request #433 from apache/refactoring/430-Resolving-type-system-imports-through-SPIs-slows-things-down-too-much

Issue #430: Resolving type system imports through sp is slows things down too much

Author: reckart

.asf.yaml

@@ -15,17 +15,17 @@
 # limitations under the License.
 #
 
+notifications:
+  commits:      [email protected]
+  issues:       [email protected]
+  pullrequests: [email protected]
+  jobs:         [email protected]
+  jira_options: link label
 github:
   description: "Apache UIMA Java SDK"
   homepage: https://uima.apache.org
   dependabot_alerts:  true
   dependabot_updates: false
-  notifications:
-    commits:      [email protected]
-    issues:       [email protected]
-    pullrequests: [email protected]
-    jobs:         [email protected]
-    jira_options: link label
   labels:
   - apache
   - uima

uima-bnd-plugin/src/main/java/org/apache/uima/tools/bnd/UimaBndPlugin.java

@@ -24,6 +24,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 import java.util.regex.Pattern;
 
 import org.apache.uima.UIMAFramework;
@@ -44,12 +45,14 @@
 import aQute.bnd.osgi.Analyzer;
 import aQute.bnd.osgi.Resource;
 import aQute.bnd.service.AnalyzerPlugin;
+import aQute.bnd.service.Plugin;
+import aQute.lib.converter.Converter;
+import aQute.service.reporter.Reporter;
 
-@BndPlugin(name = "UIMA")
+@BndPlugin(name = "UIMA", parameters = UimaBndPlugin.Configuration.class)
 public class UimaBndPlugin
-    implements AnalyzerPlugin
+    implements AnalyzerPlugin, Plugin
 {
-
     private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
     private static final Pattern XML_FILE = Pattern.compile(".*\\.xml");
@@ -58,6 +61,21 @@ public class UimaBndPlugin
 
     private final static XMLParser PARSER = UIMAFramework.getXMLParser();
 
+    private Reporter reporter;
+    private UimaBndPlugin.Configuration configuration;
+
+    @Override
+    public void setProperties(Map<String, String> aMap) throws Exception
+    {
+        configuration = Converter.cnv(UimaBndPlugin.Configuration.class, aMap);
+    }
+
+    @Override
+    public void setReporter(Reporter aReporter)
+    {
+        reporter = aReporter;
+    }
+
     @Override
     public boolean analyzeJar(Analyzer analyzer) throws Exception
     {
@@ -69,37 +87,37 @@ public boolean analyzeJar(Analyzer analyzer) throws Exception
             for (var entry : resources.entrySet()) {
                 var path = entry.getKey();
                 var resource = entry.getValue();
-    
+
                 try {
                     if (XML_FILE.matcher(path).matches()) {
-                        importsProcessed += analyzeXmlFile(analyzer, path, resource);
+                        importsProcessed += analyzeXmlFile(analyzer, path, resource, 0);
                     }
                 }
                 catch (Exception e) {
                     analyzer.error("Unexpected exception in processing resource (%s): %s", path, e);
                 }
             }
         }
-        
+
         LOG.info("UIMA bnd plugin processed {} imports", importsProcessed);
 
         return false;
     }
 
-    private int analyzeXmlFile(Analyzer analyzer, String path, Resource resource) throws Exception
+    private int analyzeXmlFile(Analyzer analyzer, String path, Resource resource, int level) throws Exception
     {
         var desc = readUimaDescriptor(resource);
         if (desc == null) {
             return 0;
         }
 
-        LOG.debug("Found {}: {}", desc.getClass().getSimpleName(), path);
+        LOG.debug("{}Found {}: {}", repeat("  ", level), desc.getClass().getSimpleName(), path);
         var imports = getImportsFromDescriptor(desc);
 
         var importsProcessed = 0;
         for (var imp : imports) {
             if (imp.getName() != null) {
-                handleImportByName(analyzer, path, imp);
+                handleImportByName(analyzer, path, imp, level);
                 importsProcessed++;
                 continue;
             }
@@ -108,22 +126,21 @@ private int analyzeXmlFile(Analyzer analyzer, String path, Resource resource) th
                 handleImportByLocation(imp);
                 continue;
             }
-            
+
             LOG.warn(
-                    "Found UIMA type system import without name and location - ignoring, please fix your type system description");
+                    "Found UIMA import without name and location - ignoring, please fix your type system description");
         }
-        
+
         return importsProcessed;
     }
 
     private void handleImportByLocation(Import imp)
     {
-        LOG.warn(
-                "Found UIMA type system import by location: {} - ignoring, please only use import-by-name",
+        LOG.warn("Ignoring UIMA import by location (please only use import-by-name): {}",
                 imp.getLocation());
     }
 
-    private void handleImportByName(Analyzer analyzer, String path, Import imp)
+    private void handleImportByName(Analyzer analyzer, String path, Import imp, int level) throws Exception
     {
         var tsdPackage = imp.getName();
         int lastSeparatorPosition = tsdPackage.lastIndexOf('.');
@@ -132,18 +149,29 @@ private void handleImportByName(Analyzer analyzer, String path, Import imp)
             tsdPackage = tsdPackage.substring(0, lastSeparatorPosition);
         }
 
-        LOG.debug("Found UIMA type system import by name: {}", tsdPackage);
-
         var pack = analyzer.getPackageRef(tsdPackage);
         if (!QN.matcher(pack.getFQN()).matches()) {
-            analyzer.warning("Type system import does not seem to refer to a package (%s): %s",
-                    path, pack);
+            analyzer.warning("Import does not seem to refer to a package (%s): %s", path, pack);
         }
 
-        if (!analyzer.getReferred().containsKey(pack)) {
+        var alreadyKnownImport = analyzer.getReferred().containsKey(pack);
+        if (!alreadyKnownImport) {
             var attrs = new Attrs();
             analyzer.getReferred().put(pack, attrs);
         }
+
+        LOG.debug("{}Found UIMA import by name: {} {}", repeat("  ", level), tsdPackage, alreadyKnownImport ? "" : "(new)");
+        
+        var importedResourcePath = imp.getName().replace('.', '/') + ".xml";
+        var importedResource = analyzer.findResource(importedResourcePath);
+        if (importedResource == null) {
+            analyzer.warning("Imported resource not found on classpath: {}", importedResourcePath);
+            return;
+        }
+
+        if (configuration == null || configuration.transitive(false)) {
+            analyzeXmlFile(analyzer, importedResourcePath, importedResource, level + 1);
+        }
     }
 
     private List<Import> getImportsFromDescriptor(XMLizable desc)
@@ -194,4 +222,22 @@ private static <T> List<T> asList(T[] aList)
 
         return Arrays.asList(aList);
     }
+    
+    private static String repeat(String aString, int aCount) {
+      if (aCount == 0) {
+        return "";
+      }
+      
+      var buf = new StringBuilder();
+      for (var i = 0; i < aCount; i++) {
+        buf.append(aString);
+      }
+
+      return buf.toString();
+    }
+
+    public static interface Configuration
+    {
+        boolean transitive(boolean aDefault);
+    }
 }

uimafit-maven-plugin/src/main/java/org/apache/uima/fit/maven/GenerateDescriptorsMojo.java

@@ -149,7 +149,7 @@ public void execute() throws MojoExecutionException {
     componentLoader = Util.getClassloader(project, includeScope);
 
     // List of components that is later written to META-INF/org.apache.uima.fit/components.txt
-    StringBuilder componentsManifest = new StringBuilder();
+    var componentsManifest = new StringBuilder();
 
     int countGenerated = 0;
     for (String file : files) {

uimaj-core/src/main/java/org/apache/uima/internal/util/ClassLoaderUtils.java

@@ -38,8 +38,8 @@ private ClassLoaderUtils() {
    * <ol>
    * <li>The {@link UimaContext} in the {@link UimaContextHolder} of the current thread(if any)</li>
    * <li>The current thread-context class loader (if any)</li>
-   * <li>The class loader through which uimaFIT (i.e. this class) was loaded.</li>
-   * <li>For backwards compatibility then delegates to {@link #getDefaultClassLoader()}</li>
+   * <li>The class loader through which UIMA was loaded.</li>
+   * <li>Finally checks the system classloader</li>
    * </ol>
    *
    * @return a class loader or {@code null} if no suitable class loader could be found.
@@ -60,7 +60,27 @@ public static ClassLoader findClassLoader() {
       return uimaFITClassLoader;
     }
 
-    return getDefaultClassLoader();
+    ClassLoader cl;
+
+    try {
+      cl = UIMAFramework.class.getClassLoader();
+      if (cl != null) {
+        return cl;
+      }
+    } catch (Throwable ex) {
+      // Fall-through
+    }
+
+    try {
+      cl = ClassLoader.getSystemClassLoader();
+      if (cl != null) {
+        return cl;
+      }
+    } catch (Throwable ex) {
+      // Fall-through
+    }
+
+    return null;
   }
 
   /**
@@ -122,36 +142,4 @@ private static ClassLoader getExtensionClassLoader(ResourceManager aResMgr) {
 
     return null;
   }
-
-  private static ClassLoader getDefaultClassLoader() {
-    ClassLoader cl;
-    try {
-      cl = Thread.currentThread().getContextClassLoader();
-      if (cl != null) {
-        return cl;
-      }
-    } catch (Throwable ex) {
-      // Fall-through
-    }
-
-    try {
-      cl = UIMAFramework.class.getClassLoader();
-      if (cl != null) {
-        return cl;
-      }
-    } catch (Throwable ex) {
-      // Fall-through
-    }
-
-    try {
-      cl = ClassLoader.getSystemClassLoader();
-      if (cl != null) {
-        return cl;
-      }
-    } catch (Throwable ex) {
-      // Fall-through
-    }
-
-    return null;
-  }
 }

uimaj-core/src/main/java/org/apache/uima/resource/RelativePathResolver.java

@@ -129,7 +129,7 @@ public interface RelativePathResolver {
    * @return the absolute URL at which the file exists, <code>null</code> it none could be found.
    * @deprecated Use {@link #resolveRelativePath(String)} instead.
    */
-  @Deprecated
+  @Deprecated(since = "3.6.0")
   URL resolveRelativePath(URL aUrl);
 
   /**

uimaj-core/src/main/java/org/apache/uima/resource/impl/RelativePathResolver_impl.java

@@ -37,6 +37,7 @@
 import java.util.List;
 import java.util.StringTokenizer;
 
+import org.apache.uima.UIMAFramework;
 import org.apache.uima.resource.RelativePathResolver;
 
 /**
@@ -59,6 +60,8 @@ public RelativePathResolver_impl() {
   }
 
   public RelativePathResolver_impl(ClassLoader aClassLoader) {
+    mClassLoader = aClassLoader;
+
     // initialize data path based on uima.datapath System property; if not
     // present fall back on user.dir
     String dataPath = null;
@@ -88,8 +91,6 @@ public RelativePathResolver_impl(ClassLoader aClassLoader) {
       mDataPath = null;
       mBaseUrls = null;
     }
-
-    mClassLoader = aClassLoader;
   }
 
   @Override
@@ -376,9 +377,6 @@ public URL resolveRelativePath(URL aUrl) {
     return absURL;
   }
 
-  /**
-   * @see org.apache.uima.resource.RelativePathResolver#setPathResolverClassLoader(java.lang.ClassLoader)
-   */
   @Override
   public void setPathResolverClassLoader(ClassLoader aClassLoader) {
     mClassLoader = aClassLoader;

uimaj-core/src/main/java/org/apache/uima/resource/impl/ResourceManager_impl.java

@@ -226,6 +226,7 @@ public ResourceRegistration(Object aResource, ExternalResourceDescription aDescr
    * Cache of imported descriptor URLs from which the parsed objects in importCache were created, so
    * that these URLs are not re-parsed if the same URL is imported again.
    */
+  @Deprecated(since = "3.3.0")
   private Map<String, Set<String>> importUrlsCache = Collections.synchronizedMap(new HashMap<>());
 
   /**