From c8098d0e089d84268877bd30d4b1c0eae110deb3 Mon Sep 17 00:00:00 2001 From: michajlo Date: Fri, 29 Sep 2017 22:45:11 +0200 Subject: Only create builtins for rules once per PackageFactory Previously we'd do this on demand. This simplifies a bit. PiperOrigin-RevId: 170526646 --- .../build/lib/packages/PackageFactory.java | 107 +++++++++++---------- .../devtools/build/lib/packages/RuleFactory.java | 4 +- 2 files changed, 60 insertions(+), 51 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index f9c00884a2..53c8f0af1b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -76,6 +76,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.TreeMap; import java.util.concurrent.Future; @@ -331,6 +332,7 @@ public final class PackageFactory { private static final Logger logger = Logger.getLogger(PackageFactory.class.getName()); private final RuleFactory ruleFactory; + private final ImmutableMap ruleFunctions; private final RuleClassProvider ruleClassProvider; private AtomicReference syscalls; @@ -406,6 +408,7 @@ public final class PackageFactory { Package.Builder.Helper packageBuilderHelper) { this.platformSetRegexps = platformSetRegexps; this.ruleFactory = new RuleFactory(ruleClassProvider, attributeContainerFactory); + this.ruleFunctions = buildRuleFunctions(ruleFactory); this.ruleClassProvider = ruleClassProvider; threadPool = new ThreadPoolExecutor(100, 100, 15L, TimeUnit.SECONDS, new LinkedBlockingQueue(), @@ -589,7 +592,7 @@ public final class PackageFactory { } GlobList globList = GlobList.captureResults(includes, excludes, matches); - return new MutableList(globList, env); + return new MutableList<>(globList, env); } /** @@ -1187,26 +1190,6 @@ public final class PackageFactory { }; } - // Helper function for createRuleFunction. - private static void addRule(RuleFactory ruleFactory, - String ruleClassName, - PackageContext context, - Map kwargs, - FuncallExpression ast, - Environment env) - throws RuleFactory.InvalidRuleException, Package.NameConflictException, InterruptedException { - RuleClass ruleClass = getBuiltInRuleClass(ruleClassName, ruleFactory); - BuildLangTypedAttributeValuesMap attributeValues = new BuildLangTypedAttributeValuesMap(kwargs); - AttributeContainer attributeContainer = ruleFactory.getAttributeContainer(ruleClass); - RuleFactory.createAndAddRule(context, ruleClass, attributeValues, ast, env, attributeContainer); - } - - private static RuleClass getBuiltInRuleClass(String ruleClassName, RuleFactory ruleFactory) { - if (ruleFactory.getRuleClassNames().contains(ruleClassName)) { - return ruleFactory.getRuleClass(ruleClassName); - } - throw new IllegalArgumentException("no such rule class: " + ruleClassName); - } /** * Get the PackageContext by looking up in the environment. @@ -1224,28 +1207,56 @@ public final class PackageFactory { return value; } - /** - * Returns a function-value implementing the build rule "ruleClass" (e.g. cc_library) in the - * specified package context. - */ - private static BuiltinFunction newRuleFunction( - final RuleFactory ruleFactory, final String ruleClass) { - return new BuiltinFunction( - ruleClass, FunctionSignature.KWARGS, BuiltinFunction.USE_AST_ENV, /*isRule=*/ true) { - - @SuppressWarnings({"unchecked", "unused"}) - public Runtime.NoneType invoke( - Map kwargs, FuncallExpression ast, Environment env) - throws EvalException, InterruptedException { - env.checkLoadingOrWorkspacePhase(ruleClass, ast.getLocation()); - try { - addRule(ruleFactory, ruleClass, getContext(env, ast), kwargs, ast, env); - } catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) { - throw new EvalException(ast.getLocation(), e.getMessage()); - } - return Runtime.NONE; + private static ImmutableMap buildRuleFunctions(RuleFactory ruleFactory) { + ImmutableMap.Builder result = ImmutableMap.builder(); + for (String ruleClass : ruleFactory.getRuleClassNames()) { + result.put(ruleClass, new RuleFunction(ruleClass, ruleFactory)); + } + return result.build(); + } + + /** {@link BuiltinFunction} adapter for creating {@link Rule}s for native {@link RuleClass}es. */ + private static class RuleFunction extends BuiltinFunction { + private final String ruleClassName; + private final RuleFactory ruleFactory; + private final RuleClass ruleClass; + + RuleFunction(String ruleClassName, RuleFactory ruleFactory) { + super(ruleClassName, FunctionSignature.KWARGS, BuiltinFunction.USE_AST_ENV, /*isRule=*/ true); + this.ruleClassName = ruleClassName; + this.ruleFactory = Preconditions.checkNotNull(ruleFactory, "ruleFactory was null"); + this.ruleClass = Preconditions.checkNotNull( + ruleFactory.getRuleClass(ruleClassName), + "No such rule class: %s", + ruleClassName); + } + + @SuppressWarnings({"unchecked", "unused"}) + public Runtime.NoneType invoke( + Map kwargs, FuncallExpression ast, Environment env) + throws EvalException, InterruptedException { + env.checkLoadingOrWorkspacePhase(ruleClassName, ast.getLocation()); + try { + addRule(getContext(env, ast), kwargs, ast, env); + } catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) { + throw new EvalException(ast.getLocation(), e.getMessage()); } - }; + return Runtime.NONE; + } + + private void addRule( + PackageContext context, + Map kwargs, + FuncallExpression ast, + Environment env) + throws RuleFactory.InvalidRuleException, Package.NameConflictException, + InterruptedException { + BuildLangTypedAttributeValuesMap attributeValues = + new BuildLangTypedAttributeValuesMap(kwargs); + AttributeContainer attributeContainer = ruleFactory.getAttributeContainer(ruleClass); + RuleFactory.createAndAddRule( + context, ruleClass, attributeValues, ast, env, attributeContainer); + } } /** @@ -1542,9 +1553,7 @@ public final class PackageFactory { for (String nativeFunction : Runtime.getFunctionNames(SkylarkNativeModule.class)) { builder.put(nativeFunction, Runtime.getFunction(SkylarkNativeModule.class, nativeFunction)); } - for (String ruleClass : ruleFactory.getRuleClassNames()) { - builder.put(ruleClass, newRuleFunction(ruleFactory, ruleClass)); - } + builder.putAll(ruleFunctions); builder.put("package", newPackageFunction(packageArguments)); for (EnvironmentExtension extension : environmentExtensions) { for (BaseFunction function : extension.nativeModuleFunctions()) { @@ -1558,7 +1567,6 @@ public final class PackageFactory { private void buildPkgEnv( Environment pkgEnv, PackageContext context, - RuleFactory ruleFactory, PackageIdentifier packageId) { // TODO(bazel-team): remove the naked functions that are redundant with the nativeModule, // or if not possible, at least make them straight copies from the native module variant. @@ -1577,9 +1585,8 @@ public final class PackageFactory { .setup("repository_name", SkylarkNativeModule.repositoryName) .setup("environment_group", newEnvironmentGroupFunction.apply(context)); - for (String ruleClass : ruleFactory.getRuleClassNames()) { - BaseFunction ruleFunction = newRuleFunction(ruleFactory, ruleClass); - pkgEnv.setup(ruleClass, ruleFunction); + for (Entry entry : ruleFunctions.entrySet()) { + pkgEnv.setup(entry.getKey(), entry.getValue()); } for (EnvironmentExtension extension : environmentExtensions) { @@ -1663,7 +1670,7 @@ public final class PackageFactory { PackageContext context = new PackageContext( pkgBuilder, globber, eventHandler, ruleFactory.getAttributeContainerFactory()); - buildPkgEnv(pkgEnv, context, ruleFactory, packageId); + buildPkgEnv(pkgEnv, context, packageId); if (containsError) { pkgBuilder.setContainsErrors(); diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java index 0ff628e165..ba3608035a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java @@ -38,6 +38,8 @@ import javax.annotation.Nullable; * performs a number of checks and associates the {@link Rule} and the owning {@link Package} * with each other. * + *

This class is immutable, once created the set of managed {@link RuleClass}es will not change. + * *

Note: the code that actually populates the RuleClass map has been moved to {@link * RuleClassProvider}. */ @@ -338,7 +340,7 @@ public class RuleFactory { FuncallExpression generator = topCall.first; BaseFunction function = topCall.second; String name = generator.getNameArg(); - + ImmutableMap.Builder builder = ImmutableMap.builder(); for (Map.Entry attributeAccessor : args.getAttributeAccessors()) { String attributeName = args.getName(attributeAccessor); -- cgit v1.2.3