Use another cache to store preprocessing results instead of redoing the work.

This should have no effect.

--
MOS_MIGRATED_REVID=102342697

2 files changed, 20 insertions, 5 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 2c7b068131..9e3b8d516b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -84,6 +84,7 @@ public class PackageFunction implements SkyFunction {
   private final PackageFactory packageFactory;
   private final CachingPackageLocator packageLocator;
   private final Cache<PackageIdentifier, Package.LegacyBuilder> packageFunctionCache;
+  private final Cache<PackageIdentifier, Preprocessor.Result> preprocessCache;
   private final AtomicBoolean showLoadingProgress;
   private final AtomicInteger numPackagesLoaded;
   private final Profiler profiler = Profiler.instance();
@@ -96,6 +97,7 @@ public class PackageFunction implements SkyFunction {
   public PackageFunction(Reporter reporter, PackageFactory packageFactory,
       CachingPackageLocator pkgLocator, AtomicBoolean showLoadingProgress,
       Cache<PackageIdentifier, Package.LegacyBuilder> packageFunctionCache,
+      Cache<PackageIdentifier, Preprocessor.Result> preprocessCache,
       AtomicInteger numPackagesLoaded) {
     this.reporter = reporter;
 
@@ -107,6 +109,7 @@ public class PackageFunction implements SkyFunction {
     this.packageLocator = pkgLocator;
     this.showLoadingProgress = showLoadingProgress;
     this.packageFunctionCache = packageFunctionCache;
+    this.preprocessCache = preprocessCache;
     this.numPackagesLoaded = numPackagesLoaded;
   }
 
@@ -795,10 +798,14 @@ public class PackageFunction implements SkyFunction {
         Globber globber = packageFactory.createLegacyGlobber(buildFilePath.getParentDirectory(),
             packageId, packageLocator);
         StoredEventHandler localReporter = new StoredEventHandler();
-        Preprocessor.Result preprocessingResult = replacementSource == null
-            ? packageFactory.preprocess(packageId, buildFilePath, inputSource, globber,
-                localReporter)
-                : Preprocessor.Result.noPreprocessing(replacementSource);
+        Preprocessor.Result preprocessingResult = preprocessCache.getIfPresent(packageId);
+        if (preprocessingResult == null) {
+          preprocessingResult = replacementSource == null
+              ? packageFactory.preprocess(packageId, buildFilePath, inputSource, globber,
+                  localReporter)
+              : Preprocessor.Result.noPreprocessing(replacementSource);
+          preprocessCache.put(packageId, preprocessingResult);
+        }
 
         SkylarkImportResult importResult =
             discoverSkylarkImports(
@@ -811,6 +818,7 @@ public class PackageFunction implements SkyFunction {
         if (importResult == null) {
           return null;
         }
+        preprocessCache.invalidate(packageId);
 
         pkgBuilder = packageFactory.createPackageFromPreprocessingResult(externalPkg, packageId,
             buildFilePath, preprocessingResult, localReporter.getEvents(), preludeStatements,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 8f44d5d1d7..5fc379eb7e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -176,6 +176,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   // [skyframe-loading]
   private final Cache<PackageIdentifier, Package.LegacyBuilder> packageFunctionCache =
       newPkgFunctionCache();
+  private final Cache<PackageIdentifier, Preprocessor.Result> preprocessCache =
+      newPreprocessCache();
 
   private final AtomicInteger numPackagesLoaded = new AtomicInteger(0);
 
@@ -323,7 +325,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     map.put(SkyFunctions.RECURSIVE_PKG, new RecursivePkgFunction());
     map.put(SkyFunctions.PACKAGE, new PackageFunction(
         reporter, pkgFactory, packageManager, showLoadingProgress, packageFunctionCache,
-        numPackagesLoaded));
+        preprocessCache, numPackagesLoaded));
     map.put(SkyFunctions.TARGET_MARKER, new TargetMarkerFunction());
     map.put(SkyFunctions.TRANSITIVE_TARGET, new TransitiveTargetFunction(ruleClassProvider));
     map.put(SkyFunctions.TRANSITIVE_TRAVERSAL, new TransitiveTraversalFunction());
@@ -592,6 +594,10 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     return CacheBuilder.newBuilder().build();
   }
 
+  protected Cache<PackageIdentifier, Preprocessor.Result> newPreprocessCache() {
+    return CacheBuilder.newBuilder().build();
+  }
+
   /**
    * Injects the build info factory map that will be used when constructing build info
    * actions/artifacts. Unchanged across the life of the Blaze server, although it must be injected
@@ -845,6 +851,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
 
     // If the PackageFunction was interrupted, there may be stale entries here.
     packageFunctionCache.invalidateAll();
+    preprocessCache.invalidateAll();
     numPackagesLoaded.set(0);
 
     // Reset the stateful SkyframeCycleReporter, which contains cycles from last run.