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

Commit c55c7389 authored by Kweku Adams's avatar Kweku Adams
Browse files

Hold wakelock earlier.

Start holding a wakelock from the instant we decide to start running a
job instead of waiting until the JobService is bound. This ensures that:
1. The CPU doesn't go to suspend between when we tell controllers that
   the job is starting, when we start the service, and when we
   successfully bind to the service
2. Execution timing is accurate and we don't accidentally count CPU
   sleep time as execution time.

Bug: 189985233
Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job
Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job
Test: atest CtsJobSchedulerTestCases
Change-Id: I95afa1e8d78448b8c8ee31ce5d9b68821318a23a
parent c00c253b
Loading
Loading
Loading
Loading
+34 −20
Original line number Diff line number Diff line
@@ -804,11 +804,21 @@ class JobConcurrencyManager {
            @WorkType final int workType) {
        final List<StateController> controllers = mService.mControllers;
        final int numControllers = controllers.size();
        final PowerManager.WakeLock wl =
                mPowerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, jobStatus.getTag());
        wl.setWorkSource(mService.deriveWorkSource(
                jobStatus.getSourceUid(), jobStatus.getSourcePackageName()));
        wl.setReferenceCounted(false);
        // Since the quota controller will start counting from the time prepareForExecutionLocked()
        // is called, hold a wakelock to make sure the CPU doesn't suspend between that call and
        // when the service actually starts.
        wl.acquire();
        try {
            for (int ic = 0; ic < numControllers; ic++) {
                controllers.get(ic).prepareForExecutionLocked(jobStatus);
            }
        final PackageStats packageStats =
                getPkgStatsLocked(jobStatus.getSourceUserId(), jobStatus.getSourcePackageName());
            final PackageStats packageStats = getPkgStatsLocked(
                    jobStatus.getSourceUserId(), jobStatus.getSourcePackageName());
            packageStats.adjustStagedCount(false, jobStatus.shouldTreatAsExpeditedJob());
            if (!worker.executeRunnableJob(jobStatus, workType)) {
                Slog.e(TAG, "Error executing " + jobStatus);
@@ -821,12 +831,16 @@ class JobConcurrencyManager {
                mWorkCountTracker.onJobStarted(workType);
                packageStats.adjustRunningCount(true, jobStatus.shouldTreatAsExpeditedJob());
                mActivePkgStats.add(
                    jobStatus.getSourceUserId(), jobStatus.getSourcePackageName(), packageStats);
                        jobStatus.getSourceUserId(), jobStatus.getSourcePackageName(),
                        packageStats);
            }
            final List<JobStatus> pendingJobs = mService.mPendingJobs;
            if (pendingJobs.remove(jobStatus)) {
                mService.mJobPackageTracker.noteNonpending(jobStatus);
            }
        } finally {
            wl.release();
        }
    }

    @GuardedBy("mLock")
+12 −2
Original line number Diff line number Diff line
@@ -917,8 +917,18 @@ public class JobSchedulerService extends com.android.server.SystemService
        return mConstants;
    }

    public boolean isChainedAttributionEnabled() {
        return WorkSource.isChainedBatteryAttributionEnabled(getContext());
    @NonNull
    public WorkSource deriveWorkSource(int sourceUid, @Nullable String sourcePackageName) {
        if (WorkSource.isChainedBatteryAttributionEnabled(getContext())) {
            WorkSource ws = new WorkSource();
            ws.createWorkChain()
                    .addNode(sourceUid, sourcePackageName)
                    .addNode(Process.SYSTEM_UID, "JobScheduler");
            return ws;
        } else {
            return sourcePackageName == null
                    ? new WorkSource(sourceUid) : new WorkSource(sourceUid, sourcePackageName);
        }
    }

    @Nullable
+9 −33
Original line number Diff line number Diff line
@@ -42,7 +42,6 @@ import android.os.Message;
import android.os.PowerManager;
import android.os.RemoteException;
import android.os.UserHandle;
import android.os.WorkSource;
import android.util.EventLog;
import android.util.IndentingPrintWriter;
import android.util.Slog;
@@ -109,6 +108,7 @@ public final class JobServiceContext implements ServiceConnection {
    private final Object mLock;
    private final IBatteryStats mBatteryStats;
    private final JobPackageTracker mJobPackageTracker;
    private final PowerManager mPowerManager;
    private PowerManager.WakeLock mWakeLock;

    // Execution state.
@@ -205,6 +205,7 @@ public final class JobServiceContext implements ServiceConnection {
        mCallbackHandler = new JobServiceHandler(looper);
        mJobConcurrencyManager = concurrencyManager;
        mCompletedListener = service;
        mPowerManager = mContext.getSystemService(PowerManager.class);
        mAvailable = true;
        mVerb = VERB_FINISHED;
        mPreferredUid = NO_PREFERRED_UID;
@@ -271,6 +272,12 @@ public final class JobServiceContext implements ServiceConnection {
            // it was inflated from disk with not-yet-coherent delay/deadline bounds.
            job.clearPersistedUtcTimes();

            mWakeLock = mPowerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, job.getTag());
            mWakeLock.setWorkSource(
                    mService.deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
            mWakeLock.setReferenceCounted(false);
            mWakeLock.acquire();

            mVerb = VERB_BINDING;
            scheduleOpTimeOutLocked();
            final Intent intent = new Intent().setComponent(job.getServiceComponent());
@@ -306,6 +313,7 @@ public final class JobServiceContext implements ServiceConnection {
                mRunningCallback = null;
                mParams = null;
                mExecutionStartTimeElapsed = 0L;
                mWakeLock.release();
                mVerb = VERB_FINISHED;
                removeOpTimeOutLocked();
                return false;
@@ -495,42 +503,10 @@ public final class JobServiceContext implements ServiceConnection {
                return;
            }
            this.service = IJobService.Stub.asInterface(service);
            final PowerManager pm =
                    (PowerManager) mContext.getSystemService(Context.POWER_SERVICE);
            PowerManager.WakeLock wl = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK,
                    runningJob.getTag());
            wl.setWorkSource(deriveWorkSource(runningJob));
            wl.setReferenceCounted(false);
            wl.acquire();

            // We use a new wakelock instance per job.  In rare cases there is a race between
            // teardown following job completion/cancellation and new job service spin-up
            // such that if we simply assign mWakeLock to be the new instance, we orphan
            // the currently-live lock instead of cleanly replacing it.  Watch for this and
            // explicitly fast-forward the release if we're in that situation.
            if (mWakeLock != null) {
                Slog.w(TAG, "Bound new job " + runningJob + " but live wakelock " + mWakeLock
                        + " tag=" + mWakeLock.getTag());
                mWakeLock.release();
            }
            mWakeLock = wl;
            doServiceBoundLocked();
        }
    }

    private WorkSource deriveWorkSource(JobStatus runningJob) {
        final int jobUid = runningJob.getSourceUid();
        if (WorkSource.isChainedBatteryAttributionEnabled(mContext)) {
            WorkSource workSource = new WorkSource();
            workSource.createWorkChain()
                    .addNode(jobUid, null)
                    .addNode(android.os.Process.SYSTEM_UID, "JobScheduler");
            return workSource;
        } else {
            return new WorkSource(jobUid);
        }
    }

    /** If the client service crashes we reschedule this job and clean up. */
    @Override
    public void onServiceDisconnected(ComponentName name) {
+6 −22
Original line number Diff line number Diff line
@@ -18,11 +18,9 @@ package com.android.server.job.controllers;

import static com.android.server.job.JobSchedulerService.sElapsedRealtimeClock;

import android.annotation.Nullable;
import android.app.AlarmManager;
import android.app.AlarmManager.OnAlarmListener;
import android.content.Context;
import android.os.Process;
import android.os.UserHandle;
import android.os.WorkSource;
import android.util.IndentingPrintWriter;
@@ -62,8 +60,6 @@ public final class TimeController extends StateController {
    private long mNextDelayExpiredElapsedMillis;
    private volatile long mLastFiredDelayExpiredElapsedMillis;

    private final boolean mChainedAttributionEnabled;

    private AlarmManager mAlarmService = null;
    /** List of tracked jobs, sorted asc. by deadline */
    private final List<JobStatus> mTrackedJobs = new LinkedList<>();
@@ -73,7 +69,6 @@ public final class TimeController extends StateController {

        mNextJobExpiredElapsedMillis = Long.MAX_VALUE;
        mNextDelayExpiredElapsedMillis = Long.MAX_VALUE;
        mChainedAttributionEnabled = mService.isChainedAttributionEnabled();
    }

    /**
@@ -117,7 +112,8 @@ public final class TimeController extends StateController {
            it.add(job);

            job.setTrackingController(JobStatus.TRACKING_TIME);
            WorkSource ws = deriveWorkSource(job.getSourceUid(), job.getSourcePackageName());
            WorkSource ws =
                    mService.deriveWorkSource(job.getSourceUid(), job.getSourcePackageName());

            // Only update alarms if the job would be ready with the relevant timing constraint
            // satisfied.
@@ -165,7 +161,7 @@ public final class TimeController extends StateController {
            } else if (wouldBeReadyWithConstraintLocked(job, JobStatus.CONSTRAINT_DEADLINE)) {
                // This job's deadline is earlier than the current set alarm. Update the alarm.
                setDeadlineExpiredAlarmLocked(job.getLatestRunTimeElapsed(),
                        deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
                        mService.deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
            }
        }
        if (job.hasTimingDelayConstraint()
@@ -177,7 +173,7 @@ public final class TimeController extends StateController {
                    && wouldBeReadyWithConstraintLocked(job, JobStatus.CONSTRAINT_TIMING_DELAY)) {
                // This job's delay is earlier than the current set alarm. Update the alarm.
                setDelayExpiredAlarmLocked(job.getEarliestRunTime(),
                        deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
                        mService.deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
            }
        }
    }
@@ -248,7 +244,7 @@ public final class TimeController extends StateController {
                }
            }
            setDeadlineExpiredAlarmLocked(nextExpiryTime,
                    deriveWorkSource(nextExpiryUid, nextExpiryPackageName));
                    mService.deriveWorkSource(nextExpiryUid, nextExpiryPackageName));
        }
    }

@@ -312,19 +308,7 @@ public final class TimeController extends StateController {
                mStateChangedListener.onControllerStateChanged();
            }
            setDelayExpiredAlarmLocked(nextDelayTime,
                    deriveWorkSource(nextDelayUid, nextDelayPackageName));
        }
    }

    private WorkSource deriveWorkSource(int uid, @Nullable String packageName) {
        if (mChainedAttributionEnabled) {
            WorkSource ws = new WorkSource();
            ws.createWorkChain()
                    .addNode(uid, packageName)
                    .addNode(Process.SYSTEM_UID, "JobScheduler");
            return ws;
        } else {
            return packageName == null ? new WorkSource(uid) : new WorkSource(uid, packageName);
                    mService.deriveWorkSource(nextDelayUid, nextDelayPackageName));
        }
    }

+0 −5
Original line number Diff line number Diff line
@@ -90,11 +90,6 @@ public class JobSchedulerServiceTest {
            super(context);
            mAppStateTracker = mock(AppStateTrackerImpl.class);
        }

        @Override
        public boolean isChainedAttributionEnabled() {
            return false;
        }
    }

    @Before