Fix for #3306: Speed up HistoryGuru.getRepository(File) call for Opengrok installations with many Repositories

   and deep file trees by caching previous lookup results for dir -> Repository.
   Additional refactorings to make this new functionality testable against legacy implementation:
      - Introduce Jimfs for testing which will allow testing Windows, MacOS or UNIX filesystem on any machine
      - Refactor PathUtils to use Paths instead of Files (this is needed to use Jimfs)

Author: azakkerman

opengrok-indexer/pom.xml

@@ -137,6 +137,12 @@ Portions Copyright (c) 2020-2020, Lubos Kosco <[email protected]>.
             <artifactId>junit-vintage-engine</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.hamcrest</groupId>
+            <artifactId>hamcrest-library</artifactId>
+            <version>${hamcrest.version}</version>
+            <scope>test</scope>
+        </dependency>
         <dependency> <!-- TODO: remove! (moving Messages to web module) -->
             <groupId>org.awaitility</groupId>
             <artifactId>awaitility</artifactId>
@@ -155,6 +161,12 @@ Portions Copyright (c) 2020-2020, Lubos Kosco <[email protected]>.
             <version>2.28.2</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.google.jimfs</groupId>
+            <artifactId>jimfs</artifactId>
+            <version>1.1</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>com.cronutils</groupId>
             <artifactId>cron-utils</artifactId>

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java

@@ -29,6 +29,7 @@
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -407,8 +408,8 @@ public String getPathRelativeToSourceRoot(File file)
             throw new FileNotFoundException("sourceRoot is not defined");
         }
 
-        String maybeRelPath = PathUtils.getRelativeToCanonical(file.getPath(),
-                sourceRoot, getAllowedSymlinks(), getCanonicalRoots());
+        String maybeRelPath = PathUtils.getRelativeToCanonical(file.toPath(),
+                Paths.get(sourceRoot), getAllowedSymlinks(), getCanonicalRoots());
         File maybeRelFile = new File(maybeRelPath);
         if (!maybeRelFile.isAbsolute()) {
             /*

opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java

@@ -28,14 +28,14 @@
 import java.io.InputStream;
 import java.lang.reflect.InvocationTargetException;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CountDownLatch;
@@ -89,6 +89,11 @@ public final class HistoryGuru {
      */
     private final Map<String, String> repositoryRoots = new ConcurrentHashMap<>();
 
+    /**
+     * Interface to perform repository lookup for a given file path and HistoryGuru state.
+     */
+    private final RepositoryLookup repositoryLookup;
+
     /**
      * Creates a new instance of HistoryGuru, and try to set the default source
      * control system.
@@ -110,6 +115,7 @@ private HistoryGuru() {
             }
         }
         historyCache = cache;
+        repositoryLookup = RepositoryLookup.cached();
     }
 
     /**
@@ -733,38 +739,8 @@ private List<Repository> getReposFromString(Collection<String> repositories) {
         return repos;
     }
 
-    protected Repository getRepository(File path) {
-        File file = path;
-        Set<String> rootKeys = repositoryRoots.keySet();
-
-        while (file != null) {
-            String nextPath = file.getPath();
-            for (String rootKey : rootKeys) {
-                String rel;
-                try {
-                    rel = PathUtils.getRelativeToCanonical(nextPath, rootKey);
-                } catch (IOException e) {
-                    LOGGER.log(Level.WARNING,
-                        "Failed to get relative to canonical for " + nextPath,
-                        e);
-                    return null;
-                }
-                Repository repo;
-                if (rel.equals(nextPath)) {
-                    repo = repositories.get(nextPath);
-                } else {
-                    String inRootPath = Paths.get(rootKey, rel).toString();
-                    repo = repositories.get(inRootPath);
-                }
-                if (repo != null) {
-                    return repo;
-                }
-            }
-
-            file = file.getParentFile();
-        }
-
-        return null;
+    protected Repository getRepository(File file) {
+        return repositoryLookup.getRepository(file.toPath(), repositoryRoots.keySet(), repositories, PathUtils::getRelativeToCanonical);
     }
 
     /**
@@ -774,10 +750,9 @@ protected Repository getRepository(File path) {
      * @param repos repository paths
      */
     public void removeRepositories(Collection<String> repos) {
-        for (String repo : repos) {
-            repositories.remove(repo);
-        }
-
+        Set<Repository> removedRepos = repos.stream().map(repositories::remove)
+            .filter(Objects::nonNull).collect(Collectors.toSet());
+        repositoryLookup.repositoriesRemoved(removedRepos);
         // Re-map the repository roots.
         repositoryRoots.clear();
         List<Repository> ccopy = new ArrayList<>(repositories.values());
@@ -825,8 +800,7 @@ public void invalidateRepositories(Collection<? extends RepositoryInfo> repos, L
      */
     public void invalidateRepositories(Collection<? extends RepositoryInfo> repos, CommandTimeoutType cmdType) {
         if (repos == null || repos.isEmpty()) {
-            repositoryRoots.clear();
-            repositories.clear();
+            clear();
             return;
         }
 
@@ -885,14 +859,19 @@ public void run() {
         }
         executor.shutdown();
 
-        repositoryRoots.clear();
-        repositories.clear();
+        clear();
         newrepos.forEach((_key, repo) -> putRepository(repo));
 
         elapsed.report(LOGGER, String.format("done invalidating %d repositories", newrepos.size()),
                 "history.repositories.invalidate");
     }
 
+    private void clear() {
+        repositoryRoots.clear();
+        repositories.clear();
+        repositoryLookup.clear();
+    }
+
     /**
      * Adds the specified {@code repository} to this instance's repository map
      * and repository-root map (if not already there).

opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryInfo.java

@@ -169,7 +169,7 @@ public void setDirectoryName(File dir) {
         String path;
         String originalPath = dir.getPath();
         try {
-            path = PathUtils.getRelativeToCanonical(originalPath, rootPath);
+            path = PathUtils.getRelativeToCanonical(dir.toPath(), Paths.get(rootPath));
             // OpenGrok has a weird convention that directoryNameRelative must start with a path separator,
             // as it is elsewhere directly appended to env.getSourceRootPath() and also stored as such.
             if (!path.equals(originalPath)) {

opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryLookup.java

@@ -0,0 +1,83 @@
+/*
+ * CDDL HEADER START
+ *
+ * The contents of this file are subject to the terms of the
+ * Common Development and Distribution License (the "License").
+ * You may not use this file except in compliance with the License.
+ *
+ * See LICENSE.txt included in this distribution for the specific
+ * language governing permissions and limitations under the License.
+ *
+ * When distributing Covered Code, include this CDDL HEADER in each
+ * file and include the License file at LICENSE.txt.
+ * If applicable, add the following below this CDDL HEADER, with the
+ * fields enclosed by brackets "[]" replaced with your own identifying
+ * information: Portions Copyright [yyyy] [name of copyright owner]
+ *
+ * CDDL HEADER END
+ */
+
+/*
+ * Copyright (c) 2020, Anatoly Akkerman <[email protected]>, <[email protected]>.
+ */
+package org.opengrok.indexer.history;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Interface for finding enclosing Repository for a given Path, used by HistoryGuru.
+ * <p>
+ * Two implementations exists
+ * - uncached, (legacy behavior) extracted from HistoryGuru into a stand-alone impl
+ * - cached, new implementation, which reduces number of expensive canonicalization calls
+ * <p>
+ * We preserve both cached and uncached implementations in order to verify correctness of the cached impl.
+ */
+public interface RepositoryLookup {
+
+    static RepositoryLookupCached cached() {
+        return new RepositoryLookupCached();
+    }
+
+    static RepositoryLookupUncached uncached() {
+        return new RepositoryLookupUncached();
+    }
+
+    /**
+     * This interface allows intercepting PathUtils.getRelativeToCanonical in order to measure the impact of caching.
+     *
+     * In practice, PathUtils::getRelativeToCanonical is the implementation of this interface
+     */
+    @FunctionalInterface
+    interface PathCanonicalizer {
+        String resolve(Path path, Path relativeTo) throws IOException;
+    }
+
+    /**
+     * Find enclosing repository for a given path.
+     *
+     * @param path path to find encolsing repository for
+     * @param repoParentDirs Set of repository parent dirs (parents of repository roots)
+     * @param repositories Map of repository root to Repository
+     * @param canonicalizer PathCanonicalizer reference
+     * @return enclosing Repository or null if not found
+     */
+    Repository getRepository(Path path, Set<String> repoParentDirs, Map<String, Repository> repositories,
+        PathCanonicalizer canonicalizer);
+
+    /**
+     * Lifecycle method to clear internal cache (if any) when repositories are invalidated.
+     */
+    void clear();
+
+    /**
+     * Lifecycle method to invalidate any cache entries that point to given repositories that are being removed.
+     *
+     * @param removedRepos
+     */
+    void repositoriesRemoved(Collection<Repository> removedRepos);
+}