Merge "Revert "Reduce allocations on location delivery"" into sc-v2-dev am: 679377ad25

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/16496669

Change-Id: I65e04f55ae82cd14021068af6fb921df78142f80
This commit is contained in:
Soonil Nagarkar
2021-12-17 07:21:21 +00:00
committed by Automerger Merge Worker

View File

@@ -177,7 +177,7 @@ public class LocationProviderManager extends
protected interface LocationTransport { protected interface LocationTransport {
void deliverOnLocationChanged(LocationResult locationResult, void deliverOnLocationChanged(LocationResult locationResult,
@Nullable IRemoteCallback onCompleteCallback) throws Exception; @Nullable Runnable onCompleteCallback) throws Exception;
void deliverOnFlushComplete(int requestCode) throws Exception; void deliverOnFlushComplete(int requestCode) throws Exception;
} }
@@ -197,8 +197,9 @@ public class LocationProviderManager extends
@Override @Override
public void deliverOnLocationChanged(LocationResult locationResult, public void deliverOnLocationChanged(LocationResult locationResult,
@Nullable IRemoteCallback onCompleteCallback) throws RemoteException { @Nullable Runnable onCompleteCallback) throws RemoteException {
mListener.onLocationChanged(locationResult.asList(), onCompleteCallback); mListener.onLocationChanged(locationResult.asList(),
SingleUseCallback.wrap(onCompleteCallback));
} }
@Override @Override
@@ -226,7 +227,7 @@ public class LocationProviderManager extends
@Override @Override
public void deliverOnLocationChanged(LocationResult locationResult, public void deliverOnLocationChanged(LocationResult locationResult,
@Nullable IRemoteCallback onCompleteCallback) @Nullable Runnable onCompleteCallback)
throws PendingIntent.CanceledException { throws PendingIntent.CanceledException {
BroadcastOptions options = BroadcastOptions.makeBasic(); BroadcastOptions options = BroadcastOptions.makeBasic();
options.setDontSendToRestrictedApps(true); options.setDontSendToRestrictedApps(true);
@@ -242,34 +243,20 @@ public class LocationProviderManager extends
intent.putExtra(KEY_LOCATIONS, locationResult.asList().toArray(new Location[0])); intent.putExtra(KEY_LOCATIONS, locationResult.asList().toArray(new Location[0]));
} }
PendingIntent.OnFinished onFinished = null;
// send() SHOULD only run the completion callback if it completes successfully. however, // send() SHOULD only run the completion callback if it completes successfully. however,
// b/201299281 (which could not be fixed in the S timeframe) means that it's possible // b/199464864 (which could not be fixed in the S timeframe) means that it's possible
// for send() to throw an exception AND run the completion callback. if this happens, we // for send() to throw an exception AND run the completion callback. if this happens, we
// would over-release the wakelock... we take matters into our own hands to ensure that // would over-release the wakelock... we take matters into our own hands to ensure that
// the completion callback can only be run if send() completes successfully. this means // the completion callback can only be run if send() completes successfully. this means
// the completion callback may be run inline - but as we've never specified what thread // the completion callback may be run inline - but as we've never specified what thread
// the callback is run on, this is fine. // the callback is run on, this is fine.
GatedCallback gatedCallback; GatedCallback gatedCallback = new GatedCallback(onCompleteCallback);
if (onCompleteCallback != null) {
gatedCallback = new GatedCallback(() -> {
try {
onCompleteCallback.sendResult(null);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
});
onFinished = (pI, i, rC, rD, rE) -> gatedCallback.run();
} else {
gatedCallback = new GatedCallback(null);
}
mPendingIntent.send( mPendingIntent.send(
mContext, mContext,
0, 0,
intent, intent,
onFinished, (pI, i, rC, rD, rE) -> gatedCallback.run(),
null, null,
null, null,
options.toBundle()); options.toBundle());
@@ -306,7 +293,7 @@ public class LocationProviderManager extends
@Override @Override
public void deliverOnLocationChanged(@Nullable LocationResult locationResult, public void deliverOnLocationChanged(@Nullable LocationResult locationResult,
@Nullable IRemoteCallback onCompleteCallback) @Nullable Runnable onCompleteCallback)
throws RemoteException { throws RemoteException {
// ILocationCallback doesn't currently support completion callbacks // ILocationCallback doesn't currently support completion callbacks
Preconditions.checkState(onCompleteCallback == null); Preconditions.checkState(onCompleteCallback == null);
@@ -727,13 +714,6 @@ public class LocationProviderManager extends
final PowerManager.WakeLock mWakeLock; final PowerManager.WakeLock mWakeLock;
// b/206340085 - if we allocate a new wakelock releaser object for every delivery we
// increase the risk of resource starvation. if a client stops processing deliveries the
// system server binder allocation pool will be starved as we continue to queue up
// deliveries, each with a new allocation. in order to mitigate this, we use a single
// releaser object per registration rather than per delivery.
final ExternalWakeLockReleaser mWakeLockReleaser;
private volatile ProviderTransport mProviderTransport; private volatile ProviderTransport mProviderTransport;
private int mNumLocationsDelivered = 0; private int mNumLocationsDelivered = 0;
private long mExpirationRealtimeMs = Long.MAX_VALUE; private long mExpirationRealtimeMs = Long.MAX_VALUE;
@@ -747,7 +727,6 @@ public class LocationProviderManager extends
.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, WAKELOCK_TAG); .newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, WAKELOCK_TAG);
mWakeLock.setReferenceCounted(true); mWakeLock.setReferenceCounted(true);
mWakeLock.setWorkSource(request.getWorkSource()); mWakeLock.setWorkSource(request.getWorkSource());
mWakeLockReleaser = new ExternalWakeLockReleaser(identity, mWakeLock);
} }
@Override @Override
@@ -964,7 +943,7 @@ public class LocationProviderManager extends
} }
listener.deliverOnLocationChanged(deliverLocationResult, listener.deliverOnLocationChanged(deliverLocationResult,
mUseWakeLock ? mWakeLockReleaser : null); mUseWakeLock ? mWakeLock::release : null);
EVENT_LOG.logProviderDeliveredLocations(mName, locationResult.size(), EVENT_LOG.logProviderDeliveredLocations(mName, locationResult.size(),
getIdentity()); getIdentity());
} }
@@ -2782,7 +2761,7 @@ public class LocationProviderManager extends
@GuardedBy("this") @GuardedBy("this")
private boolean mRun; private boolean mRun;
GatedCallback(@Nullable Runnable callback) { GatedCallback(Runnable callback) {
mCallback = callback; mCallback = callback;
} }
@@ -2817,24 +2796,4 @@ public class LocationProviderManager extends
} }
} }
} }
private static class ExternalWakeLockReleaser extends IRemoteCallback.Stub {
private final CallerIdentity mIdentity;
private final PowerManager.WakeLock mWakeLock;
ExternalWakeLockReleaser(CallerIdentity identity, PowerManager.WakeLock wakeLock) {
mIdentity = identity;
mWakeLock = Objects.requireNonNull(wakeLock);
}
@Override
public void sendResult(Bundle data) {
try {
mWakeLock.release();
} catch (RuntimeException e) {
Log.e(TAG, "wakelock over-released by " + mIdentity, e);
}
}
}
} }