Merge pull request #436 from apache/refactoring/435-Improve-performan…

…ce-of-ImportResolver

Issue #435: improve performance of import resolver

.github/workflows/maven.yml

@@ -0,0 +1,67 @@
+# This workflow will build a Java project with Maven, and cache/restore any dependencies to improve the workflow execution time
+# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-java-with-maven
+
+# This workflow uses actions that are not certified by GitHub.
+# They are provided by a third-party and are governed by
+# separate terms of service, privacy policy, and support
+# documentation.
+
+name: Java CI with Maven
+
+on:
+  push:
+    branches: [ "main", "release/**" ]
+  pull_request:
+    branches: [ "main", "release/**" ]
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        language: [ 'java' ]
+        os: [ubuntu-latest, windows-latest]
+        jdk: [17]
+
+    runs-on: ${{ matrix.os }}
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+      with:
+        ref: ${{ github.head_ref || github.ref }}
+
+    - name: Set up JDK ${{ matrix.jdk }}
+      uses: actions/setup-java@v4
+      with:
+        java-version: ${{ matrix.jdk }}
+        distribution: 'temurin'
+        cache: maven
+        server-id: apache.snapshots.https
+        server-username: NEXUS_USERNAME
+        server-password: NEXUS_PASSWORD
+
+    - name: Set up Maven
+      uses: stCarolas/setup-maven@v5
+      with:
+        maven-version: 3.9.9
+
+    - name: Set up cache date
+      run: echo "CACHE_DATE=$(date +'%Y-%m-%d')" >> $GITHUB_ENV
+
+    - name: Build with Maven
+      run: mvn --show-version --batch-mode --no-transfer-progress clean verify
+
+    - name: Publish Test Report
+      uses: mikepenz/action-junit-report@v5
+      if: success() || failure() # always run even if the previous step fails
+      with:
+        report_paths: '**/target/surefire-reports/TEST-*.xml'
+
+    - name: Upload to Nexus
+      if: matrix.os == 'ubuntu-latest' && github.event_name != 'pull_request'
+      env:
+        # `NEXUS_USERNAME` and `NEXUS_PASSWORD` are used in `~/.m2/settings.xml` created by `setup-java` action
+        NEXUS_USERNAME: ${{ secrets.NEXUS_USER }}
+        NEXUS_PASSWORD: ${{ secrets.NEXUS_PW }}
+      run: mvn --show-version --batch-mode --errors --no-transfer-progress -DskipTests deploy

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

@@ -21,6 +21,9 @@
 import java.lang.invoke.MethodHandles;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.ServiceConfigurationError;
 import java.util.ServiceLoader;
 import java.util.Spliterator;
@@ -35,6 +38,15 @@ public class ServiceLoaderUtil {
   private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
   private static final int MAX_BROKEN_SERVICES = 16;
 
+  private static final WeakIdentityMap<ClassLoader, Map<Class<?>, List<?>>> cl_to_services = //
+          WeakIdentityMap.newHashMap();
+
+  public static void clearServiceCache() {
+    synchronized (cl_to_services) {
+      cl_to_services.clear();
+    }
+  }
+
   public static <T> Stream<T> loadServicesSafely(Class<T> aService) {
     var cl = ClassLoaderUtils.findClassLoader();
     return loadServicesSafely(aService, cl, null);
@@ -46,9 +58,29 @@ public static <T> Stream<T> loadServicesSafely(Class<T> aService, ClassLoader aC
 
   static <T> Stream<T> loadServicesSafely(Class<T> aService, ClassLoader aClassLoader,
           Collection<Throwable> aErrorCollector) {
-    var loader = ServiceLoader.load(aService, aClassLoader);
-    return StreamSupport.stream(
-            new ServiceLoaderSpliterator<T>(aService, loader.iterator(), aErrorCollector), false);
+
+    Map<Class<?>, List<?>> servicesMap;
+
+    synchronized (cl_to_services) {
+      servicesMap = cl_to_services.get(aClassLoader);
+      if (servicesMap == null) {
+        servicesMap = new LinkedHashMap<>();
+        cl_to_services.put(aClassLoader, servicesMap);
+      }
+    }
+
+    synchronized (servicesMap) {
+      @SuppressWarnings("unchecked")
+      var services = (List<T>) servicesMap.get(aService);
+      if (services == null) {
+        var loader = ServiceLoader.load(aService, aClassLoader);
+        services = StreamSupport.stream(
+                new ServiceLoaderSpliterator<T>(aService, loader.iterator(), aErrorCollector),
+                false).toList();
+        servicesMap.put(aService, services);
+      }
+      return services.stream();
+    }
   }
 
   private static class ServiceLoaderSpliterator<T> implements Spliterator<T> {

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

@@ -30,6 +30,7 @@
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.net.MalformedURLException;
+import java.net.URI;
 import java.net.URL;
 import java.net.URLDecoder;
 import java.util.ArrayList;
@@ -226,24 +227,75 @@ public void setDataPath(String aPath) throws MalformedURLException {
 
   @Override
   public URL resolveRelativePath(String aPathOrUrl) {
-    // check if an URL was passed in - if so, we fall back to the old logic which is a bit odd
-    // because for relative URLs, it basically discards the protocol. This is behavior we may
-    // want to change on the next major release... e.g. to require that relative paths are
-    // always specified without a protocol.
+    if (aPathOrUrl == null) {
+      return null;
+    }
+
+    URI uri = null;
     try {
-      var url = new URL(aPathOrUrl);
-      return resolveRelativePath(url);
-    } catch (MalformedURLException e) {
-      // ignore and move on
+      // Try parsing as URI as this is less overhead than parsing as URL
+      uri = URI.create(aPathOrUrl);
+    } catch (Exception e) {
+      // Location is not a URI (and consequently not a URL)
     }
 
-    // try each base URL
+    if (uri != null && uri.isAbsolute() && uri.getPath() != null
+            && !"path".equals(uri.getScheme())) {
+      try {
+        if ("file".equals(uri.getScheme())) {
+          // Try faster short-cut when using a file location avoiding creation of URL object if
+          // not necessary
+          if (new File(uri.getPath()).exists()) {
+            return new URL(aPathOrUrl);
+          }
+        } else {
+          var absUrl = new URL(aPathOrUrl);
+          // if file exists here, return this URL
+          if (fileExistsAtUrl(absUrl)) {
+            return absUrl;
+          }
+        }
+      } catch (MalformedURLException e) {
+        // Not found
+        return null;
+      }
+    }
+
+    String scheme;
+    String relativePath;
+    if (uri != null && uri.isAbsolute()) {
+      // If we have a URI like `file:some/path/res.xml` this counts as absolute but we won't get a
+      // path, only a scheme-specific part. We want to treat these cases as relative.
+      scheme = uri.getScheme();
+      relativePath = uri.getSchemeSpecificPart();
+    } else {
+      scheme = null;
+      relativePath = aPathOrUrl;
+    }
+
+    if (relativePath.startsWith("/")) {
+      relativePath = relativePath.substring(1);
+    }
+
+    // Try resolving relative locations against the base URLs
     for (var baseUrl : mBaseUrls) {
+      if (scheme != null && !"path".equals(scheme) && !scheme.equals(baseUrl.getProtocol())) {
+        continue;
+      }
+
       try {
-        var absUrl = new URL(baseUrl, aPathOrUrl);
-        // if file exists here, return this URL
-        if (fileExistsAtUrl(absUrl)) {
-          return absUrl;
+        if ("file".equals(baseUrl.getProtocol())) {
+          // Try faster short-cut when using a file location avoiding creation of URL object if
+          // not necessary
+          if (new File(baseUrl.getPath(), relativePath).exists()) {
+            return new URL(baseUrl, relativePath);
+          }
+        } else {
+          var absUrl = new URL(baseUrl, relativePath);
+          // if file exists here, return this URL
+          if (fileExistsAtUrl(absUrl)) {
+            return absUrl;
+          }
         }
       } catch (MalformedURLException e) {
         // ignore and move on to next base URL
@@ -252,19 +304,23 @@ public URL resolveRelativePath(String aPathOrUrl) {
 
     // fallback on classloader
     URL absURL = null;
-    if (mClassLoader != null) {
-      absURL = mClassLoader.getResource(aPathOrUrl);
-    }
 
-    // fallback on TCCL
-    if (absURL == null) {
-      var tccl = Thread.currentThread().getContextClassLoader();
-      absURL = tccl.getResource(aPathOrUrl);
-    }
+    if (scheme == null || "path".equals(scheme)) {
+      if (mClassLoader != null) {
+        absURL = mClassLoader.getResource(relativePath);
+      }
 
-    // if no ClassLoader specified (could be the bootstrap classloader), try the system classloader
-    if (absURL == null && mClassLoader == null) {
-      absURL = ClassLoader.getSystemClassLoader().getResource(aPathOrUrl);
+      // fallback on TCCL
+      if (absURL == null) {
+        var tccl = Thread.currentThread().getContextClassLoader();
+        absURL = tccl.getResource(relativePath);
+      }
+
+      // if no ClassLoader specified (could be the bootstrap classloader), try the system
+      // classloader
+      if (absURL == null && mClassLoader == null) {
+        absURL = ClassLoader.getSystemClassLoader().getResource(relativePath);
+      }
     }
 
     return absURL;
@@ -276,10 +332,18 @@ public URL resolveRelativePath(URL aUrl) {
     // try each base URL
     for (var baseUrl : mBaseUrls) {
       try {
-        var absUrl = new URL(baseUrl, aUrl.toString());
-        // if file exists here, return this URL
-        if (fileExistsAtUrl(absUrl)) {
-          return absUrl;
+        if ("file".equals(baseUrl.getProtocol())) {
+          // Try faster short-cut when using a file location avoiding creation of URL object if not
+          // necessary
+          if (new File(baseUrl.getPath(), aUrl.getPath()).exists()) {
+            return new URL(baseUrl, aUrl.getPath());
+          }
+        } else {
+          var absUrl = new URL(baseUrl, aUrl.getPath());
+          // if file exists here, return this URL
+          if (fileExistsAtUrl(absUrl)) {
+            return absUrl;
+          }
         }
       } catch (MalformedURLException e) {
         // ignore and move on to next base URL
@@ -324,11 +388,19 @@ public void setPathResolverClassLoader(ClassLoader aClassLoader) {
    * Utility method that checks to see if a file exists at the specified URL.
    */
   protected boolean fileExistsAtUrl(URL aUrl) {
+    if (aUrl == null) {
+      return false;
+    }
+
+    if ("file".equals(aUrl.getProtocol())) {
+      return new File(aUrl.getPath()).exists();
+    }
+
     try {
       // Ensure that we actually always check the resource for existence. In case of a JAR URL,
       // this is also important to ensure that the ZIP/JAR file is closed again.
       var connection = aUrl.openConnection();
-      connection.setDefaultUseCaches(false);
+      connection.setUseCaches(false);
       try (var testStream = connection.getInputStream()) {
         return true;
       }

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

@@ -501,7 +501,7 @@ public Class<?> getResourceClass(String aName) {
   }
 
   @Override
-  public InputStream getResourceAsStream(String aKey, String[] aParams)
+  public InputStream getResourceAsStream(String aKey, String... aParams)
           throws ResourceAccessException {
     return getResourceAsStreamCommon(getResource(aKey, aParams));
   }
@@ -533,7 +533,7 @@ private URL getResourceAsStreamCommonUrl(Object resource) {
   }
 
   @Override
-  public URL getResourceURL(String aKey, String[] aParams) throws ResourceAccessException {
+  public URL getResourceURL(String aKey, String... aParams) throws ResourceAccessException {
     return getResourceAsStreamCommonUrl(getResource(aKey, aParams));
   }