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

Commit 60538aeb authored by Kweku Adams's avatar Kweku Adams
Browse files

Stop caching entire ServiceInfo object. 

All we care about from the ServiceInfo object is the intended process
name, so cache only that value instead of the entire ServiceInfo object.
The cache is meant to help reduce calls to PackageManager for data that
won't change often.

Bug: 141645789
Test: atest CtsJobSchedulerTestCases
Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job
Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job
Change-Id: Ice4aa6dc1a917927ebaf80b0c4a2cba52fe8ed6d
parent e9d7c365

apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java

+3 −4
Original line number Diff line number Diff line
@@ -2554,9 +2554,9 @@ public class JobSchedulerService extends com.android.server.SystemService 
    }



    private boolean isComponentUsable(@NonNull JobStatus job) {

        final ServiceInfo service = job.serviceInfo;

        final String processName = job.serviceProcessName;



        if (service == null) {

        if (processName == null) {

            if (DEBUG) {

                Slog.v(TAG, "isComponentUsable: " + job.toShortString()

                        + " component not present");
@@ -2565,8 +2565,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
        }



        // Everything else checked out so far, so this is the final yes/no check

        final boolean appIsBad = mActivityManagerInternal.isAppBad(

                service.processName, service.applicationInfo.uid);

        final boolean appIsBad = mActivityManagerInternal.isAppBad(processName, job.getUid());

        if (DEBUG && appIsBad) {

            Slog.i(TAG, "App is bad for " + job.toShortString() + " so not runnable");

        }

apex/jobscheduler/service/java/com/android/server/job/controllers/ComponentController.java

+28 −22
Original line number Diff line number Diff line
@@ -93,7 +93,12 @@ public class ComponentController extends StateController { 
        }

    };



    private final SparseArrayMap<ComponentName, ServiceInfo> mServiceInfoCache =

    /**

     * Cache containing the processName of the ServiceInfo (see {@link ServiceInfo#processName})

     * if the Service exists and is available.

     * {@code null} will be stored if the service is currently unavailable.

     */

    private final SparseArrayMap<ComponentName, String> mServiceProcessCache =

            new SparseArrayMap<>();



    private final ComponentStateUpdateFunctor mComponentStateUpdateFunctor =
@@ -135,18 +140,18 @@ public class ComponentController extends StateController { 
    @Override

    @GuardedBy("mLock")

    public void onUserRemovedLocked(int userId) {

        mServiceInfoCache.delete(userId);

        mServiceProcessCache.delete(userId);

    }



    @Nullable

    @GuardedBy("mLock")

    private ServiceInfo getServiceInfoLocked(JobStatus jobStatus) {

    private String getServiceProcessLocked(JobStatus jobStatus) {

        final ComponentName service = jobStatus.getServiceComponent();

        final int userId = jobStatus.getUserId();

        if (mServiceInfoCache.contains(userId, service)) {

        if (mServiceProcessCache.contains(userId, service)) {

            // Return whatever is in the cache, even if it's null. When something changes, we

            // clear the cache.

            return mServiceInfoCache.get(userId, service);

            return mServiceProcessCache.get(userId, service);

        }



        ServiceInfo si;
@@ -165,30 +170,31 @@ public class ComponentController extends StateController { 
            // Write null to the cache so we don't keep querying PM.

            si = null;

        }

        mServiceInfoCache.add(userId, service, si);

        final String processName = si == null ? null : si.processName;

        mServiceProcessCache.add(userId, service, processName);



        return si;

        return processName;

    }



    @GuardedBy("mLock")

    private boolean updateComponentEnabledStateLocked(JobStatus jobStatus) {

        final ServiceInfo service = getServiceInfoLocked(jobStatus);

        final String processName = getServiceProcessLocked(jobStatus);



        if (DEBUG && service == null) {

        if (DEBUG && processName == null) {

            Slog.v(TAG, jobStatus.toShortString() + " component not present");

        }

        final ServiceInfo ogService = jobStatus.serviceInfo;

        jobStatus.serviceInfo = service;

        return !Objects.equals(ogService, service);

        final String ogProcess = jobStatus.serviceProcessName;

        jobStatus.serviceProcessName = processName;

        return !Objects.equals(ogProcess, processName);

    }



    @GuardedBy("mLock")

    private void clearComponentsForPackageLocked(final int userId, final String pkg) {

        final int uIdx = mServiceInfoCache.indexOfKey(userId);

        for (int c = mServiceInfoCache.numElementsForKey(userId) - 1; c >= 0; --c) {

            final ComponentName cn = mServiceInfoCache.keyAt(uIdx, c);

        final int uIdx = mServiceProcessCache.indexOfKey(userId);

        for (int c = mServiceProcessCache.numElementsForKey(userId) - 1; c >= 0; --c) {

            final ComponentName cn = mServiceProcessCache.keyAt(uIdx, c);

            if (cn.getPackageName().equals(pkg)) {

                mServiceInfoCache.delete(userId, cn);

                mServiceProcessCache.delete(userId, cn);

            }

        }

    }
@@ -207,7 +213,7 @@ public class ComponentController extends StateController { 


    private void updateComponentStateForUser(final int userId) {

        synchronized (mLock) {

            mServiceInfoCache.delete(userId);

            mServiceProcessCache.delete(userId);

            updateComponentStatesLocked(jobStatus -> {

                // Using user ID instead of source user ID because the service will run under the

                // user ID, not source user ID.
@@ -247,15 +253,15 @@ public class ComponentController extends StateController { 
    @Override

    @GuardedBy("mLock")

    public void dumpControllerStateLocked(IndentingPrintWriter pw, Predicate<JobStatus> predicate) {

        for (int u = 0; u < mServiceInfoCache.numMaps(); ++u) {

            final int userId = mServiceInfoCache.keyAt(u);

            for (int p = 0; p < mServiceInfoCache.numElementsForKey(userId); ++p) {

                final ComponentName componentName = mServiceInfoCache.keyAt(u, p);

        for (int u = 0; u < mServiceProcessCache.numMaps(); ++u) {

            final int userId = mServiceProcessCache.keyAt(u);

            for (int p = 0; p < mServiceProcessCache.numElementsForKey(userId); ++p) {

                final ComponentName componentName = mServiceProcessCache.keyAt(u, p);

                pw.print(userId);

                pw.print("-");

                pw.print(componentName);

                pw.print(": ");

                pw.print(mServiceInfoCache.valueAt(u, p));

                pw.print(mServiceProcessCache.valueAt(u, p));

                pw.println();

            }

        }

apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java

+3 −4
Original line number Diff line number Diff line
@@ -35,7 +35,6 @@ import android.app.job.JobParameters; 
import android.app.job.JobWorkItem;

import android.content.ClipData;

import android.content.ComponentName;

import android.content.pm.ServiceInfo;

import android.net.Network;

import android.net.NetworkRequest;

import android.net.Uri;
@@ -355,7 +354,7 @@ public final class JobStatus { 
    public ArraySet<Uri> changedUris;

    public ArraySet<String> changedAuthorities;

    public Network network;

    public ServiceInfo serviceInfo;

    public String serviceProcessName;



    /** The evaluated bias of the job when it started running. */

    public int lastEvaluatedBias;
@@ -1747,7 +1746,7 @@ public final class JobStatus { 
        // run if its constraints are satisfied).

        // DeviceNotDozing implicit constraint must be satisfied

        // NotRestrictedInBackground implicit constraint must be satisfied

        return mReadyNotDozing && mReadyNotRestrictedInBg && (serviceInfo != null)

        return mReadyNotDozing && mReadyNotRestrictedInBg && (serviceProcessName != null)

                && (mReadyDeadlineSatisfied || isConstraintsSatisfied(satisfiedConstraints));

    }
@@ -2286,7 +2285,7 @@ public final class JobStatus { 
            pw.println(mReadyDynamicSatisfied);

        }

        pw.print("readyComponentEnabled: ");

        pw.println(serviceInfo != null);

        pw.println(serviceProcessName != null);

        if ((getFlags() & JobInfo.FLAG_EXPEDITED) != 0) {

            pw.print("expeditedQuotaApproved: ");

            pw.print(mExpeditedQuotaApproved);

services/tests/mockingservicestests/src/com/android/server/job/controllers/BatteryControllerTest.java

+1 −2
Original line number Diff line number Diff line
@@ -34,7 +34,6 @@ import android.content.ComponentName; 
import android.content.Context;

import android.content.Intent;

import android.content.pm.PackageManagerInternal;

import android.content.pm.ServiceInfo;

import android.os.BatteryManagerInternal;

import android.os.RemoteException;

import android.util.ArraySet;
@@ -188,7 +187,7 @@ public class BatteryControllerTest { 
            JobInfo jobInfo) {

        JobStatus js = JobStatus.createFromJobInfo(

                jobInfo, callingUid, packageName, SOURCE_USER_ID, testTag);

        js.serviceInfo = mock(ServiceInfo.class);

        js.serviceProcessName = "testProcess";

        // Make sure tests aren't passing just because the default bucket is likely ACTIVE.

        js.setStandbyBucket(FREQUENT_INDEX);

        return js;

services/tests/mockingservicestests/src/com/android/server/job/controllers/JobStatusTest.java

+1 −2
Original line number Diff line number Diff line
@@ -50,7 +50,6 @@ import android.app.job.JobInfo; 
import android.app.usage.UsageStatsManagerInternal;

import android.content.ComponentName;

import android.content.pm.PackageManagerInternal;

import android.content.pm.ServiceInfo;

import android.net.Uri;

import android.os.SystemClock;

import android.provider.MediaStore;
@@ -810,7 +809,7 @@ public class JobStatusTest { 


    private static JobStatus createJobStatus(JobInfo job) {

        JobStatus jobStatus = JobStatus.createFromJobInfo(job, 0, null, -1, "JobStatusTest");

        jobStatus.serviceInfo = mock(ServiceInfo.class);

        jobStatus.serviceProcessName = "testProcess";

        return jobStatus;

    }

}