diff options
6 files changed, 84 insertions, 70 deletions
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 1e7deca5b9..b522bce19d 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.collect.nestedset.Order; 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 @@ public abstract class AbstractConfiguredTarget 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 @@ public abstract class AbstractConfiguredTarget @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 @@ public abstract class AbstractConfiguredTarget 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 @@ public abstract class AbstractConfiguredTarget 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 93b2646f86..0f0b5569b5 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.NativeClassObjectConstructor; 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 @@ public final class DefaultProvider extends SkylarkClassObject { 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 @@ public final class DefaultProvider extends SkylarkClassObject { } }; - 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); + } - attrBuilder.put( - FILES_FIELD, SkylarkNestedSet.of(Artifact.class, fileProvider.getFilesToBuild())); - attrBuilder.put(FilesToRunProvider.SKYLARK_NAME, 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; + } - 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 e7ec0dd169..75495da725 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 @@ public abstract class FileConfiguredTarget extends AbstractConfiguredTarget 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 abstract class FileConfiguredTarget extends AbstractConfiguredTarget 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 43174eac20..fcd4e91e35 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.ImmutableCollection; 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 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget { 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 4a14b13e7f..287923673a 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.common.collect.ImmutableMap; 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 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget { 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 077e2ec5e5..6a9c6d625b 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 @@ public final class RuleConfiguredTargetBuilder { 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); |