Skip to content

Commit

Permalink
Remove systrace markers from collectResults
Browse files Browse the repository at this point in the history 
Summary: We don't use them and they inflate our traces since systrace isn't free.

Reviewed By: mihaelao

Differential Revision: D15907802

fbshipit-source-id: 6bbe136719449e881e0b6894c6043cfc362b00dd
  • Loading branch information
@astreet @facebook-github-bot
astreet authored and facebook-github-bot committed Jun 21, 2019
1 parent a1a933f commit 3107467
Showing 1 changed file with 19 additions and 102 deletions.
121 changes: 19 additions & 102 deletions litho-core/src/main/java/com/facebook/litho/LayoutState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,6 @@ private static LayoutOutput createGenericLayoutOutput(

private static LayoutOutput createHostLayoutOutput(
LayoutState layoutState, InternalNode node, boolean isPhantom) {
final boolean isTracing = ComponentsSystrace.isTracing();
if (isTracing) {
ComponentsSystrace.beginSection(
"createHostLayoutOutput:" + node.getSimpleName());
}

final HostComponent hostComponent = HostComponent.create();

Expand Down Expand Up @@ -299,39 +294,25 @@ private static LayoutOutput createHostLayoutOutput(
}
}

if (isTracing) {
ComponentsSystrace.endSection();
}

return hostOutput;
}

private static LayoutOutput createDrawableLayoutOutput(
Component component, LayoutState layoutState, InternalNode node, boolean hasHostView) {
final boolean isTracing = ComponentsSystrace.isTracing();
try {
if (isTracing) {
ComponentsSystrace.beginSection("createDrawableLayoutOutput:" + node.getSimpleName());
}
// The mount operation will need both the marker for the target host and its matching
// parent host to ensure the correct hierarchy when nesting the host views.
long hostMarker = layoutState.mCurrentHostMarker;
// The mount operation will need both the marker for the target host and its matching
// parent host to ensure the correct hierarchy when nesting the host views.
long hostMarker = layoutState.mCurrentHostMarker;

return createLayoutOutput(
component,
hostMarker,
layoutState,
node,
false /* useNodePadding */,
IMPORTANT_FOR_ACCESSIBILITY_NO,
layoutState.mShouldDuplicateParentState,
hasHostView,
false);
} finally {
if (isTracing) {
ComponentsSystrace.endSection();
}
}
return createLayoutOutput(
component,
hostMarker,
layoutState,
node,
false /* useNodePadding */,
IMPORTANT_FOR_ACCESSIBILITY_NO,
layoutState.mShouldDuplicateParentState,
hasHostView,
false);
}

private static LayoutOutput createLayoutOutput(
Expand All @@ -344,10 +325,6 @@ private static LayoutOutput createLayoutOutput(
boolean duplicateParentState,
boolean hasHostView,
boolean isPhantom) {
final boolean isTracing = ComponentsSystrace.isTracing();
if (isTracing) {
ComponentsSystrace.beginSection("createLayoutOutput:" + node.getSimpleName());
}
final boolean isMountViewSpec = isMountViewSpec(component);

final int hostTranslationX;
Expand Down Expand Up @@ -434,10 +411,6 @@ private static LayoutOutput createLayoutOutput(
flags |= LAYOUT_FLAG_PHANTOM;
}

if (isTracing) {
ComponentsSystrace.endSection();
}

return new LayoutOutput(
layoutOutputNodeInfo,
layoutOutputViewNodeInfo,
Expand Down Expand Up @@ -633,9 +606,6 @@ private static void collectResults(
}
final Component component = node.getTailComponent();
final boolean isTracing = ComponentsSystrace.isTracing();
if (isTracing) {
ComponentsSystrace.beginSection("collectResults:" + node.getSimpleName());
}

// Early return if collecting results of a node holding a nested tree.
if (node.isNestedTreeHolder()) {
Expand Down Expand Up @@ -699,13 +669,7 @@ private static void collectResults(

final DiffNode diffNode;
if (shouldGenerateDiffTree) {
if (isTracing) {
ComponentsSystrace.beginSection("createDiffNode:" + node.getSimpleName());
}
diffNode = createDiffNode(node, parentDiffNode);
if (isTracing) {
ComponentsSystrace.endSection();
}
if (parentDiffNode == null) {
layoutState.mDiffTreeRoot = diffNode;
}
Expand Down Expand Up @@ -780,9 +744,6 @@ private static void collectResults(
? currentDiffNode.getBackground()
: null;

if (isTracing) {
ComponentsSystrace.beginSection("addBgDrawableComponent:" + node.getSimpleName());
}
final LayoutOutput backgroundOutput =
addDrawableComponent(
node,
Expand All @@ -795,9 +756,6 @@ private static void collectResults(
if (diffNode != null) {
diffNode.setBackground(backgroundOutput);
}
if (isTracing) {
ComponentsSystrace.endSection();
}
}
}

Expand Down Expand Up @@ -834,9 +792,6 @@ private static void collectResults(
}

if (TransitionUtils.areTransitionsEnabled(scopedAndroidContext)) {
if (isTracing) {
ComponentsSystrace.beginSection("extractTransitions:" + node.getSimpleName());
}
final ArrayList<Transition> transitions = node.getTransitions();
if (transitions != null) {
for (int i = 0, size = transitions.size(); i < size; i++) {
Expand All @@ -859,9 +814,6 @@ private static void collectResults(
layoutState.mComponentsNeedingPreviousRenderData.addAll(
componentsNeedingPreviousRenderData);
}
if (isTracing) {
ComponentsSystrace.endSection();
}
}

layoutState.mCurrentX += node.getX();
Expand All @@ -884,9 +836,6 @@ private static void collectResults(
if (node.shouldDrawBorders()) {
final LayoutOutput convertBorder =
(currentDiffNode != null) ? currentDiffNode.getBorder() : null;
if (isTracing) {
ComponentsSystrace.beginSection("addBorderDrawableComponent:" + node.getSimpleName());
}
final LayoutOutput borderOutput =
addDrawableComponent(
node,
Expand All @@ -898,9 +847,6 @@ private static void collectResults(
if (diffNode != null) {
diffNode.setBorder(borderOutput);
}
if (isTracing) {
ComponentsSystrace.endSection();
}
}

// 6. Add foreground if defined.
Expand All @@ -913,9 +859,6 @@ private static void collectResults(
? currentDiffNode.getForeground()
: null;

if (isTracing) {
ComponentsSystrace.beginSection("addFgDrawableComponent:" + node.getSimpleName());
}
final LayoutOutput foregroundOutput =
addDrawableComponent(
node,
Expand All @@ -928,17 +871,11 @@ private static void collectResults(
if (diffNode != null) {
diffNode.setForeground(foregroundOutput);
}
if (isTracing) {
ComponentsSystrace.endSection();
}
}
}

// 7. Add VisibilityOutputs if any visibility-related event handlers are present.
if (node.hasVisibilityHandlers()) {
if (isTracing) {
ComponentsSystrace.beginSection("addVisibilityHandlers:" + node.getSimpleName());
}
final VisibilityOutput visibilityOutput = createVisibilityOutput(node, layoutState);
final long previousId =
shouldUseCachedOutputs && currentDiffNode.getVisibilityOutput() != null
Expand All @@ -952,9 +889,6 @@ private static void collectResults(
if (diffNode != null) {
diffNode.setVisibilityOutput(visibilityOutput);
}
if (isTracing) {
ComponentsSystrace.endSection();
}
}

// 8. If we're in a testing environment, maintain an additional data structure with
Expand All @@ -967,9 +901,6 @@ private static void collectResults(
// 9. Extract the Working Range registrations.
List<WorkingRangeContainer.Registration> registrations = node.getWorkingRangeRegistrations();
if (registrations != null && !registrations.isEmpty()) {
if (isTracing) {
ComponentsSystrace.beginSection("extractWorkingRanges:" + node.getSimpleName());
}
if (layoutState.mWorkingRangeContainer == null) {
layoutState.mWorkingRangeContainer = new WorkingRangeContainer();
}
Expand All @@ -978,9 +909,6 @@ private static void collectResults(
layoutState.mWorkingRangeContainer.registerWorkingRange(
registration.mName, registration.mWorkingRange, registration.mComponent);
}
if (isTracing) {
ComponentsSystrace.endSection();
}
}

if (component != null) {
Expand All @@ -994,9 +922,6 @@ private static void collectResults(
rect.bottom = rect.top + node.getHeight();
}

if (isTracing) {
ComponentsSystrace.beginSection("keepComponentDelegates:" + node.getSimpleName());
}
for (Component delegate : node.getComponents()) {
final Rect copyRect = new Rect();
copyRect.set(rect);
Expand All @@ -1021,9 +946,6 @@ private static void collectResults(
layoutState.mComponentKeyToBounds.put(delegate.getGlobalKey(), copyRect);
}
}
if (isTracing) {
ComponentsSystrace.endSection();
}
}

// 10. If enabled, show a debug foreground layer covering the whole LithoView showing which
Expand Down Expand Up @@ -1072,10 +994,6 @@ private static void collectResults(
addCurrentAffinityGroupToTransitionMapping(layoutState);
layoutState.mCurrentTransitionId = currentTransitionId;
layoutState.mCurrentLayoutOutputAffinityGroup = currentLayoutOutputAffinityGroup;

if (isTracing) {
ComponentsSystrace.endSection();
}
}

Map<String, Rect> getComponentKeyToBounds() {
Expand Down Expand Up @@ -1276,10 +1194,6 @@ private static LayoutOutput addDrawableLayoutOutput(
private static int addHostLayoutOutput(
InternalNode node, LayoutState layoutState, DiffNode diffNode, boolean isPhantom) {
final Component component = node.getTailComponent();
final boolean isTracing = ComponentsSystrace.isTracing();
if (isTracing) {
ComponentsSystrace.beginSection("addHostLayoutOutput:" + node.getSimpleName());
}

// Only the root host is allowed to wrap view mount specs as a layout output
// is unconditionally added for it.
Expand Down Expand Up @@ -1309,9 +1223,6 @@ private static int addHostLayoutOutput(
maybeAddLayoutOutputToAffinityGroup(
layoutState.mCurrentLayoutOutputAffinityGroup, OutputUnitType.HOST, hostLayoutOutput);

if (isTracing) {
ComponentsSystrace.endSection();
}
return hostOutputPosition;
}

Expand Down Expand Up @@ -1561,7 +1472,13 @@ private static void setSizeAfterMeasureAndCollectResults(
return;
}

if (isTracing) {
ComponentsSystrace.beginSection("collectResults");
}
collectResults(c, root, layoutState, null);
if (isTracing) {
ComponentsSystrace.endSection();
}

if (isTracing) {
ComponentsSystrace.beginSection("sortMountableOutputs");
Expand Down

0 comments on commit 3107467

Please sign in to comment.