diff options
Diffstat (limited to 'src/main/java/com/google')
9 files changed, 145 insertions, 41 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index ab9981b93d..4a1bd249f7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -26,7 +26,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Multimap; @@ -45,6 +44,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollectio import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.ConfigurationResolver; +import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; @@ -954,9 +954,12 @@ public class BuildView { @Override protected List<BuildConfiguration> getConfigurations( - ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragments, - Iterable<BuildOptions> buildOptions) { - Preconditions.checkArgument(ct.getConfiguration().fragmentClasses().equals(fragments)); + FragmentClassSet fragments, Iterable<BuildOptions> buildOptions) { + Preconditions.checkArgument( + ct.getConfiguration().fragmentClasses().equals(fragments), + "Mismatch: %s %s", + ct, + fragments); Dependency asDep = Dependency.withTransitionAndAspects(ct.getLabel(), NoTransition.INSTANCE, AspectCollection.EMPTY); ImmutableList.Builder<BuildConfiguration> builder = ImmutableList.builder(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index c87ca9b03a..ea542dfffa 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java @@ -18,12 +18,12 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedSet; import com.google.devtools.build.lib.analysis.AspectCollection.AspectCycleOnPathException; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.DynamicTransitionMapper; +import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.TransitionResolver; @@ -875,8 +875,7 @@ public abstract class DependencyResolver { */ @Nullable protected abstract List<BuildConfiguration> getConfigurations( - ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragments, - Iterable<BuildOptions> buildOptions) + FragmentClassSet fragments, Iterable<BuildOptions> buildOptions) throws InvalidConfigurationException, InterruptedException; /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 10daadda48..3b54a8b17c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -26,7 +26,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; -import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Interner; import com.google.common.collect.Interners; import com.google.common.collect.Iterables; @@ -1112,6 +1111,7 @@ public class BuildConfiguration implements BuildEvent { private final String checksum; private final ImmutableSortedMap<Class<? extends Fragment>, Fragment> fragments; + private final FragmentClassSet fragmentClassSet; private final ImmutableMap<String, Class<? extends Fragment>> skylarkVisibleFragments; private final String repositoryName; @@ -1390,6 +1390,7 @@ public class BuildConfiguration implements BuildEvent { String repositoryName) { this.directories = directories; this.fragments = makeFragmentsMap(fragmentsMap); + this.fragmentClassSet = FragmentClassSet.of(this.fragments.keySet()); this.skylarkVisibleFragments = buildIndexOfSkylarkVisibleFragments(); this.repositoryName = repositoryName; @@ -1475,12 +1476,11 @@ public class BuildConfiguration implements BuildEvent { * configuration is assumed to have). */ public BuildConfiguration clone( - Set<Class<? extends BuildConfiguration.Fragment>> fragmentClasses, - RuleClassProvider ruleClassProvider) { + FragmentClassSet fragmentClasses, RuleClassProvider ruleClassProvider) { ClassToInstanceMap<Fragment> fragmentsMap = MutableClassToInstanceMap.create(); for (Fragment fragment : fragments.values()) { - if (fragmentClasses.contains(fragment.getClass())) { + if (fragmentClasses.fragmentClasses().contains(fragment.getClass())) { fragmentsMap.put(fragment.getClass(), fragment); } } @@ -1847,8 +1847,8 @@ public class BuildConfiguration implements BuildEvent { } /** Which fragments does this configuration contain? */ - public ImmutableSortedSet<Class<? extends Fragment>> fragmentClasses() { - return fragments.keySet(); + public FragmentClassSet fragmentClasses() { + return fragmentClassSet; } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java index 249ac0c5e4..57cf02b1f1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java @@ -19,7 +19,6 @@ import com.google.common.base.Joiner; import com.google.common.base.Verify; import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.LinkedListMultimap; @@ -123,8 +122,7 @@ public final class ConfigurationResolver { Map<FragmentsAndTransition, List<BuildOptions>> transitionsMap = new LinkedHashMap<>(); // The fragments used by the current target's configuration. - ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> ctgFragments = - ctgValue.getConfiguration().fragmentClasses(); + FragmentClassSet ctgFragments = ctgValue.getConfiguration().fragmentClasses(); BuildOptions ctgOptions = ctgValue.getConfiguration().getOptions(); // Stores the configuration-resolved versions of each dependency. This method must preserve the @@ -183,7 +181,7 @@ public final class ConfigurationResolver { depFragments); } - boolean sameFragments = depFragments.equals(ctgFragments); + boolean sameFragments = depFragments.equals(ctgFragments.fragmentClasses()); Transition transition = dep.getTransition(); if (sameFragments) { @@ -233,9 +231,12 @@ public final class ConfigurationResolver { // If we get here, we have to get the configuration from Skyframe. for (BuildOptions options : toOptions) { - keysToEntries.put( - BuildConfigurationValue.key(sameFragments ? ctgFragments : depFragments, options), - depsEntry); + if (sameFragments) { + keysToEntries.put(BuildConfigurationValue.key(ctgFragments, options), depsEntry); + + } else { + keysToEntries.put(BuildConfigurationValue.key(depFragments, options), depsEntry); + } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentClassSet.java b/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentClassSet.java new file mode 100644 index 0000000000..a419224b7e --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/FragmentClassSet.java @@ -0,0 +1,98 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.analysis.config; + +import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.Interner; +import com.google.devtools.build.lib.concurrent.BlazeInterners; +import com.google.devtools.build.lib.util.Fingerprint; +import java.util.Arrays; +import javax.annotation.Nullable; + +/** + * A wrapper class for an {@code ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>>} + * object. Interning these objects allows us to do cheap reference equality checks when these sets + * are in frequently used keys. For good measure, we also compute a fingerprint. + */ +public class FragmentClassSet { + private final ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragments; + + // Lazily initialized. + @Nullable private volatile byte[] fingerprint; + private volatile int hashCode; + + private FragmentClassSet( + ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragments) { + this.fragments = fragments; + } + + private static final Interner<FragmentClassSet> interner = BlazeInterners.newWeakInterner(); + + public static FragmentClassSet of( + ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragments) { + return interner.intern(new FragmentClassSet(fragments)); + } + + public ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragmentClasses() { + return fragments; + } + + /** + * Lazily initialize {@link #fingerprint} and {@link #hashCode}. Keeps computation off critical + * path of build, while still avoiding expensive computation for equality and hash code each time. + * + * <p>We check for nullity of {@link #fingerprint} to see if this method has already been called. + * Using {@link #hashCode} after this method is called is safe because it is set here before + * {@link #fingerprint} is set, so if {@link #fingerprint} is non-null then {@link #hashCode} is + * definitely set. + */ + private void maybeInitializeFingerprintAndHashCode() { + if (fingerprint != null) { + return; + } + synchronized (this) { + if (fingerprint != null) { + return; + } + Fingerprint fingerprint = new Fingerprint(); + for (Class<? extends BuildConfiguration.Fragment> fragment : fragments) { + fingerprint.addString(fragment.getName()); + } + byte[] computedFingerprint = fingerprint.digestAndReset(); + hashCode = Arrays.hashCode(computedFingerprint); + this.fingerprint = computedFingerprint; + } + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } else if (!(other instanceof FragmentClassSet)) { + return false; + } else { + maybeInitializeFingerprintAndHashCode(); + FragmentClassSet that = (FragmentClassSet) other; + that.maybeInitializeFingerprintAndHashCode(); + return Arrays.equals(this.fingerprint, that.fingerprint); + } + } + + @Override + public int hashCode() { + maybeInitializeFingerprintAndHashCode(); + return hashCode; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java index c42539aace..df86614495 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java @@ -17,8 +17,8 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Interner; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; import com.google.devtools.build.lib.analysis.config.BuildOptions; +import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.skyframe.serialization.InjectingObjectCodec; @@ -67,28 +67,29 @@ public class BuildConfigurationValue implements SkyValue { @ThreadSafe public static Key key( Set<Class<? extends BuildConfiguration.Fragment>> fragments, BuildOptions buildOptions) { - return keyInterner.intern( - new Key( - ImmutableSortedSet.copyOf(BuildConfiguration.lexicalFragmentSorter, fragments), - buildOptions)); + return key( + FragmentClassSet.of( + ImmutableSortedSet.copyOf(BuildConfiguration.lexicalFragmentSorter, fragments)), + buildOptions); } - static final class Key implements SkyKey, Serializable { - static final ObjectCodec<Key> CODEC = new Codec(); + public static Key key(FragmentClassSet fragmentClassSet, BuildOptions buildOptions) { + return keyInterner.intern(new Key(fragmentClassSet, buildOptions)); + } - private final ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragments; + static final class Key implements SkyKey, Serializable { + private final FragmentClassSet fragments; private final BuildOptions buildOptions; // If hashCode really is -1, we'll recompute it from scratch each time. Oh well. private volatile int hashCode = -1; - private Key( - ImmutableSortedSet<Class<? extends Fragment>> fragments, BuildOptions buildOptions) { + private Key(FragmentClassSet fragments, BuildOptions buildOptions) { this.fragments = fragments; this.buildOptions = Preconditions.checkNotNull(buildOptions); } ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> getFragments() { - return fragments; + return fragments.fragmentClasses(); } BuildOptions getBuildOptions() { @@ -131,8 +132,9 @@ public class BuildConfigurationValue implements SkyValue { public void serialize(Key obj, CodedOutputStream codedOut) throws SerializationException, IOException { BuildOptions.CODEC.serialize(obj.buildOptions, codedOut); - codedOut.writeInt32NoTag(obj.fragments.size()); - for (Class<? extends BuildConfiguration.Fragment> fragment : obj.fragments) { + codedOut.writeInt32NoTag(obj.fragments.fragmentClasses().size()); + for (Class<? extends BuildConfiguration.Fragment> fragment : + obj.fragments.fragmentClasses()) { StringCodecs.asciiOptimized().serialize(fragment.getName(), codedOut); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java index 5d4e24aad7..118a636294 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildIn import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; +import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.events.Event; @@ -112,8 +113,8 @@ public final class SkyframeBuildView { private BuildConfiguration topLevelHostConfiguration; // Fragment-limited versions of the host configuration. It's faster to create/cache these here // than to store them in Skyframe. - private Map<Set<Class<? extends BuildConfiguration.Fragment>>, BuildConfiguration> - hostConfigurationCache = Maps.newConcurrentMap(); + private Map<FragmentClassSet, BuildConfiguration> hostConfigurationCache = + Maps.newConcurrentMap(); private BuildConfigurationCollection configurations; @@ -545,10 +546,10 @@ public final class SkyframeBuildView { // trims a host configuration to the same scope as a target configuration. Since their options // are different, the host instance may actually be able to produce the fragment. So it's // wrong and potentially dangerous to unilaterally exclude it. - Set<Class<? extends BuildConfiguration.Fragment>> fragmentClasses = + FragmentClassSet fragmentClasses = config.trimConfigurations() ? config.fragmentClasses() - : ruleClassProvider.getAllFragments(); + : FragmentClassSet.of(ruleClassProvider.getAllFragments()); BuildConfiguration hostConfig = hostConfigurationCache.get(fragmentClasses); if (hostConfig != null) { return hostConfig; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java index 4c5a674f33..e627332dab 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java @@ -14,12 +14,12 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSortedSet; import com.google.devtools.build.lib.analysis.DependencyResolver; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.DynamicTransitionMapper; +import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -126,8 +126,7 @@ public final class SkyframeDependencyResolver extends DependencyResolver { @Nullable @Override protected List<BuildConfiguration> getConfigurations( - ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragments, - Iterable<BuildOptions> buildOptions) + FragmentClassSet fragments, Iterable<BuildOptions> buildOptions) throws InvalidConfigurationException, InterruptedException { List<SkyKey> keys = new ArrayList<>(); for (BuildOptions options : buildOptions) { 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 cca618a403..d7da8c0778 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 @@ -71,6 +71,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollectio import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; import com.google.devtools.build.lib.analysis.config.ConfigurationResolver; +import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; @@ -1623,7 +1624,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { @VisibleForTesting public BuildConfiguration getConfigurationForTesting( ExtendedEventHandler eventHandler, - ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> fragments, + FragmentClassSet fragments, BuildOptions options) throws InterruptedException { SkyKey key = BuildConfigurationValue.key(fragments, options); |