Skip to content

Commit

Permalink
Split the cycle between vfs and profiler.
Browse files Browse the repository at this point in the history
- Move ProfilerInfo into a subpackage (it's not necessary for profiling,
  just for analyzing a profile).
- Make some fields in Profiler public for ProfileInfo.
- Mark Profiler as ThreadSafe; there's no cyclic dependency here.
  • Loading branch information
ulfjack committed May 28, 2017
1 parent 4fdd3d4 commit 44553fc
Show file tree
Hide file tree
Showing 19 changed files with 63 additions and 39 deletions.
21 changes: 20 additions & 1 deletion src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,37 @@ java_library(
],
)

java_library(
name = "profiler",
srcs = glob([
"profiler/*.java",
]),
deps = [
":base-util",
":clock",
":concurrent",
":preconditions",
"//third_party:guava",
],
)

# Virtual file system; do not use externally!
java_library(
name = "vfs",
srcs = glob([
"profiler/*.java",
"vfs/*.java",
]),
visibility = ["//visibility:public"],
exports = [
":profiler",
],
deps = [
":base-util",
":clock",
":concurrent",
":os_util",
":preconditions",
":profiler",
"//src/main/java/com/google/devtools/common/options",
"//src/main/java/com/google/devtools/build/lib/shell",
"//third_party:guava",
Expand All @@ -123,13 +140,15 @@ java_library(
java_library(
name = "profiler-output",
srcs = glob([
"profiler/analysis/*.java",
"profiler/chart/*.java",
"profiler/output/*.java",
"profiler/statistics/*.java",
]),
deps = [
":util",
":vfs",
":profiler",
"//src/main/java/com/google/devtools/build/lib/actions",
"//third_party:guava",
"//third_party:jsr305",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.profiler.PredicateBasedStatRecorder.RecorderAndPredicate;
import com.google.devtools.build.lib.profiler.StatRecorder.VfsHeuristics;
import com.google.devtools.build.lib.util.Clock;
Expand Down Expand Up @@ -115,19 +116,19 @@
*
* @see ProfilerTask enum for recognized task types.
*/
//@ThreadSafe - commented out to avoid cyclic dependency with lib.util package
@ThreadSafe
public final class Profiler {
private static final Logger LOG = Logger.getLogger(Profiler.class.getName());

static final int MAGIC = 0x11223344;
public static final int MAGIC = 0x11223344;

// File version number. Note that merely adding new record types in
// the ProfilerTask does not require bumping version number as long as original
// enum values are not renamed or deleted.
static final int VERSION = 0x03;
public static final int VERSION = 0x03;

// EOF marker. Must be < 0.
static final int EOF_MARKER = -1;
public static final int EOF_MARKER = -1;

// Profiler will check for gathered data and persist all of it in the
// separate thread every SAVE_DELAY ms.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.devtools.build.lib.profiler;
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;
Expand All @@ -26,6 +26,9 @@
import com.google.common.collect.MultimapBuilder.ListMultimapBuilder;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.VarInt;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -77,7 +80,7 @@ public static final class AggregateAttr {
/**
* Immutable compact representation of the Map<ProfilerTask, AggregateAttr>.
*/
static final class CompactStatistics {
public static final class CompactStatistics {
final byte[] content;

CompactStatistics(byte[] content) {
Expand All @@ -103,13 +106,13 @@ static final class CompactStatistics {
content = sink.position() > 0 ? Arrays.copyOf(sink.array(), sink.position()) : null;
}

boolean isEmpty() { return content == null; }
public boolean isEmpty() { return content == null; }

/**
* Converts instance back into AggregateAttr[TASK_COUNT]. See
* constructor documentation for more information.
*/
AggregateAttr[] toArray() {
public AggregateAttr[] toArray() {
AggregateAttr[] stats = new AggregateAttr[TASK_COUNT];
if (!isEmpty()) {
ByteBuffer source = ByteBuffer.wrap(content);
Expand All @@ -126,7 +129,7 @@ AggregateAttr[] toArray() {
/**
* Returns AggregateAttr instance for the given ProfilerTask value.
*/
AggregateAttr getAttr(ProfilerTask task) {
public AggregateAttr getAttr(ProfilerTask task) {
if (isEmpty()) { return ZERO; }
ByteBuffer source = ByteBuffer.wrap(content);
byte id = (byte) task.ordinal();
Expand Down Expand Up @@ -181,9 +184,9 @@ public final class Task implements Comparable<Task> {
public final long startTime;
public final long durationNanos;
public final ProfilerTask type;
final CompactStatistics stats;
public final CompactStatistics stats;
// Contains statistic for a task and all subtasks. Populated only for root tasks.
CompactStatistics aggregatedStats = null;
public CompactStatistics aggregatedStats = null;
// Subtasks are stored as an array for performance and memory utilization
// reasons (we can easily deal with millions of those objects).
public Task[] subtasks = NO_TASKS;
Expand Down Expand Up @@ -1104,5 +1107,4 @@ public static void aggregateProfile(ProfileInfo profileInfo, InfoListener report
reporter.info("Aggregating task statistics");
profileInfo.calculateStats();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@

package com.google.devtools.build.lib.profiler.chart;

import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.Task;

import java.util.EnumSet;
import java.util.Set;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

package com.google.devtools.build.lib.profiler.chart;

import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;

/**
* Provides some common functions for {@link ChartCreator}s.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@

package com.google.devtools.build.lib.profiler.chart;

import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfileInfo.CriticalPathEntry;
import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.CriticalPathEntry;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.Task;

import java.util.EnumSet;

Expand Down
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.ProfileInfo.CriticalPathEntry;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.CriticalPathEntry;
import com.google.devtools.build.lib.profiler.statistics.CriticalPathStatistics;
import com.google.devtools.build.lib.profiler.statistics.CriticalPathStatistics.MiddleManStatistics;
import com.google.devtools.build.lib.util.Pair;
Expand Down
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.ProfileInfo.CriticalPathEntry;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.CriticalPathEntry;
import com.google.devtools.build.lib.profiler.statistics.CriticalPathStatistics;
import com.google.devtools.build.lib.profiler.statistics.CriticalPathStatistics.MiddleManStatistics;
import com.google.devtools.build.lib.util.Pair;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
package com.google.devtools.build.lib.profiler.output;

import com.google.common.base.Optional;
import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.chart.AggregatingChartCreator;
import com.google.devtools.build.lib.profiler.chart.Chart;
import com.google.devtools.build.lib.profiler.chart.ChartCreator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@

import com.google.common.base.Predicate;
import com.google.devtools.build.lib.actions.MiddlemanAction;
import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfileInfo.CriticalPathEntry;
import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
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.util.Pair;

import java.util.ArrayList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
// limitations under the License.
package com.google.devtools.build.lib.profiler.statistics;

import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfileInfo.InfoListener;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.InfoListener;
import com.google.devtools.build.lib.vfs.Path;

import java.io.IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@

import com.google.common.base.Predicate;
import com.google.common.collect.Iterators;
import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfileInfo.AggregateAttr;
import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.AggregateAttr;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.Task;
import com.google.devtools.build.lib.util.Preconditions;

import java.util.EnumMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.profiler.statistics;

import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;

import java.util.EnumMap;
import java.util.Iterator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
import com.google.common.collect.Table;
import com.google.common.collect.Table.Cell;
import com.google.common.collect.Tables;
import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.Task;

import java.util.Arrays;
import java.util.EnumMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Maps.EntryTransformer;
import com.google.common.collect.Multimap;
import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.Task;
import com.google.devtools.build.lib.util.LongArrayList;

import java.util.Collection;
Expand Down
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.statistics;

import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.Task;
import com.google.devtools.build.lib.util.LongArrayList;
import java.util.List;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.profiler.ProfileInfo;
import com.google.devtools.build.lib.profiler.ProfileInfo.InfoListener;
import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.InfoListener;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo.Task;
import com.google.devtools.build.lib.profiler.output.HtmlCreator;
import com.google.devtools.build.lib.profiler.output.PhaseText;
import com.google.devtools.build.lib.profiler.statistics.CriticalPathStatistics;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.junit.Assert.assertSame;

import com.google.devtools.build.lib.profiler.Profiler.ProfiledTaskKinds;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.profiler.chart.AggregatingChartCreator;
import com.google.devtools.build.lib.profiler.chart.Chart;
import com.google.devtools.build.lib.profiler.chart.ChartBar;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static org.junit.Assert.fail;

import com.google.devtools.build.lib.profiler.Profiler.ProfiledTaskKinds;
import com.google.devtools.build.lib.profiler.analysis.ProfileInfo;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.testutil.Suite;
import com.google.devtools.build.lib.testutil.TestSpec;
Expand Down

0 comments on commit 44553fc

Please sign in to comment.