diff options
author | 2016-02-01 22:46:10 +0000 | |
---|---|---|
committer | 2016-02-02 14:57:47 +0000 | |
commit | a0eccb7441dfc7a3bcb6ef2e384cd1f63b9b03cb (patch) | |
tree | 88bd211285bb2793fd29bcab8ee381df2a1ffa15 /src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java | |
parent | cb0d65f2095f4998cd8905f1edca714e736a45d3 (diff) |
Trim the BuildOptions input of ConfigurationFragment.key to only those
options actually needed by the fragment. This protects against, e.g.,
unnecessarily duplicating CppConfiguration instances when only Java flags
change.
This is a recommit of ca1b21ac6d8a58041db822725b42de151b163dee which was
rolled back because it broke LIPO.
This change is particularly important for dynamic configurations, which may
mix and match fragments arbitrarily throughout a build. This not only has
performance implications, but also correctness implications: code that
expects two configured targets to have the same fragment (value) shouldn't
break just because the second CT's configuration is a trimmed version of the
first's.
The original change breaks FDO/LIPO because CppConfiguration can't be
shared across configurations. That's because it mutates state when
prepareHook() is called, and each configuration calls prepareHook. We
should ultimately solve this by refactoring the FDO/LIPO implementation
but don't want to block dynamic configuration progress on that. So this
change only enables trimming for dynamic configurations.
--
MOS_MIGRATED_REVID=113570250
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java | 8 |
1 files changed, 6 insertions, 2 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java index 8da3a9cc3a..ac541411cd 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.skyframe.ConfigurationFragmentValue.ConfigurationFragmentKey; import com.google.devtools.build.lib.vfs.Path; @@ -43,10 +44,13 @@ import java.io.IOException; public class ConfigurationFragmentFunction implements SkyFunction { private final Supplier<ImmutableList<ConfigurationFragmentFactory>> configurationFragments; + private final RuleClassProvider ruleClassProvider; public ConfigurationFragmentFunction( - Supplier<ImmutableList<ConfigurationFragmentFactory>> configurationFragments) { + Supplier<ImmutableList<ConfigurationFragmentFactory>> configurationFragments, + RuleClassProvider ruleClassProvider) { this.configurationFragments = configurationFragments; + this.ruleClassProvider = ruleClassProvider; } @Override @@ -58,7 +62,7 @@ public class ConfigurationFragmentFunction implements SkyFunction { ConfigurationFragmentFactory factory = getFactory(configurationFragmentKey.getFragmentType()); try { PackageProviderForConfigurations packageProvider = - new SkyframePackageLoaderWithValueEnvironment(env); + new SkyframePackageLoaderWithValueEnvironment(env, ruleClassProvider); ConfigurationEnvironment confEnv = new ConfigurationBuilderEnvironment(packageProvider); Fragment fragment = factory.create(confEnv, buildOptions); |