Skip to content

Commit

Permalink
Clean up critical path statistics.
Browse files Browse the repository at this point in the history 
- Delete old (unsued) way to compute critical paths.
- There's no need to implement our own linked list ;-)
- There are also no fake entries (anymore?).

RELNOTES: None
PiperOrigin-RevId: 289404256
  • Loading branch information
@meisterT @copybara-github
meisterT authored and copybara-github committed Jan 13, 2020
1 parent f297ba8 commit 63de242
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 182 deletions.
124 changes: 0 additions & 124 deletions src/main/java/com/google/devtools/build/lib/profiler/analysis/ProfileInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.profiler.analysis;

import static com.google.devtools.build.lib.profiler.ProfilerTask.CRITICAL_PATH;
import static com.google.devtools.build.lib.profiler.ProfilerTask.TASK_COUNT;

import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
Expand All @@ -31,13 +29,11 @@
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
import java.nio.ByteBuffer;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.Deque;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -202,10 +198,6 @@ public boolean hasStats() {
return !stats.isEmpty();
}

public boolean isFake() {
return id < 0;
}

public long getInheritedDuration() {
return stats.getTotalTime();
}
Expand Down Expand Up @@ -277,24 +269,6 @@ public int compareTo(Task task) {
}
}

/**
* Represents node on critical build path
*/
public static final class CriticalPathEntry {
public final Task task;
public final long duration;
public final long cumulativeDuration;
public final CriticalPathEntry next;

public CriticalPathEntry(Task task, long duration, CriticalPathEntry next) {
this.task = task;
this.duration = duration;
this.next = next;
this.cumulativeDuration =
duration + (next != null ? next.cumulativeDuration : 0);
}
}

/**
* Helper class to create space-efficient task multimap, used to associate
* array of tasks with specific key.
Expand Down Expand Up @@ -352,15 +326,12 @@ public interface InfoListener {
public List<Task> rootTasksById; // Not final due to the late initialization.
public final List<Task> phaseTasks;

public final Map<Task, Task[]> actionDependencyMap;

private ProfileInfo(String comment) {
this.comment = comment;

descriptionList = Lists.newArrayListWithExpectedSize(10000);
allTasksById = Lists.newArrayListWithExpectedSize(50000);
phaseTasks = Lists.newArrayList();
actionDependencyMap = Maps.newHashMapWithExpectedSize(10000);
}

private void addTask(Task task) {
Expand Down Expand Up @@ -493,101 +464,6 @@ public long getPhaseDuration(Task phaseTask) {
return duration;
}

/**
* Calculates critical path for the specific action excluding specified nested task types (e.g.
* VFS-related time) and not accounting for overhead related to the Blaze scheduler.
*/
private CriticalPathEntry computeCriticalPathForAction(
Set<Task> ignoredTasks,
Task actionTask,
Map<Task, CriticalPathEntry> cache,
Deque<Task> stack) {

// Loop check is expensive for the Deque (and we don't want to use hash sets because adding
// and removing elements was shown to be very expensive). To avoid quadratic costs we're
// checking for infinite loop only when deque's size equal to the power of 2 and >= 32.
if ((stack.size() & 0x1F) == 0 && Integer.bitCount(stack.size()) == 1) {
if (stack.contains(actionTask)) {
// This situation will appear if build has ended with the
// IllegalStateException thrown by the
// ParallelBuilder.getNextCompletedAction(), warning user about
// possible cycle in the dependency graph. But the exception text
// is more friendly and will actually identify the loop.
// Do not use Preconditions class below due to the very expensive
// toString() calls used in the message.
throw new IllegalStateException ("Dependency graph contains loop:\n"
+ actionTask + " in the\n" + Joiner.on('\n').join(stack));
}
}
stack.addLast(actionTask);
CriticalPathEntry entry;
try {
entry = cache.get(actionTask);
long entryDuration = 0;
if (entry == null) {
Task[] actionPrerequisites = actionDependencyMap.get(actionTask);
if (actionPrerequisites != null) {
for (Task task : actionPrerequisites) {
CriticalPathEntry candidate =
computeCriticalPathForAction(ignoredTasks, task, cache, stack);
if (entry == null || entryDuration < candidate.cumulativeDuration) {
entry = candidate;
entryDuration = candidate.cumulativeDuration;
}
}
}
if (actionTask.type == ProfilerTask.ACTION) {
long duration = actionTask.durationNanos;
if (ignoredTasks.contains(actionTask)) {
duration = 0L;
}

entry = new CriticalPathEntry(actionTask, duration, entry);
cache.put(actionTask, entry);
}
}
} finally {
stack.removeLast();
}
return entry;
}

/**
* Returns the critical path information from the {@code CriticalPathComputer} recorded stats.
* This code does not have the "Critical" column (Time difference if we removed this node from
* the critical path).
*/
public CriticalPathEntry getCriticalPathNewVersion() {
for (Task task : rootTasksById) {
if (task.type == CRITICAL_PATH) {
CriticalPathEntry entry = null;
for (Task shared : task.subtasks) {
entry = new CriticalPathEntry(shared, shared.durationNanos, entry);
}
return entry;
}
}
return null;
}

/**
* Calculates critical path for the given action graph excluding specified tasks (usually ones
* that belong to the "real" critical path).
*/
public CriticalPathEntry getCriticalPath() {
Task actionTask = getPhaseTask(ProfilePhase.EXECUTE);
if (actionTask == null) {
return null;
}
Map <Task, CriticalPathEntry> cache = Maps.newHashMapWithExpectedSize(1000);
CriticalPathEntry result =
computeCriticalPathForAction(new HashSet<>(), actionTask, cache, new ArrayDeque<>());
if (result != null) {
return result;
}
return getCriticalPathNewVersion();
}

/**
* Returns an empty array used to store task statistics. Array index
* corresponds to the ProfilerTask ordinal() value associated with the
Expand Down
22 changes: 8 additions & 14 deletions src/main/java/com/google/devtools/build/lib/profiler/output/CriticalPathText.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.profiler.output;

import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.CriticalPathEntry;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.statistics.CriticalPathStatistics;
import com.google.devtools.build.lib.util.TimeUtilities;
import java.io.PrintStream;
Expand All @@ -33,23 +33,17 @@ public CriticalPathText(PrintStream out, CriticalPathStatistics critPathStats) {
* Print total and optimal critical paths if available.
*/
public void printCriticalPaths() {
CriticalPathEntry totalPath = criticalPathStats.getTotalPath();
printCriticalPath("Critical path", totalPath);
}

private void printCriticalPath(String title, CriticalPathEntry path) {
lnPrintf("%s (%s):", title, TimeUtilities.prettyTime(path.cumulativeDuration));
long totalPathTimeNanos = criticalPathStats.getTotalDuration().toNanos();
lnPrintf("%s (%s):", "Critical path", TimeUtilities.prettyTime(totalPathTimeNanos));
lnPrintf("%6s %11s %8s %s", "Id", "Time", "Percentage", "Description");

long totalPathTime = path.cumulativeDuration;

for (CriticalPathEntry pathEntry : criticalPathStats.getFilteredPath(path)) {
String desc = pathEntry.task.getDescription().replace(':', ' ');
for (Task pathEntry : criticalPathStats.getCriticalPathEntries()) {
String desc = pathEntry.getDescription().replace(':', ' ');
lnPrintf(
"%6d %11s %8s %s",
pathEntry.task.id,
TimeUtilities.prettyTime(pathEntry.duration),
prettyPercentage((double) pathEntry.duration / totalPathTime),
pathEntry.id,
TimeUtilities.prettyTime(pathEntry.durationNanos),
prettyPercentage((double) pathEntry.durationNanos / totalPathTimeNanos),
desc);
}
}
Expand Down
68 changes: 24 additions & 44 deletions src/main/java/com/google/devtools/build/lib/profiler/statistics/CriticalPathStatistics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,61 +13,41 @@
// limitations under the License.
package com.google.devtools.build.lib.profiler.statistics;

import com.google.devtools.build.lib.profiler.ProfilerTask;
import static com.google.devtools.build.lib.profiler.ProfilerTask.CRITICAL_PATH;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.CriticalPathEntry;
import java.util.Iterator;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.Task;
import java.time.Duration;
import java.util.ArrayList;

/**
* Keeps a predefined list of {@link CriticalPathEntry}'s cumulative durations and allows iterating
* over pairs of their descriptions and relative durations.
* Keeps a predefined list of {@link Task}'s cumulative durations and allows iterating over pairs of
* their descriptions and relative durations.
*/
public final class CriticalPathStatistics {
/**
* The actual critical path.
*/
private final CriticalPathEntry totalPath;
private final ImmutableList<Task> criticalPathEntries;
private Duration totalDuration = Duration.ZERO;

public CriticalPathStatistics(ProfileInfo info) {
totalPath = info.getCriticalPath();
ArrayList<Task> criticalPathEntries = new ArrayList<>();
for (Task task : info.rootTasksById) {
if (task.type == CRITICAL_PATH) {
for (Task criticalPathEntry : task.subtasks) {
totalDuration = totalDuration.plus(Duration.ofNanos(criticalPathEntry.durationNanos));
criticalPathEntries.add(criticalPathEntry);
}
}
}
this.criticalPathEntries = ImmutableList.copyOf(criticalPathEntries);
}

/**
* @return the critical path obtained by not filtering out any {@link ProfilerTask}
*/
public CriticalPathEntry getTotalPath() {
return totalPath;
public Duration getTotalDuration() {
return totalDuration;
}

/**
* Constructs a filtered Iterable from a critical path.
*
* <p>Ignores all fake (task id < 0) path entries.
*/
public Iterable<CriticalPathEntry> getFilteredPath(final CriticalPathEntry path) {
return () ->
new Iterator<CriticalPathEntry>() {
private CriticalPathEntry nextEntry = path;

@Override
public boolean hasNext() {
return nextEntry != null;
}

@Override
public CriticalPathEntry next() {
CriticalPathEntry current = nextEntry;
do {
nextEntry = nextEntry.next;
} while (nextEntry != null && nextEntry.task.isFake());
return current;
}

@Override
public void remove() {
throw new UnsupportedOperationException();
}
};
public ImmutableList<Task> getCriticalPathEntries() {
return criticalPathEntries;
}
}

0 comments on commit 63de242

Please sign in to comment.