Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 232a0456 authored by Soonil Nagarkar's avatar Soonil Nagarkar
Browse files

Reduce allocations on location delivery 

Allocating a new Binder object for every location delivery is causing
resource starvation when location deliveries are not processed fast
enough. Instead, allocate an object per registration, to substantially
reduce the number of total allocations.

Bug: 206340085
Test: manual
Change-Id: Ib0ce770a059acab37b431cb557f96c7fc008aec7
parent a66bc891

services/core/java/com/android/server/location/provider/LocationProviderManager.java

+52 −11
Original line number Diff line number Diff line
@@ -177,7 +177,7 @@ public class LocationProviderManager extends 
    protected interface LocationTransport {



        void deliverOnLocationChanged(LocationResult locationResult,

                @Nullable Runnable onCompleteCallback) throws Exception;

                @Nullable IRemoteCallback onCompleteCallback) throws Exception;

        void deliverOnFlushComplete(int requestCode) throws Exception;

    }
@@ -197,9 +197,8 @@ public class LocationProviderManager extends 


        @Override

        public void deliverOnLocationChanged(LocationResult locationResult,

                @Nullable Runnable onCompleteCallback) throws RemoteException {

            mListener.onLocationChanged(locationResult.asList(),

                    SingleUseCallback.wrap(onCompleteCallback));

                @Nullable IRemoteCallback onCompleteCallback) throws RemoteException {

            mListener.onLocationChanged(locationResult.asList(), onCompleteCallback);

        }



        @Override
@@ -227,7 +226,7 @@ public class LocationProviderManager extends 


        @Override

        public void deliverOnLocationChanged(LocationResult locationResult,

                @Nullable Runnable onCompleteCallback)

                @Nullable IRemoteCallback onCompleteCallback)

                throws PendingIntent.CanceledException {

            BroadcastOptions options = BroadcastOptions.makeBasic();

            options.setDontSendToRestrictedApps(true);
@@ -243,20 +242,34 @@ public class LocationProviderManager extends 
                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,

            // b/199464864 (which could not be fixed in the S timeframe) means that it's possible

            // b/201299281 (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

            // 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 may be run inline - but as we've never specified what thread

            // the callback is run on, this is fine.

            GatedCallback gatedCallback = new GatedCallback(onCompleteCallback);

            GatedCallback gatedCallback;

            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(

                    mContext,

                    0,

                    intent,

                    (pI, i, rC, rD, rE) -> gatedCallback.run(),

                    onFinished,

                    null,

                    null,

                    options.toBundle());
@@ -293,7 +306,7 @@ public class LocationProviderManager extends 


        @Override

        public void deliverOnLocationChanged(@Nullable LocationResult locationResult,

                @Nullable Runnable onCompleteCallback)

                @Nullable IRemoteCallback onCompleteCallback)

                throws RemoteException {

            // ILocationCallback doesn't currently support completion callbacks

            Preconditions.checkState(onCompleteCallback == null);
@@ -714,6 +727,13 @@ public class LocationProviderManager extends 


        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 int mNumLocationsDelivered = 0;

        private long mExpirationRealtimeMs = Long.MAX_VALUE;
@@ -727,6 +747,7 @@ public class LocationProviderManager extends 
                    .newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, WAKELOCK_TAG);

            mWakeLock.setReferenceCounted(true);

            mWakeLock.setWorkSource(request.getWorkSource());

            mWakeLockReleaser = new ExternalWakeLockReleaser(identity, mWakeLock);

        }



        @Override
@@ -943,7 +964,7 @@ public class LocationProviderManager extends 
                    }



                    listener.deliverOnLocationChanged(deliverLocationResult,

                            mUseWakeLock ? mWakeLock::release : null);

                            mUseWakeLock ? mWakeLockReleaser : null);

                    EVENT_LOG.logProviderDeliveredLocations(mName, locationResult.size(),

                            getIdentity());

                }
@@ -2761,7 +2782,7 @@ public class LocationProviderManager extends 
        @GuardedBy("this")

        private boolean mRun;



        GatedCallback(Runnable callback) {

        GatedCallback(@Nullable Runnable callback) {

            mCallback = callback;

        }
@@ -2796,4 +2817,24 @@ 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);

            }

        }

    }

}