Fix memory regression introduced by default providers
The memory regression was introduced in https://github.com/bazelbuild/bazel/commit/360fb4d9a1e2c44154b17aeb866e07bac2dd1b5b , now default providers
are optimized and are built only on demand for all types of targets.
PiperOrigin-RevId: 152957220
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java
index 1e7deca..b522bce 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java
@@ -24,11 +24,13 @@
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.ClassObjectConstructor;
import com.google.devtools.build.lib.packages.PackageSpecification;
+import com.google.devtools.build.lib.packages.SkylarkClassObject;
import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
+import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
/**
@@ -42,6 +44,9 @@
private final NestedSet<PackageSpecification> visibility;
+ // Cached on-demand default provider
+ private final AtomicReference<DefaultProvider> defaultProvider = new AtomicReference<>();
+
// Accessors for Skylark
private static final String DATA_RUNFILES_FIELD = "data_runfiles";
private static final String DEFAULT_RUNFILES_FIELD = "default_runfiles";
@@ -106,15 +111,13 @@
@Override
public Object getValue(String name) {
- // Standard fields should be proxied to their default provider object
- DefaultProvider defaultProvider =
- (DefaultProvider) get(DefaultProvider.SKYLARK_CONSTRUCTOR.getKey());
switch (name) {
case FILES_FIELD:
case DEFAULT_RUNFILES_FIELD:
case DATA_RUNFILES_FIELD:
case FilesToRunProvider.SKYLARK_NAME:
- return defaultProvider.getValue(name);
+ // Standard fields should be proxied to their default provider object
+ return getDefaultProvider().getValue(name);
case LABEL_FIELD:
return getLabel();
default:
@@ -130,14 +133,10 @@
EvalUtils.getDataTypeName(key)));
}
ClassObjectConstructor constructor = (ClassObjectConstructor) key;
- SkylarkProviders provider = getProvider(SkylarkProviders.class);
- if (provider != null) {
- Object declaredProvider = provider.getDeclaredProvider(constructor.getKey());
- if (declaredProvider != null) {
- return declaredProvider;
- }
+ Object declaredProvider = get(constructor.getKey());
+ if (declaredProvider != null) {
+ return declaredProvider;
}
- // Either provider or declaredProvider is null
throw new EvalException(loc, String.format(
"Object of type Target doesn't contain declared provider %s",
constructor.getPrintableName()));
@@ -175,4 +174,30 @@
FILES_FIELD,
FilesToRunProvider.SKYLARK_NAME);
}
+
+ private DefaultProvider getDefaultProvider() {
+ if (defaultProvider.get() == null) {
+ defaultProvider.compareAndSet(
+ null,
+ DefaultProvider.build(
+ getProvider(RunfilesProvider.class),
+ getProvider(FileProvider.class),
+ getProvider(FilesToRunProvider.class)));
+ }
+ return defaultProvider.get();
+ }
+
+ /** Returns a declared provider provided by this target. Only meant to use from Skylark. */
+ @Nullable
+ @Override
+ public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) {
+ if (providerKey.equals(DefaultProvider.SKYLARK_CONSTRUCTOR.getKey())) {
+ return getDefaultProvider();
+ }
+ SkylarkProviders skylarkProviders = getProvider(SkylarkProviders.class);
+ if (skylarkProviders != null) {
+ return skylarkProviders.getDeclaredProvider(providerKey);
+ }
+ return null;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DefaultProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/DefaultProvider.java
index 93b2646..0f0b556 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DefaultProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DefaultProvider.java
@@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis;
+import com.google.common.collect.ImmutableCollection;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -22,6 +24,7 @@
import com.google.devtools.build.lib.packages.SkylarkClassObject;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
/** DefaultProvider is provided by all targets implicitly and contains all standard fields. */
@Immutable
@@ -31,6 +34,17 @@
private static final String DATA_RUNFILES_FIELD = "data_runfiles";
private static final String DEFAULT_RUNFILES_FIELD = "default_runfiles";
private static final String FILES_FIELD = "files";
+ private static final ImmutableList<String> KEYS =
+ ImmutableList.of(
+ DATA_RUNFILES_FIELD,
+ DEFAULT_RUNFILES_FIELD,
+ FILES_FIELD,
+ FilesToRunProvider.SKYLARK_NAME);
+
+ private final RunfilesProvider runfilesProvider;
+ private final FileProvider fileProvider;
+ private final FilesToRunProvider filesToRunProvider;
+ private final AtomicReference<SkylarkNestedSet> files = new AtomicReference<>();
public static final String SKYLARK_NAME = "DefaultInfo";
public static final ClassObjectConstructor SKYLARK_CONSTRUCTOR =
@@ -43,27 +57,47 @@
}
};
- private DefaultProvider(ClassObjectConstructor constructor, Map<String, Object> values) {
- super(constructor, values);
+ private DefaultProvider(
+ ClassObjectConstructor constructor,
+ RunfilesProvider runfilesProvider,
+ FileProvider fileProvider,
+ FilesToRunProvider filesToRunProvider) {
+ // Fields map is not used here to prevent memory regression
+ super(constructor, ImmutableMap.<String, Object>of());
+ this.runfilesProvider = runfilesProvider;
+ this.fileProvider = fileProvider;
+ this.filesToRunProvider = filesToRunProvider;
}
public static DefaultProvider build(
RunfilesProvider runfilesProvider,
FileProvider fileProvider,
FilesToRunProvider filesToRunProvider) {
- ImmutableMap.Builder<String, Object> attrBuilder = new ImmutableMap.Builder<>();
- if (runfilesProvider != null) {
- attrBuilder.put(DATA_RUNFILES_FIELD, runfilesProvider.getDataRunfiles());
- attrBuilder.put(DEFAULT_RUNFILES_FIELD, runfilesProvider.getDefaultRunfiles());
- } else {
- attrBuilder.put(DATA_RUNFILES_FIELD, Runfiles.EMPTY);
- attrBuilder.put(DEFAULT_RUNFILES_FIELD, Runfiles.EMPTY);
+ return new DefaultProvider(
+ SKYLARK_CONSTRUCTOR, runfilesProvider, fileProvider, filesToRunProvider);
+ }
+
+ @Override
+ public Object getValue(String name) {
+ switch (name) {
+ case DATA_RUNFILES_FIELD:
+ return (runfilesProvider == null) ? Runfiles.EMPTY : runfilesProvider.getDataRunfiles();
+ case DEFAULT_RUNFILES_FIELD:
+ return (runfilesProvider == null) ? Runfiles.EMPTY : runfilesProvider.getDefaultRunfiles();
+ case FILES_FIELD:
+ if (files.get() == null) {
+ files.compareAndSet(
+ null, SkylarkNestedSet.of(Artifact.class, fileProvider.getFilesToBuild()));
+ }
+ return files.get();
+ case FilesToRunProvider.SKYLARK_NAME:
+ return filesToRunProvider;
}
+ return null;
+ }
- attrBuilder.put(
- FILES_FIELD, SkylarkNestedSet.of(Artifact.class, fileProvider.getFilesToBuild()));
- attrBuilder.put(FilesToRunProvider.SKYLARK_NAME, filesToRunProvider);
-
- return new DefaultProvider(SKYLARK_CONSTRUCTOR, attrBuilder.build());
+ @Override
+ public ImmutableCollection<String> getKeys() {
+ return KEYS;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/FileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/FileConfiguredTarget.java
index e7ec0dd..75495da 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/FileConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/FileConfiguredTarget.java
@@ -14,18 +14,14 @@
package com.google.devtools.build.lib.analysis;
-import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
-import com.google.devtools.build.lib.packages.ClassObjectConstructor;
import com.google.devtools.build.lib.packages.FileTarget;
-import com.google.devtools.build.lib.packages.SkylarkClassObject;
import com.google.devtools.build.lib.rules.fileset.FilesetProvider;
import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.util.FileType;
-import javax.annotation.Nullable;
/**
* A ConfiguredTarget for a source FileTarget. (Generated files use a
@@ -44,17 +40,10 @@
FileProvider fileProvider = new FileProvider(filesToBuild);
FilesToRunProvider filesToRunProvider =
FilesToRunProvider.fromSingleExecutableArtifact(artifact);
- SkylarkClassObject defaultProvider =
- DefaultProvider.build(null, fileProvider, filesToRunProvider);
- SkylarkProviders skylarkProviders =
- new SkylarkProviders(
- ImmutableMap.<String, Object>of(),
- ImmutableMap.of(DefaultProvider.SKYLARK_CONSTRUCTOR.getKey(), defaultProvider));
TransitiveInfoProviderMap.Builder builder =
TransitiveInfoProviderMap.builder()
.put(VisibilityProvider.class, this)
.put(LicensesProvider.class, this)
- .put(SkylarkProviders.class, skylarkProviders)
.add(fileProvider)
.add(filesToRunProvider);
if (this instanceof FilesetProvider) {
@@ -93,10 +82,4 @@
public Object get(String providerKey) {
return null;
}
-
- @Nullable
- @Override
- public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) {
- return getProvider(SkylarkProviders.class).getDeclaredProvider(providerKey);
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
index 43174ea..fcd4e91 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
@@ -17,12 +17,9 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.packages.ClassObjectConstructor;
-import com.google.devtools.build.lib.packages.SkylarkClassObject;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import javax.annotation.Nullable;
/**
* A single dependency with its configured target and aspects merged together.
@@ -57,12 +54,6 @@
return getProvider(SkylarkProviders.class).getValue(providerKey);
}
- @Nullable
- @Override
- public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) {
- return getProvider(SkylarkProviders.class).getDeclaredProvider(providerKey);
- }
-
@Override
public <P extends TransitiveInfoProvider> P getProvider(Class<P> providerClass) {
AnalysisUtils.checkProvider(providerClass);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
index 4a14b13..2879236 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
@@ -19,10 +19,8 @@
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.packages.ClassObjectConstructor;
import com.google.devtools.build.lib.packages.OutputFile;
import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.packages.SkylarkClassObject;
import com.google.devtools.build.lib.util.Preconditions;
import javax.annotation.Nullable;
@@ -110,15 +108,6 @@
return getProvider(SkylarkProviders.class).getValue(providerKey);
}
- /**
- * Returns a declared provider provided by this target. Only meant to use from Skylark.
- */
- @Override
- public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) {
- return getProvider(SkylarkProviders.class).getDeclaredProvider(providerKey);
- }
-
-
@Override
public final Rule getTarget() {
return (Rule) super.getTarget();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 077e2ec..6a9c6d6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -135,14 +135,6 @@
addSkylarkTransitiveInfo(OutputGroupProvider.SKYLARK_NAME, outputGroupProvider);
}
- // Populate default provider fields and build it
- DefaultProvider defaultProvider =
- DefaultProvider.build(
- providersBuilder.getProvider(RunfilesProvider.class),
- providersBuilder.getProvider(FileProvider.class),
- filesToRunProvider);
- skylarkDeclaredProviders.put(defaultProvider.getConstructor().getKey(), defaultProvider);
-
TransitiveInfoProviderMap providers = providersBuilder.build();
addRegisteredProvidersToSkylarkProviders(providers);
