From f1e0d346c8235c855e61afc2adb870e4b895e002 Mon Sep 17 00:00:00 2001 From: John Cater Date: Fri, 5 Mar 2021 13:44:19 -0800 Subject: [PATCH] Clean up RuleContext to use a Table instead of a Map of Maps. Closes #13164. PiperOrigin-RevId: 361216667 --- .../build/lib/analysis/RuleContext.java | 66 +++++++------------ 1 file changed, 25 insertions(+), 41 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 359182081364fe..12da782bb7d47e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.ImmutableTable; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; @@ -217,7 +218,7 @@ public ImmutableList getFiles() { private final ConstraintSemantics constraintSemantics; private final ImmutableSet requiredConfigFragments; private final List makeVariableExpanders = new ArrayList<>(); - private final ImmutableMap> execProperties; + private final ImmutableTable execProperties; /** Map of exec group names to ActionOwners. */ private final Map actionOwners = new HashMap<>(); @@ -635,17 +636,16 @@ private static ImmutableMap computeExecProperties( Map execProperties = new HashMap<>(); if (executionPlatform != null) { - Map> execPropertiesPerGroup = + ImmutableTable execPropertiesPerGroup = parseExecGroups(executionPlatform.execProperties()); - if (execPropertiesPerGroup.containsKey(DEFAULT_EXEC_GROUP_NAME)) { - execProperties.putAll(execPropertiesPerGroup.get(DEFAULT_EXEC_GROUP_NAME)); - execPropertiesPerGroup.remove(DEFAULT_EXEC_GROUP_NAME); + if (execPropertiesPerGroup.containsRow(DEFAULT_EXEC_GROUP_NAME)) { + execProperties.putAll(execPropertiesPerGroup.row(DEFAULT_EXEC_GROUP_NAME)); } - for (Map.Entry> execGroup : execPropertiesPerGroup.entrySet()) { - if (execGroups.contains(execGroup.getKey())) { - execProperties.putAll(execGroup.getValue()); + for (String execGroup : execPropertiesPerGroup.rowKeySet()) { + if (execGroups.contains(execGroup)) { + execProperties.putAll(execPropertiesPerGroup.row(execGroup)); } } } @@ -1404,10 +1404,10 @@ public ImmutableSortedSet getRequiredConfigFragments() { return ans.build(); } - private ImmutableMap> parseExecProperties( + private ImmutableTable parseExecProperties( Map execProperties) throws InvalidExecGroupException { if (execProperties.isEmpty()) { - return ImmutableMap.of(DEFAULT_EXEC_GROUP_NAME, ImmutableMap.of()); + return ImmutableTable.of(); } else { return parseExecProperties( execProperties, toolchainContexts == null ? null : toolchainContexts.getExecGroups()); @@ -1420,25 +1420,21 @@ private ImmutableMap> parseExecProperties( * former get parsed into the default exec group, the latter get parsed into their relevant exec * groups. */ - private static Map> parseExecGroups( + private static ImmutableTable parseExecGroups( Map rawExecProperties) { - Map> consolidatedProperties = new HashMap<>(); - consolidatedProperties.put(DEFAULT_EXEC_GROUP_NAME, new HashMap<>()); + ImmutableTable.Builder execProperties = ImmutableTable.builder(); for (Map.Entry execProperty : rawExecProperties.entrySet()) { String rawProperty = execProperty.getKey(); int delimiterIndex = rawProperty.indexOf('.'); if (delimiterIndex == -1) { - consolidatedProperties - .get(DEFAULT_EXEC_GROUP_NAME) - .put(rawProperty, execProperty.getValue()); + execProperties.put(DEFAULT_EXEC_GROUP_NAME, rawProperty, execProperty.getValue()); } else { String execGroup = rawProperty.substring(0, delimiterIndex); String property = rawProperty.substring(delimiterIndex + 1); - consolidatedProperties.putIfAbsent(execGroup, new HashMap<>()); - consolidatedProperties.get(execGroup).put(property, execProperty.getValue()); + execProperties.put(execGroup, property, execProperty.getValue()); } } - return consolidatedProperties; + return execProperties.build(); } /** @@ -1446,13 +1442,13 @@ private static Map> parseExecGroups( * If given a set of exec groups, validates all the exec groups in the map are applicable to the * action. */ - private static ImmutableMap> parseExecProperties( + private static ImmutableTable parseExecProperties( Map rawExecProperties, @Nullable Set execGroups) throws InvalidExecGroupException { - Map> consolidatedProperties = parseExecGroups(rawExecProperties); + ImmutableTable consolidatedProperties = + parseExecGroups(rawExecProperties); if (execGroups != null) { - for (Map.Entry> execGroup : consolidatedProperties.entrySet()) { - String execGroupName = execGroup.getKey(); + for (String execGroupName : consolidatedProperties.rowKeySet()) { if (!execGroupName.equals(DEFAULT_EXEC_GROUP_NAME) && !execGroups.contains(execGroupName)) { throw new InvalidExecGroupException( String.format( @@ -1461,14 +1457,7 @@ private static ImmutableMap> parseExecPrope } } - // Copy everything to immutable maps. - ImmutableMap.Builder> execProperties = - new ImmutableMap.Builder<>(); - for (Map.Entry> execGroupMap : consolidatedProperties.entrySet()) { - execProperties.put(execGroupMap.getKey(), ImmutableMap.copyOf(execGroupMap.getValue())); - } - - return execProperties.build(); + return consolidatedProperties; } /** @@ -1480,16 +1469,16 @@ private static ImmutableMap> parseExecPrope * @param execProperties Map of exec group name to map of properties and values */ private static ImmutableMap getExecProperties( - String execGroup, Map> execProperties) { - if (!execProperties.containsKey(execGroup) || execGroup.equals(DEFAULT_EXEC_GROUP_NAME)) { - return execProperties.get(DEFAULT_EXEC_GROUP_NAME); + String execGroup, ImmutableTable execProperties) { + if (!execProperties.containsRow(execGroup) || execGroup.equals(DEFAULT_EXEC_GROUP_NAME)) { + return execProperties.row(DEFAULT_EXEC_GROUP_NAME); } // Use a HashMap to build here because we expect duplicate keys to happen // (and rewrite previous entries). Map targetAndGroupProperties = - new HashMap<>(execProperties.get(DEFAULT_EXEC_GROUP_NAME)); - targetAndGroupProperties.putAll(execProperties.get(execGroup)); + new HashMap<>(execProperties.row(DEFAULT_EXEC_GROUP_NAME)); + targetAndGroupProperties.putAll(execProperties.row(execGroup)); return ImmutableMap.copyOf(targetAndGroupProperties); } @@ -1510,11 +1499,6 @@ public DetailedExitCode getDetailedExitCode() { } } - @VisibleForTesting - public ImmutableMap> getExecPropertiesForTesting() { - return execProperties; - } - private void checkAttributeIsDependency(String attributeName) { Attribute attributeDefinition = attributes.getAttributeDefinition(attributeName); if (attributeDefinition == null) {