Skip to content

Commit

Permalink
Pass in callsite of SurfaceControl constructor explicitly (1/3)
Browse files Browse the repository at this point in the history
Creating a new Throwable (and filling in the stack trace) can take
up to 150us. Since we do this on the critical path when sending
over SurfaceControl via binder multiple times, this is too much.
Instead, add an option to pass in callsite manually.

Bug: 159056748
Change-Id: I46c339c15a07192d61c4c546e46f260684a47120
Merged-In: I46c339c15a07192d61c4c546e46f260684a47120
Exempt-From-Owner-Approval: Large scale refactor
  • Loading branch information
XSJoJo committed Jun 26, 2020
1 parent cae0a5b commit d42ab1b
Show file tree
Hide file tree
Showing 43 changed files with 131 additions and 44 deletions.
2 changes: 1 addition & 1 deletion api/test-current.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5149,7 +5149,7 @@ package android.view {
}

public final class SurfaceControl implements android.os.Parcelable {
ctor public SurfaceControl(@NonNull android.view.SurfaceControl);
ctor public SurfaceControl(@NonNull android.view.SurfaceControl, @NonNull String);
method public static long acquireFrameRateFlexibilityToken();
method public boolean isSameSurface(@NonNull android.view.SurfaceControl);
method public static void releaseFrameRateFlexibilityToken(long);
Expand Down
7 changes: 7 additions & 0 deletions core/java/android/os/StrictMode.java
Original file line number Diff line number Diff line change
Expand Up @@ -1918,9 +1918,16 @@ private static void handleApplicationStrictModeViolation(int penaltyMask,
}

private static class AndroidCloseGuardReporter implements CloseGuard.Reporter {

@Override
public void report(String message, Throwable allocationSite) {
onVmPolicyViolation(new LeakedClosableViolation(message, allocationSite));
}

@Override
public void report(String message) {
onVmPolicyViolation(new LeakedClosableViolation(message));
}
}

/** Called from Parcel.writeNoException() */
Expand Down
5 changes: 5 additions & 0 deletions core/java/android/os/strictmode/LeakedClosableViolation.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,9 @@ public LeakedClosableViolation(String message, Throwable allocationSite) {
super(message);
initCause(allocationSite);
}

/** @hide */
public LeakedClosableViolation(String message) {
super(message);
}
}
2 changes: 1 addition & 1 deletion core/java/android/view/InsetsSourceControl.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public InsetsSourceControl(@InternalInsetsType int type, @Nullable SurfaceContro
public InsetsSourceControl(InsetsSourceControl other) {
mType = other.mType;
if (other.mLeash != null) {
mLeash = new SurfaceControl(other.mLeash);
mLeash = new SurfaceControl(other.mLeash, "InsetsSourceControl");
} else {
mLeash = null;
}
Expand Down
42 changes: 29 additions & 13 deletions core/java/android/view/SurfaceControl.java
Original file line number Diff line number Diff line change
Expand Up @@ -499,14 +499,12 @@ public boolean removeOnReparentListener(@NonNull OnReparentListener listener) {
private static final int INTERNAL_DATASPACE_DISPLAY_P3 = 143261696;
private static final int INTERNAL_DATASPACE_SCRGB = 411107328;

private void assignNativeObject(long nativeObject) {
private void assignNativeObject(long nativeObject, String callsite) {
if (mNativeObject != 0) {
release();
}
if (nativeObject != 0) {
Trace.traceBegin(Trace.TRACE_TAG_WINDOW_MANAGER, "closeGuard");
mCloseGuard.open("release");
Trace.traceEnd(Trace.TRACE_TAG_WINDOW_MANAGER);
mCloseGuard.openWithCallSite("release", callsite);
}
mNativeObject = nativeObject;
mNativeHandle = mNativeObject != 0 ? nativeGetHandle(nativeObject) : 0;
Expand All @@ -515,12 +513,12 @@ private void assignNativeObject(long nativeObject) {
/**
* @hide
*/
public void copyFrom(@NonNull SurfaceControl other) {
public void copyFrom(@NonNull SurfaceControl other, String callsite) {
mName = other.mName;
mWidth = other.mWidth;
mHeight = other.mHeight;
mLocalOwnerView = other.mLocalOwnerView;
assignNativeObject(nativeCopyFromSurfaceControl(other.mNativeObject));
assignNativeObject(nativeCopyFromSurfaceControl(other.mNativeObject), callsite);
}

/**
Expand Down Expand Up @@ -621,6 +619,7 @@ public static class Builder {
private WeakReference<View> mLocalOwnerView;
private SurfaceControl mParent;
private SparseIntArray mMetadata;
private String mCallsite = "SurfaceControl.Builder";

/**
* Begin building a SurfaceControl with a given {@link SurfaceSession}.
Expand Down Expand Up @@ -654,7 +653,7 @@ public SurfaceControl build() {
}
return new SurfaceControl(
mSession, mName, mWidth, mHeight, mFormat, mFlags, mParent, mMetadata,
mLocalOwnerView);
mLocalOwnerView, mCallsite);
}

/**
Expand Down Expand Up @@ -912,6 +911,18 @@ public Builder setFlags(int flags) {
return this;
}

/**
* Sets the callsite this SurfaceControl is constructed from.
*
* @param callsite String uniquely identifying callsite that created this object. Used for
* leakage tracking.
* @hide
*/
public Builder setCallsite(String callsite) {
mCallsite = callsite;
return this;
}

private Builder setFlags(int flags, int mask) {
mFlags = (mFlags & ~mask) | flags;
return this;
Expand Down Expand Up @@ -943,10 +954,13 @@ private Builder setFlags(int flags, int mask) {
* @param h The surface initial height.
* @param flags The surface creation flags.
* @param metadata Initial metadata.
* @param callsite String uniquely identifying callsite that created this object. Used for
* leakage tracking.
* @throws throws OutOfResourcesException If the SurfaceControl cannot be created.
*/
private SurfaceControl(SurfaceSession session, String name, int w, int h, int format, int flags,
SurfaceControl parent, SparseIntArray metadata, WeakReference<View> localOwnerView)
SurfaceControl parent, SparseIntArray metadata, WeakReference<View> localOwnerView,
String callsite)
throws OutOfResourcesException, IllegalArgumentException {
if (name == null) {
throw new IllegalArgumentException("name must not be null");
Expand Down Expand Up @@ -978,18 +992,20 @@ private SurfaceControl(SurfaceSession session, String name, int w, int h, int fo
"Couldn't allocate SurfaceControl native object");
}
mNativeHandle = nativeGetHandle(mNativeObject);
mCloseGuard.open("release");
mCloseGuard.openWithCallSite("release", callsite);
}

/**
* Copy constructor. Creates a new native object pointing to the same surface as {@code other}.
*
* @param other The object to copy the surface from.
* @param callsite String uniquely identifying callsite that created this object. Used for
* leakage tracking.
* @hide
*/
@TestApi
public SurfaceControl(@NonNull SurfaceControl other) {
copyFrom(other);
public SurfaceControl(@NonNull SurfaceControl other, @NonNull String callsite) {
copyFrom(other, callsite);
}

private SurfaceControl(Parcel in) {
Expand All @@ -1015,7 +1031,7 @@ public void readFromParcel(Parcel in) {
if (in.readInt() != 0) {
object = nativeReadFromParcel(in);
}
assignNativeObject(object);
assignNativeObject(object, "readFromParcel");
}

@Override
Expand Down Expand Up @@ -2209,7 +2225,7 @@ public static boolean setDisplayBrightness(IBinder displayToken, float brightnes
public static SurfaceControl mirrorSurface(SurfaceControl mirrorOf) {
long nativeObj = nativeMirrorSurface(mirrorOf.mNativeObject);
SurfaceControl sc = new SurfaceControl();
sc.assignNativeObject(nativeObj);
sc.assignNativeObject(nativeObj, "mirrorSurface");
return sc;
}

Expand Down
7 changes: 4 additions & 3 deletions core/java/android/view/SurfaceControlViewHost.java
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,10 @@ public SurfaceControlViewHost(@NonNull Context c, @NonNull Display d,
public SurfaceControlViewHost(@NonNull Context context, @NonNull Display display,
@Nullable IBinder hostToken) {
mSurfaceControl = new SurfaceControl.Builder()
.setContainerLayer()
.setName("SurfaceControlViewHost")
.build();
.setContainerLayer()
.setName("SurfaceControlViewHost")
.setCallsite("SurfaceControlViewHost")
.build();
mWm = new WindowlessWindowManager(context.getResources().getConfiguration(),
mSurfaceControl, hostToken);
mViewRoot = new ViewRootImpl(context, display, mWm);
Expand Down
2 changes: 2 additions & 0 deletions core/java/android/view/SurfaceView.java
Original file line number Diff line number Diff line change
Expand Up @@ -993,13 +993,15 @@ protected void updateSurface() {
.setFormat(mFormat)
.setParent(viewRoot.getBoundsLayer())
.setFlags(mSurfaceFlags)
.setCallsite("SurfaceView.updateSurface")
.build();
mBackgroundControl = new SurfaceControl.Builder(mSurfaceSession)
.setName("Background for -" + name)
.setLocalOwnerView(this)
.setOpaque(true)
.setColorLayer()
.setParent(mSurfaceControl)
.setCallsite("SurfaceView.updateSurface")
.build();

} else if (mSurfaceControl == null) {
Expand Down
1 change: 1 addition & 0 deletions core/java/android/view/View.java
Original file line number Diff line number Diff line change
Expand Up @@ -26372,6 +26372,7 @@ public final boolean startDragAndDrop(ClipData data, DragShadowBuilder shadowBui
.setParent(root.getSurfaceControl())
.setBufferSize(shadowSize.x, shadowSize.y)
.setFormat(PixelFormat.TRANSLUCENT)
.setCallsite("View.startDragAndDrop")
.build();
final Surface surface = new Surface();
surface.copyFrom(surfaceControl);
Expand Down
1 change: 1 addition & 0 deletions core/java/android/view/ViewRootImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -1780,6 +1780,7 @@ public SurfaceControl getBoundsLayer() {
.setContainerLayer()
.setName("Bounds for - " + getTitle().toString())
.setParent(getRenderSurfaceControl())
.setCallsite("ViewRootImpl.getBoundsLayer")
.build();
setBoundsLayerCrop();
mTransaction.show(mBoundsLayer).apply();
Expand Down
5 changes: 3 additions & 2 deletions core/java/android/view/WindowlessWindowManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ public int addToDisplay(IWindow window, int seq, WindowManager.LayoutParams attr
.setParent(mRootSurface)
.setFormat(attrs.format)
.setBufferSize(getSurfaceWidth(attrs), getSurfaceHeight(attrs))
.setName(attrs.getTitle().toString());
.setName(attrs.getTitle().toString())
.setCallsite("WindowlessWindowManager.addToDisplay");
final SurfaceControl sc = b.build();

if (((attrs.inputFeatures &
Expand Down Expand Up @@ -248,7 +249,7 @@ public int relayout(IWindow window, int seq, WindowManager.LayoutParams inAttrs,
if (viewFlags == View.VISIBLE) {
t.setBufferSize(sc, getSurfaceWidth(attrs), getSurfaceHeight(attrs))
.setOpaque(sc, isOpaque(attrs)).show(sc).apply();
outSurfaceControl.copyFrom(sc);
outSurfaceControl.copyFrom(sc, "WindowlessWindowManager.relayout");
} else {
t.hide(sc).apply();
outSurfaceControl.release();
Expand Down
1 change: 1 addition & 0 deletions core/java/android/widget/Magnifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,7 @@ private static class InternalPopupWindow {
.setName("magnifier surface")
.setFlags(SurfaceControl.HIDDEN)
.setParent(parentSurfaceControl)
.setCallsite("InternalPopupWindow")
.build();
mSurface = new Surface();
mSurface.copyFrom(mSurfaceControl);
Expand Down
1 change: 1 addition & 0 deletions core/java/android/window/TaskEmbedder.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ public boolean initialize(SurfaceControl parent) {
.setContainerLayer()
.setParent(parent)
.setName(name)
.setCallsite("TaskEmbedder.initialize")
.build();

if (!onInitialize()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ private InsetsSourceControl createControl(@InternalInsetsType int type) {

// Simulate binder behavior by copying SurfaceControl. Otherwise, InsetsController will
// attempt to release mLeash directly.
SurfaceControl copy = new SurfaceControl(mLeash);
SurfaceControl copy = new SurfaceControl(mLeash, "InsetsControllerTest.createControl");
return new InsetsSourceControl(type, copy, new Point());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,14 @@ public void onTaskAppeared(RunningTaskInfo taskInfo, SurfaceControl leash) {
// Initialize dim surfaces:
mPrimaryDim = new SurfaceControl.Builder(mSurfaceSession)
.setParent(mPrimarySurface).setColorLayer()
.setName("Primary Divider Dim").build();
.setName("Primary Divider Dim")
.setCallsite("SplitScreenTaskOrganizer.onTaskAppeared")
.build();
mSecondaryDim = new SurfaceControl.Builder(mSurfaceSession)
.setParent(mSecondarySurface).setColorLayer()
.setName("Secondary Divider Dim").build();
.setName("Secondary Divider Dim")
.setCallsite("SplitScreenTaskOrganizer.onTaskAppeared")
.build();
SurfaceControl.Transaction t = getTransaction();
t.setLayer(mPrimaryDim, Integer.MAX_VALUE);
t.setColor(mPrimaryDim, new float[]{0f, 0f, 0f});
Expand Down
5 changes: 3 additions & 2 deletions services/core/java/com/android/server/display/ColorFade.java
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,9 @@ private boolean createSurface() {
if (mSurfaceControl == null) {
Transaction t = new Transaction();
try {
final SurfaceControl.Builder builder =
new SurfaceControl.Builder(mSurfaceSession).setName("ColorFade");
final SurfaceControl.Builder builder = new SurfaceControl.Builder(mSurfaceSession)
.setName("ColorFade")
.setCallsite("ColorFade.createSurface");
if (mMode == MODE_FADE) {
builder.setColorLayer();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,7 @@ public ViewportWindow(Context context) {
.setName(SURFACE_TITLE)
.setBufferSize(mTempPoint.x, mTempPoint.y) // not a typo
.setFormat(PixelFormat.TRANSLUCENT)
.setCallsite("ViewportWindow")
.build();
} catch (OutOfResourcesException oore) {
/* ignore */
Expand Down
3 changes: 2 additions & 1 deletion services/core/java/com/android/server/wm/ActivityRecord.java
Original file line number Diff line number Diff line change
Expand Up @@ -5910,7 +5910,8 @@ private SurfaceControl createAnimationBoundsLayer(Transaction t) {
ProtoLog.i(WM_DEBUG_APP_TRANSITIONS_ANIM, "Creating animation bounds layer");
final SurfaceControl.Builder builder = makeAnimationLeash()
.setParent(getAnimationLeashParent())
.setName(getSurfaceControl() + " - animation-bounds");
.setName(getSurfaceControl() + " - animation-bounds")
.setCallsite("ActivityRecord.createAnimationBoundsLayer");
final SurfaceControl boundsLayer = builder.build();
t.show(boundsLayer);
return boundsLayer;
Expand Down
1 change: 1 addition & 0 deletions services/core/java/com/android/server/wm/BlackFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ static class BlackSurface {
.setName("BlackSurface")
.setColorLayer()
.setParent(surfaceControl)
.setCallsite("BlackSurface")
.build();
transaction.setWindowCrop(surface, w, h);
transaction.setAlpha(surface, 1);
Expand Down
1 change: 1 addition & 0 deletions services/core/java/com/android/server/wm/Dimmer.java
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ private SurfaceControl makeDimLayer() {
.setParent(mHost.getSurfaceControl())
.setColorLayer()
.setName("Dim Layer for - " + mHost.getName())
.setCallsite("Dimmer.makeDimLayer")
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ public void unregisterOrganizer(IDisplayAreaOrganizer organizer) {

void onDisplayAreaAppeared(IDisplayAreaOrganizer organizer, DisplayArea da) {
try {
SurfaceControl outSurfaceControl = new SurfaceControl(da.getSurfaceControl());
SurfaceControl outSurfaceControl = new SurfaceControl(da.getSurfaceControl(),
"DisplayAreaOrganizerController.onDisplayAreaAppeared");
organizer.onDisplayAreaAppeared(da.getDisplayAreaInfo(), outSurfaceControl);
} catch (RemoteException e) {
// Oh well...
Expand Down
7 changes: 5 additions & 2 deletions services/core/java/com/android/server/wm/DisplayContent.java
Original file line number Diff line number Diff line change
Expand Up @@ -987,7 +987,10 @@ class DisplayContent extends WindowContainer<DisplayContent.DisplayChildWindowCo
final SurfaceControl.Builder b = mWmService.makeSurfaceBuilder(mSession)
.setOpaque(true)
.setContainerLayer();
mSurfaceControl = b.setName("Root").setContainerLayer().build();
mSurfaceControl = b.setName("Root")
.setContainerLayer()
.setCallsite("DisplayContent")
.build();

getPendingTransaction()
.setLayer(mSurfaceControl, 0)
Expand Down Expand Up @@ -1110,7 +1113,7 @@ SurfaceControl addShellRoot(@NonNull IWindow client, int windowType) {
return null;
}
mShellRoots.put(windowType, root);
SurfaceControl out = new SurfaceControl(rootLeash);
SurfaceControl out = new SurfaceControl(rootLeash, "DisplayContent.addShellRoot");
return out;
}

Expand Down
4 changes: 3 additions & 1 deletion services/core/java/com/android/server/wm/DragState.java
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ private void showInputSurface() {
mInputSurface = mService.makeSurfaceBuilder(
mService.mRoot.getDisplayContent(mDisplayContent.getDisplayId()).getSession())
.setContainerLayer()
.setName("Drag and Drop Input Consumer").build();
.setName("Drag and Drop Input Consumer")
.setCallsite("DragState.showInputSurface")
.build();
}
final InputWindowHandle h = getInputWindowHandle();
if (h == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class EmulatorDisplayOverlay {
.setName("EmulatorDisplayOverlay")
.setBufferSize(mScreenSize.x, mScreenSize.y)
.setFormat(PixelFormat.TRANSLUCENT)
.setCallsite("EmulatorDisplayOverlay")
.build();
t.setLayer(ctrl, zOrder);
t.setPosition(ctrl, 0, 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,10 @@ class InputConsumerImpl implements IBinder.DeathRecipient {
mWindowHandle.inputFeatures = 0;
mWindowHandle.scaleFactor = 1.0f;

mInputSurface = mService.makeSurfaceBuilder(mService.mRoot.getDisplayContent(displayId)
.getSession()).setContainerLayer().setName("Input Consumer " + name)
mInputSurface = mService.makeSurfaceBuilder(mService.mRoot.getDisplayContent(displayId).getSession())
.setContainerLayer()
.setName("Input Consumer " + name)
.setCallsite("InputConsumerImpl")
.build();
}

Expand Down
Loading

0 comments on commit d42ab1b

Please sign in to comment.