Skip to content

Commit

Permalink
Refactor InMemoryNodeEntry into an interface.
Browse files Browse the repository at this point in the history
Right now its only direct implementation is `IncrementalInMemoryNodeEntry`, but soon `EdglessInMemoryNodeEntry` will get its own dedicated implementation instead of inheriting from incremental.

PiperOrigin-RevId: 553809945
Change-Id: I9abcc5d503ce7890903b5cd852b2637d63fc7bf9
  • Loading branch information
justinhorvitz authored and copybara-github committed Aug 4, 2023
1 parent 6a313f1 commit 066db19
Show file tree
Hide file tree
Showing 13 changed files with 760 additions and 682 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package com.google.devtools.build.skyframe;

import com.google.common.collect.ImmutableSet;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -198,7 +199,7 @@ public void removeInProgressReverseDep(SkyKey reverseDep) {
}

@Override
public Iterable<SkyKey> getReverseDepsForDoneEntry() throws InterruptedException {
public Collection<SkyKey> getReverseDepsForDoneEntry() throws InterruptedException {
return getDelegate().getReverseDepsForDoneEntry();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ static DirtyBuildingState createNew(boolean hasLowFanout) {
* needs to be re-scheduled, and ensures that only one thread gets a true return value.
*
* <p>The second problem is solved by first adding the newly discovered deps to a node's {@link
* InMemoryNodeEntry#directDeps}, and then looping through the direct deps and registering this
* node as a reverse dependency. This ensures that the signaledDeps counter can only reach {@link
* GroupedDeps#numElements} on the very last iteration of the loop, i.e., the thread is not
* working on the node anymore. Note that this requires that there is no code after the loop in
* {@link ParallelEvaluator.Evaluate#run}.
* IncrementalInMemoryNodeEntry#directDeps}, and then looping through the direct deps and
* registering this node as a reverse dependency. This ensures that the signaledDeps counter can
* only reach {@link GroupedDeps#numElements} on the very last iteration of the loop, i.e., the
* thread is not working on the node anymore. Note that this requires that there is no code after
* the loop in {@link ParallelEvaluator.Evaluate#run}.
*/
private int signaledDeps = NOT_EVALUATING_SENTINEL;

Expand Down Expand Up @@ -114,7 +114,7 @@ static DirtyBuildingState createNew(boolean hasLowFanout) {
* value last time the node was built). They will be compared to dependencies requested on this
* build to check whether this node has changed in {@link NodeEntry#setValue}. If they are null,
* it means that this node is being built for the first time. See {@link
* InMemoryNodeEntry#directDeps} for more on dependency group storage.
* IncrementalInMemoryNodeEntry#directDeps} for more on dependency group storage.
*
* <p>Public only for the use of alternative graph implementations.
*/
Expand Down Expand Up @@ -216,7 +216,9 @@ private void checkFinishedBuildingWhenAboutToSetValue() {
* DirtyBuildingState#getNumOfGroupsInLastBuildDirectDeps()}.
*/
final void signalDep(
InMemoryNodeEntry entry, Version childVersion, @Nullable SkyKey childForDebugging) {
IncrementalInMemoryNodeEntry entry,
Version childVersion,
@Nullable SkyKey childForDebugging) {
// Synchronization isn't needed here because the only caller is InMemoryNodeEntry, which does it
// through the synchronized method signalDep.
checkState(isEvaluating(), "%s %s", entry, childForDebugging);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
* while the node is being built, but will not be stored once the node is done and written to the
* graph. Any attempt to access the edges once the node is done will fail the build fast.
*/
public class EdgelessInMemoryNodeEntry extends InMemoryNodeEntry {
// TODO(b/262871730): Do not extend IncrementalInMemoryNodeEntry.
public class EdgelessInMemoryNodeEntry extends IncrementalInMemoryNodeEntry {

public EdgelessInMemoryNodeEntry(SkyKey key) {
super(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public Map<SkyKey, NodeEntry> getBatchMap(

@ForOverride
protected InMemoryNodeEntry newNodeEntry(SkyKey key) {
return new InMemoryNodeEntry(key);
return new IncrementalInMemoryNodeEntry(key);
}

@Override
Expand Down
Loading

0 comments on commit 066db19

Please sign in to comment.