From 211a3ba2303c1df97383c810e17a031106c7271b Mon Sep 17 00:00:00 2001 From: dslomov Date: Tue, 16 May 2017 00:21:22 +0200 Subject: Do not access SkylarkProviders anywhere outside of ConfiguredTarget implementation. A first step towards applying the same memory optimizations we do for native provider representation to Skylark providers (declared and legacy). RELNOTES: None. PiperOrigin-RevId: 156111749 --- .../lib/analysis/AbstractConfiguredTarget.java | 9 ++++----- .../build/lib/analysis/ConfiguredAspect.java | 9 +++++++-- .../build/lib/analysis/ConfiguredTarget.java | 3 ++- .../analysis/EnvironmentGroupConfiguredTarget.java | 10 ---------- .../build/lib/analysis/FileConfiguredTarget.java | 2 +- .../lib/analysis/PackageGroupConfiguredTarget.java | 11 ----------- .../build/lib/analysis/RuleConfiguredTarget.java | 13 ++++++++---- .../lib/analysis/RuleConfiguredTargetBuilder.java | 2 +- .../build/lib/analysis/SkylarkProviders.java | 6 +++++- .../build/lib/rules/AliasConfiguredTarget.java | 19 ++++++++++-------- .../devtools/build/lib/rules/java/JavaCommon.java | 11 +++++++++++ .../build/lib/rules/java/JavaProvider.java | 7 +------ .../devtools/build/lib/rules/python/PyCommon.java | 23 ++++++++++------------ 13 files changed, 62 insertions(+), 63 deletions(-) (limited to 'src/main/java/com/google') 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 f3e1535916..12fd6b9224 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 @@ -89,7 +89,6 @@ public abstract class AbstractConfiguredTarget return "ConfiguredTarget(" + getTarget().getLabel() + ", " + getConfiguration() + ")"; } - @Nullable @Override public

P getProvider(Class

provider) { AnalysisUtils.checkProvider(provider); @@ -117,7 +116,7 @@ public abstract class AbstractConfiguredTarget } @Override - public Object getIndex(Object key, Location loc) throws EvalException { + public final Object getIndex(Object key, Location loc) throws EvalException { if (!(key instanceof ClassObjectConstructor)) { throw new EvalException(loc, String.format( "Type Target only supports indexing by object constructors, got %s instead", @@ -149,7 +148,7 @@ public abstract class AbstractConfiguredTarget } @Override - public ImmutableCollection getKeys() { + public final ImmutableCollection getKeys() { ImmutableList.Builder result = ImmutableList.builder(); result.addAll(ImmutableList.of( DATA_RUNFILES_FIELD, @@ -180,7 +179,7 @@ public abstract class AbstractConfiguredTarget @Nullable @Override - public Object get(SkylarkProviderIdentifier id) { + public final Object get(SkylarkProviderIdentifier id) { if (id.isLegacy()) { return get(id.getLegacyId()); } @@ -191,7 +190,7 @@ public abstract class AbstractConfiguredTarget /** Returns a declared provider provided by this target. Only meant to use from Skylark. */ @Nullable @Override - public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) { + public final SkylarkClassObject get(ClassObjectConstructor.Key providerKey) { if (providerKey.equals(DefaultProvider.SKYLARK_CONSTRUCTOR.getKey())) { return getDefaultProvider(); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java index be92473463..f1c4b4d515 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java @@ -88,9 +88,14 @@ public final class ConfiguredAspect { @Nullable @VisibleForTesting public

P getProvider(Class

providerClass) { + AnalysisUtils.checkProvider(providerClass); return providers.getProvider(providerClass); } + SkylarkProviders getSkylarkProviders() { + return providers.getProvider(SkylarkProviders.class); + } + public Object getProvider(SkylarkProviderIdentifier id) { if (id.isLegacy()) { return get(id.getLegacyId()); @@ -103,7 +108,7 @@ public final class ConfiguredAspect { if (OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey().equals(key)) { return getProvider(OutputGroupProvider.class); } - SkylarkProviders skylarkProviders = getProvider(SkylarkProviders.class); + SkylarkProviders skylarkProviders = providers.getProvider(SkylarkProviders.class); return skylarkProviders != null ? skylarkProviders.getDeclaredProvider(key) : null; } @@ -111,7 +116,7 @@ public final class ConfiguredAspect { if (OutputGroupProvider.SKYLARK_NAME.equals(legacyKey)) { return getProvider(OutputGroupProvider.class); } - SkylarkProviders skylarkProviders = getProvider(SkylarkProviders.class); + SkylarkProviders skylarkProviders = providers.getProvider(SkylarkProviders.class); return skylarkProviders != null ? skylarkProviders.get(SkylarkProviderIdentifier.forLegacy(legacyKey)) : null; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java index 5605f953bf..cee9b8544a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.analysis; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.syntax.ClassObject; import javax.annotation.Nullable; /** @@ -28,7 +29,7 @@ import javax.annotation.Nullable; * {@link TransitiveInfoCollection}s. Also, {@link ConfiguredTarget} objects should not be * accessible from the action graph. */ -public interface ConfiguredTarget extends TransitiveInfoCollection { +public interface ConfiguredTarget extends TransitiveInfoCollection, ClassObject { /** * All ConfiguredTargets have a "label" field. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java index eb7a1e1a32..e67a2b6395 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java @@ -14,11 +14,8 @@ package com.google.devtools.build.lib.analysis; -import com.google.devtools.build.lib.packages.ClassObjectConstructor; import com.google.devtools.build.lib.packages.EnvironmentGroup; -import com.google.devtools.build.lib.packages.SkylarkClassObject; import com.google.devtools.build.lib.util.Preconditions; -import javax.annotation.Nullable; /** * Dummy ConfiguredTarget for environment groups. Contains no functionality, since @@ -34,11 +31,4 @@ public final class EnvironmentGroupConfiguredTarget extends AbstractConfiguredTa public EnvironmentGroup getTarget() { return (EnvironmentGroup) super.getTarget(); } - - @Nullable - @Override - public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) { - // No providers. - return null; - } } 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 399ef6afe6..7b68c7b2b3 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 @@ -65,7 +65,7 @@ public abstract class FileConfiguredTarget extends AbstractConfiguredTarget } /** - * Returns the file type of this file target. + * Returns the file name of this file target. */ @Override public String getFilename() { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java index 3e0b855904..181162c97a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java @@ -18,12 +18,9 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.packages.ClassObjectConstructor; import com.google.devtools.build.lib.packages.PackageGroup; import com.google.devtools.build.lib.packages.PackageSpecification; -import com.google.devtools.build.lib.packages.SkylarkClassObject; import com.google.devtools.build.lib.util.Preconditions; -import javax.annotation.Nullable; /** * Dummy ConfiguredTarget for package groups. Contains no functionality, since @@ -66,12 +63,4 @@ public final class PackageGroupConfiguredTarget extends AbstractConfiguredTarget public NestedSet getPackageSpecifications() { return packageSpecifications; } - - - @Nullable - @Override - public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) { - // No providers. - return null; - } } 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 993b812f95..85e65940fd 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,6 +19,8 @@ import com.google.devtools.build.lib.analysis.config.RunUnder; import com.google.devtools.build.lib.cmdline.Label; 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.packages.SkylarkClassObjectConstructor; import com.google.devtools.build.lib.util.Preconditions; import javax.annotation.Nullable; @@ -48,7 +50,8 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget { RuleConfiguredTarget( RuleContext ruleContext, TransitiveInfoProviderMap providers, - SkylarkProviders skylarkProviders) { + ImmutableMap legacySkylarkProviders, + ImmutableMap skylarkProviders) { super(ruleContext); // We don't use ImmutableMap.Builder here to allow augmenting the initial list of 'default' // providers by passing them in. @@ -59,9 +62,11 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget { Preconditions.checkState(providerBuilder.contains(FilesToRunProvider.class)); // Initialize every SkylarkApiProvider - if (!skylarkProviders.isEmpty()) { - skylarkProviders.init(this); - providerBuilder.add(skylarkProviders); + if (!legacySkylarkProviders.isEmpty() || !skylarkProviders.isEmpty()) { + SkylarkProviders allSkylarkProviders = new SkylarkProviders(legacySkylarkProviders, + skylarkProviders); + allSkylarkProviders.init(this); + providerBuilder.add(allSkylarkProviders); } this.providers = providerBuilder.build(); 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 84fa02700c..552122a3a5 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 @@ -140,7 +140,7 @@ public final class RuleConfiguredTargetBuilder { return new RuleConfiguredTarget( ruleContext, providers, - new SkylarkProviders(skylarkProviders.build(), skylarkDeclaredProviders.build())); + skylarkProviders.build(), skylarkDeclaredProviders.build()); } /** Adds skylark providers from a skylark provider registry, and checks for collisions. */ diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java index 78cb247164..a9046eb03e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java @@ -32,9 +32,13 @@ import java.util.Set; /** * A helper class for transitive infos provided by Skylark rule implementations. + * + * DO NOT USE THIS CLASS to access Skylark providers. + * Use {@link ConfiguredTarget#get(SkylarkProviderIdentifier)} or + * {@link ConfiguredAspect#getProvider(SkylarkProviderIdentifier)}. */ @Immutable -public final class SkylarkProviders implements TransitiveInfoProvider { +final class SkylarkProviders implements TransitiveInfoProvider { private final ImmutableMap declaredProviders; private final ImmutableMap skylarkProviders; diff --git a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java index 7be13d2dda..3ac06faff7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java @@ -19,7 +19,6 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; -import com.google.devtools.build.lib.analysis.SkylarkProviders; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; @@ -127,19 +126,15 @@ public final class AliasConfiguredTarget implements ConfiguredTarget, ClassObjec ? NestedSetBuilder.emptySet(Order.STABLE_ORDER) : getProvider(FileProvider.class).getFilesToBuild()); } - if (actual instanceof ClassObject) { - return ((ClassObject) actual).getValue(name); - } - return actual == null ? null : actual.get(name); + return actual == null ? null : actual.getValue(name); } @Override public ImmutableCollection getKeys() { - ImmutableList.Builder result = ImmutableList.builder().add("label", "files"); if (actual != null) { - result.addAll(actual.getProvider(SkylarkProviders.class).getKeys()); + return actual.getKeys(); } - return result.build(); + return ImmutableList.of(); } @Override @@ -147,4 +142,12 @@ public final class AliasConfiguredTarget implements ConfiguredTarget, ClassObjec // Use the default error message. return null; } + + /** + * Returns a target this target aliases. + */ + @Nullable + public ConfiguredTarget getActual() { + return actual; + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java index dd1dece72a..848ef4419c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java @@ -37,6 +37,8 @@ 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.BuildType; +import com.google.devtools.build.lib.packages.ClassObjectConstructor; +import com.google.devtools.build.lib.packages.SkylarkClassObject; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.rules.cpp.CppCompilationContext; @@ -831,6 +833,15 @@ public class JavaCommon { return AnalysisUtils.getProviders(getDependencies(), provider); } + /** + * Gets all the deps that implement a particular provider. + */ + public final

Iterable

getDependencies( + ClassObjectConstructor.Key provider, Class

resultClass) { + return AnalysisUtils.getProviders(getDependencies(), provider, resultClass); + } + + /** * Returns true if and only if this target has the neverlink attribute set to * 1, or false if the neverlink attribute does not exist (for example, on diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaProvider.java index f8c6daec8f..1a07ff3d0e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaProvider.java @@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.Runfiles; -import com.google.devtools.build.lib.analysis.SkylarkProviders; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap; @@ -133,12 +132,8 @@ public final class JavaProvider extends SkylarkClassObject implements Transitive if (provider != null) { return provider; } - SkylarkProviders skylarkProviders = target.getProvider(SkylarkProviders.class); - if (skylarkProviders == null) { - return null; - } JavaProvider javaProvider = - (JavaProvider) skylarkProviders.getDeclaredProvider(JavaProvider.JAVA_PROVIDER.getKey()); + (JavaProvider) target.get(JavaProvider.JAVA_PROVIDER.getKey()); if (javaProvider == null) { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java index 2036401394..3ea0849634 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java @@ -30,7 +30,6 @@ import com.google.devtools.build.lib.analysis.PseudoAction; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.SkylarkProviders; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.Util; import com.google.devtools.build.lib.cmdline.Label; @@ -294,12 +293,13 @@ public final class PyCommon { private NestedSet getTransitivePythonSourcesFromSkylarkProvider( TransitiveInfoCollection dep) { SkylarkClassObject pythonSkylarkProvider = null; - SkylarkProviders skylarkProviders = dep.getProvider(SkylarkProviders.class); try { - if (skylarkProviders != null) { - pythonSkylarkProvider = skylarkProviders.getValue(PYTHON_SKYLARK_PROVIDER_NAME, - SkylarkClassObject.class); - } + pythonSkylarkProvider = SkylarkType.cast( + dep.get(PYTHON_SKYLARK_PROVIDER_NAME), + SkylarkClassObject.class, + null, + "%s should be a struct", PYTHON_SKYLARK_PROVIDER_NAME + ); if (pythonSkylarkProvider != null) { Object sourceFiles = pythonSkylarkProvider.getValue(TRANSITIVE_PYTHON_SRCS); @@ -472,13 +472,10 @@ public final class PyCommon { public static boolean checkForSharedLibraries(Iterable deps) throws EvalException{ for (TransitiveInfoCollection dep : deps) { - SkylarkProviders providers = dep.getProvider(SkylarkProviders.class); - SkylarkClassObject provider = null; - if (providers != null) { - provider = providers.getValue(PYTHON_SKYLARK_PROVIDER_NAME, - SkylarkClassObject.class); - } - if (provider != null) { + Object providerObject = dep.get(PYTHON_SKYLARK_PROVIDER_NAME); + if (providerObject != null) { + SkylarkType.checkType(providerObject, SkylarkClassObject.class, null); + SkylarkClassObject provider = (SkylarkClassObject) providerObject; Boolean isUsingSharedLibrary = provider.getValue(IS_USING_SHARED_LIBRARY, Boolean.class); if (Boolean.TRUE.equals(isUsingSharedLibrary)) { return true; -- cgit v1.2.3