Merge pull request #23240 Use Cache Without Locking for DefaultClassS…

…etAnalyser This change fixes the performance regressions from switching the `ZipFileTree` to use a locking cache in #22809. Performance test results are [here](https://builds.gradle.org/repository/download/Gradle_Release_Util_Performance_AdHocPerformanceScenarioLinuxAmd64/59685577:id/results/performance/build/test-results-largeMonolithicJavaProjectPerformanceAdHocTest.zip!/performance-tests/report/index.html). Co-authored-by: Thomas Tresansky <ttresansky@gradle.com>
gradle · Dec 20, 2022 · 08592d2 · 08592d2
2 parents 1b94111 + 0ac0abe
commit 08592d2
Show file tree

Hide file tree

Showing 8 changed files with 44 additions and 8 deletions.
diff --git a/subprojects/core/src/main/java/org/gradle/api/internal/file/DefaultFileOperations.java b/subprojects/core/src/main/java/org/gradle/api/internal/file/DefaultFileOperations.java
@@ -50,7 +50,9 @@
 import org.gradle.api.tasks.WorkResult;
 import org.gradle.api.tasks.WorkResults;
 import org.gradle.api.tasks.util.PatternSet;
+import org.gradle.cache.internal.DecompressionCache;
 import org.gradle.cache.internal.DecompressionCacheFactory;
+import org.gradle.cache.scopes.ScopedCache;
 import org.gradle.internal.Cast;
 import org.gradle.internal.Factory;
 import org.gradle.internal.file.Deleter;
@@ -85,6 +87,7 @@ public class DefaultFileOperations implements FileOperations {
     private final TaskDependencyFactory taskDependencyFactory;
     private final ProviderFactory providers;
     private final DecompressionCacheFactory decompressionCacheFactory;
+    private final ScopedCache scopedCache;
 
     public DefaultFileOperations(
         FileResolver fileResolver,
@@ -100,7 +103,8 @@ public DefaultFileOperations(
         DocumentationRegistry documentationRegistry,
         TaskDependencyFactory taskDependencyFactory,
         ProviderFactory providers,
-        DecompressionCacheFactory decompressionCacheFactory
+        DecompressionCacheFactory decompressionCacheFactory,
+        ScopedCache scopedCache
     ) {
         this.fileCollectionFactory = fileCollectionFactory;
         this.fileResolver = fileResolver;
@@ -125,6 +129,7 @@ public DefaultFileOperations(
         this.fileSystem = fileSystem;
         this.deleter = deleter;
         this.decompressionCacheFactory = decompressionCacheFactory;
+        this.scopedCache = scopedCache;
     }
 
     @Override
@@ -177,6 +182,27 @@ public FileTreeInternal zipTree(Object zipPath) {
         return new FileTreeAdapter(new ZipFileTree(fileProvider, fileSystem, directoryFileTreeFactory, fileHasher, decompressionCacheFactory.create()), taskDependencyFactory, patternSetFactory);
     }
 
+    @Override
+    public FileTreeInternal zipTreeNoLocking(Object zipPath) {
+        Provider<File> fileProvider = asFileProvider(zipPath);
+        DecompressionCache nonLockingCache = new DecompressionCache() {
+            @Override
+            public File getBaseDir() {
+                return directoryFileTreeFactory.create(scopedCache.baseDirForCrossVersionCache(DecompressionCache.EXPANSION_CACHE_KEY)).getDir();
+            }
+
+            @Override
+            public void useCache(Runnable action) {
+                action.run();
+            }
+
+            @Override
+            public void close() throws IOException {}
+        };
+
+        return new FileTreeAdapter(new ZipFileTree(fileProvider, fileSystem, directoryFileTreeFactory, fileHasher, nonLockingCache), taskDependencyFactory, patternSetFactory);
+    }
+
     @Override
     public FileTreeInternal tarTree(Object tarPath) {
         Provider<File> fileProvider = asFileProvider(tarPath);
@@ -299,6 +325,7 @@ public static DefaultFileOperations createSimple(FileResolver fileResolver, File
         ProviderFactory providers = services.get(ProviderFactory.class);
         TaskDependencyFactory taskDependencyFactory = services.get(TaskDependencyFactory.class);
         DecompressionCacheFactory decompressionCacheFactory = services.get(DecompressionCacheFactory.class);
+        ScopedCache scopedCache = services.get(ScopedCache.class);
 
         DefaultResourceHandler.Factory resourceHandlerFactory = DefaultResourceHandler.Factory.from(
             fileResolver,
@@ -322,6 +349,7 @@ public static DefaultFileOperations createSimple(FileResolver fileResolver, File
             documentationRegistry,
             taskDependencyFactory,
             providers,
-            decompressionCacheFactory);
+            decompressionCacheFactory,
+            scopedCache);
     }
 }
diff --git a/subprojects/core/src/main/java/org/gradle/api/internal/file/FileOperations.java b/subprojects/core/src/main/java/org/gradle/api/internal/file/FileOperations.java
@@ -65,6 +65,8 @@ public interface FileOperations {
 
     FileTree zipTree(Object zipPath);
 
+    FileTree zipTreeNoLocking(Object zipPath);
+
     FileTree tarTree(Object tarPath);
 
     CopySpec copySpec();

diff --git a/...re/src/main/java/org/gradle/internal/service/scopes/WorkerSharedProjectScopeServices.java b/...re/src/main/java/org/gradle/internal/service/scopes/WorkerSharedProjectScopeServices.java
@@ -49,6 +49,7 @@
 import org.gradle.cache.internal.DecompressionCacheFactory;
 import org.gradle.cache.internal.scopes.DefaultProjectScopedCache;
 import org.gradle.cache.scopes.ProjectScopedCache;
+import org.gradle.cache.scopes.ScopedCache;
 import org.gradle.internal.Factory;
 import org.gradle.internal.file.Deleter;
 import org.gradle.internal.hash.FileHasher;
@@ -97,7 +98,8 @@ protected DefaultFileOperations createFileOperations(
             DocumentationRegistry documentationRegistry,
             ProviderFactory providers,
             TaskDependencyFactory taskDependencyFactory,
-            DecompressionCacheFactory decompressionCache
+            DecompressionCacheFactory decompressionCache,
+            ScopedCache scopedCache
     ) {
         return new DefaultFileOperations(
                 fileResolver,
@@ -113,7 +115,8 @@ protected DefaultFileOperations createFileOperations(
                 documentationRegistry,
                 taskDependencyFactory,
                 providers,
-                decompressionCache);
+                decompressionCache,
+                scopedCache);
     }
 
     protected FileSystemOperations createFileSystemOperations(Instantiator instantiator, FileOperations fileOperations) {

diff --git a/...ojects/core/src/test/groovy/org/gradle/api/internal/file/DefaultFileOperationsTest.groovy b/...ojects/core/src/test/groovy/org/gradle/api/internal/file/DefaultFileOperationsTest.groovy
@@ -73,7 +73,8 @@ class DefaultFileOperationsTest extends Specification {
             TestFiles.documentationRegistry(),
             TestFiles.taskDependencyFactory(),
             TestUtil.providerFactory(),
-            TestCaches.decompressionCacheFactory(temporaryFileProvider.newTemporaryDirectory("cache"))
+            TestCaches.decompressionCacheFactory(temporaryFileProvider.newTemporaryDirectory("cache")),
+            null
         )
     }
 

diff --git a/subprojects/core/src/testFixtures/groovy/org/gradle/api/internal/file/TestFiles.java b/subprojects/core/src/testFixtures/groovy/org/gradle/api/internal/file/TestFiles.java
@@ -172,7 +172,8 @@ public static FileOperations fileOperations(File basedDir, TemporaryFileProvider
             documentationRegistry(),
             taskDependencyFactory(),
             providerFactory(),
-            TestCaches.decompressionCacheFactory(temporaryFileProvider.newTemporaryDirectory("cache-dir")));
+            TestCaches.decompressionCacheFactory(temporaryFileProvider.newTemporaryDirectory("cache-dir")),
+            null);
     }
 
     public static ApiTextResourceAdapter.Factory textResourceAdapterFactory(@Nullable TemporaryFileProvider temporaryFileProvider) {

diff --git a/.../org/gradle/api/internal/tasks/compile/incremental/classpath/DefaultClassSetAnalyzer.java b/.../org/gradle/api/internal/tasks/compile/incremental/classpath/DefaultClassSetAnalyzer.java
@@ -75,7 +75,7 @@ private ClassSetAnalysisData analyze(File classSet, boolean abiOnly) {
 
     private void visit(File classpathEntry, ClassDependentsAccumulator accumulator, boolean abiOnly) {
         if (hasExtension(classpathEntry, ".jar")) {
-            fileOperations.zipTree(classpathEntry).visit(new JarEntryVisitor(accumulator, abiOnly));
+            fileOperations.zipTreeNoLocking(classpathEntry).visit(new JarEntryVisitor(accumulator, abiOnly));
         }
         if (classpathEntry.isDirectory()) {
             fileOperations.fileTree(classpathEntry).visit(new DirectoryEntryVisitor(accumulator, abiOnly));

diff --git a/subprojects/persistent-cache/src/main/java/org/gradle/cache/internal/DecompressionCache.java b/subprojects/persistent-cache/src/main/java/org/gradle/cache/internal/DecompressionCache.java
@@ -23,6 +23,8 @@
  * A cache that can be used to store decompressed data extracted from archive files like zip and tars.
  */
 public interface DecompressionCache extends Closeable {
+    String EXPANSION_CACHE_KEY = "expanded";
+
     /**
      * Returns the root directory used by this cache to store decompressed files.
      *

diff --git a/...s/persistent-cache/src/main/java/org/gradle/cache/internal/DefaultDecompressionCache.java b/...s/persistent-cache/src/main/java/org/gradle/cache/internal/DefaultDecompressionCache.java
@@ -32,7 +32,6 @@
  * are only permitted to one client at a time.  The cache will be a Gradle cross version cache.
  */
 public class DefaultDecompressionCache implements DecompressionCache {
-    private static final String EXPANSION_CACHE_KEY = "expanded";
     private static final String EXPANSION_CACHE_NAME = "Compressed Files Expansion Cache";
 
     private final PersistentCache cache;