Skip to content

Commit

Permalink
Reduce constant factor in findActionsRecursively by using a CompactHa…
Browse files Browse the repository at this point in the history
…shSet

instead of a HashSet and by providing a single-node getDirectDeps function.
This improves runtime of the function by about 30%.

RELNOTES: None.
PiperOrigin-RevId: 236942673
  • Loading branch information
djasper authored and copybara-github committed Mar 6, 2019
1 parent 3b08beb commit 180eeef
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import com.google.devtools.build.lib.causes.LoadingFailedCause;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
Expand Down Expand Up @@ -928,7 +929,7 @@ private Iterable<ActionLookupValue> getActionLookupValuesInBuild(
Iterable<ConfiguredTargetKey> ctKeys, Iterable<AspectValueKey> aspectKeys)
throws InterruptedException {
WalkableGraph walkableGraph = SkyframeExecutorWrappingWalkableGraph.of(skyframeExecutor);
Set<SkyKey> seen = new HashSet<>();
Set<SkyKey> seen = CompactHashSet.create();
List<ActionLookupValue> result = new ArrayList<>();
for (ConfiguredTargetKey key : ctKeys) {
findActionsRecursively(walkableGraph, key, seen, result);
Expand Down Expand Up @@ -956,11 +957,8 @@ private static void findActionsRecursively(
if (value instanceof ActionLookupValue) {
result.add((ActionLookupValue) value);
}
for (Map.Entry<SkyKey, Iterable<SkyKey>> deps :
walkableGraph.getDirectDeps(ImmutableList.of(key)).entrySet()) {
for (SkyKey dep : deps.getValue()) {
findActionsRecursively(walkableGraph, dep, seen, result);
}
for (SkyKey dep : walkableGraph.getDirectDeps(key)) {
findActionsRecursively(walkableGraph, dep, seen, result);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package com.google.devtools.build.skyframe;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.devtools.build.skyframe.QueryableGraph.Reason;
Expand All @@ -34,7 +33,7 @@ public DelegatingWalkableGraph(QueryableGraph graph) {

@Nullable
private NodeEntry getEntryForValue(SkyKey key) throws InterruptedException {
NodeEntry entry = getBatch(null, Reason.WALKABLE_GRAPH_VALUE, ImmutableList.of(key)).get(key);
NodeEntry entry = graph.get(null, Reason.WALKABLE_GRAPH_VALUE, key);
return entry != null && entry.isDone() ? entry : null;
}

Expand Down Expand Up @@ -116,6 +115,14 @@ public Map<SkyKey, Iterable<SkyKey>> getDirectDeps(Iterable<SkyKey> keys)
return result;
}

@Override
public Iterable<SkyKey> getDirectDeps(SkyKey key) throws InterruptedException {
NodeEntry entry = getEntryForValue(key);
Preconditions.checkNotNull(entry, key);
Preconditions.checkState(entry.isDone(), "Node %s (with key %s) isn't done yet.", entry, key);
return entry.getDirectDeps();
}

@Override
public Map<SkyKey, Iterable<SkyKey>> getReverseDeps(Iterable<SkyKey> keys)
throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ public interface WalkableGraph {
*/
Map<SkyKey, Iterable<SkyKey>> getDirectDeps(Iterable<SkyKey> keys) throws InterruptedException;

/**
* Returns the direct dependencies of the node with the given key. A node for that key must exist
* in the graph and be done.
*/
Iterable<SkyKey> getDirectDeps(SkyKey key) throws InterruptedException;

/**
* Returns a map giving the reverse dependencies of the nodes with the given keys. A node for each
* given key must be done in the graph if it exists.
Expand Down

0 comments on commit 180eeef

Please sign in to comment.