Skip to content

Commit

Permalink
Use concrete collections, not lazy set difference/intersection/filter…
Browse files Browse the repository at this point in the history
… views, on hot codepaths in the Skyframe engine code, when we'll be iterating more than once.

PiperOrigin-RevId: 519045507
Change-Id: Iaf0b2c6cfb90b9fbe1bd8c0bd3867b3985baa5b9
  • Loading branch information
haxorz authored and copybara-github committed Mar 24, 2023
1 parent a30e255 commit 718a916
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -761,15 +762,17 @@ public void run() {
// "newDeps" refers to newly discovered this time around after a SkyFunction#compute call
// and not to be confused with the oldDeps variable which refers to the last evaluation,
// i.e. a prior call to ParallelEvaluator#eval.
Set<SkyKey> newDepsThatWerentInTheLastEvaluation;
Set<SkyKey> newDepsThatWereInTheLastEvaluation;
Collection<SkyKey> newDepsThatWerentInTheLastEvaluation;
ImmutableList<SkyKey> newDepsThatWereInTheLastEvaluation;
if (oldDeps.isEmpty()) {
// When there are no old deps (clean evaluations), avoid set views which have O(n) size.
newDepsThatWerentInTheLastEvaluation = newDeps;
newDepsThatWereInTheLastEvaluation = ImmutableSet.of();
newDepsThatWereInTheLastEvaluation = ImmutableList.of();
} else {
newDepsThatWerentInTheLastEvaluation = Sets.difference(newDeps, oldDeps);
newDepsThatWereInTheLastEvaluation = Sets.intersection(newDeps, oldDeps);
newDepsThatWerentInTheLastEvaluation =
ImmutableList.copyOf(Sets.difference(newDeps, oldDeps));
newDepsThatWereInTheLastEvaluation =
ImmutableList.copyOf(Sets.intersection(newDeps, oldDeps));
}

InterruptibleSupplier<NodeBatch> newDepsThatWerentInTheLastEvaluationNodes =
Expand Down Expand Up @@ -1026,15 +1029,14 @@ private boolean maybeHandleRegisteringNewlyDiscoveredDepsForDoneEntry(
}

env.addTemporaryDirectDepsTo(entry);
Set<SkyKey> newlyAddedNewDeps;
Set<SkyKey> previouslyRegisteredNewDeps;
Collection<SkyKey> newlyAddedNewDeps;
ImmutableCollection<SkyKey> previouslyRegisteredNewDeps;
if (oldDeps.isEmpty()) {
// When there are no old deps (clean evaluations), avoid set views which have O(n) size.
newlyAddedNewDeps = newDeps;
previouslyRegisteredNewDeps = ImmutableSet.of();
} else {
newlyAddedNewDeps = Sets.difference(newDeps, oldDeps);
previouslyRegisteredNewDeps = Sets.intersection(newDeps, oldDeps);
newlyAddedNewDeps = ImmutableList.copyOf(Sets.difference(newDeps, oldDeps));
previouslyRegisteredNewDeps = ImmutableList.copyOf(Sets.intersection(newDeps, oldDeps));
}

InterruptibleSupplier<NodeBatch> newlyAddedNewDepNodes =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,8 @@ Set<SkyKey> commitAndGetParents(NodeEntry primaryEntry) throws InterruptedExcept
GroupedDeps temporaryDirectDeps = primaryEntry.getTemporaryDirectDeps();
if (!oldDeps.isEmpty()) {
// Remove the rdep on this entry for each of its old deps that is no longer a direct dep.
Set<SkyKey> depsToRemove = Sets.difference(oldDeps, temporaryDirectDeps.toSet());
ImmutableList<SkyKey> depsToRemove =
ImmutableList.copyOf(Sets.difference(oldDeps, temporaryDirectDeps.toSet()));
NodeBatch oldDepEntries =
evaluatorContext.getGraph().getBatch(skyKey, Reason.RDEP_REMOVAL, depsToRemove);
for (SkyKey key : depsToRemove) {
Expand Down

0 comments on commit 718a916

Please sign in to comment.