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

Commit 970089c5 authored by Kweku Adams's avatar Kweku Adams
Browse files

Close preemption loop.

Prior to this change, whenever we preempted a job, the preempted job
would be stopped, but then we wouldn't explicitly start the higher
priority job until we re-evaluated all jobs and regenerated the pending
job list. This was inefficient. Now, whenever a job finishes (via
preemption or any other ending), JobConcurrencyManager will immediately
give the JobServiceContext a new job to start, if limits allow.

Bug: 178119369
Test: atest CtsJobSchedulerTestCases
Test: atest FrameworksMockingServicesTests:JobSchedulerServiceTest
Test: atest FrameworksServicesTests:PrioritySchedulingTest
Test: atest FrameworksServicesTests:WorkCountTrackerTest
Test: atest FrameworksServicesTests:WorkTypeConfigTest
Change-Id: I00a1aca6a3ec42cf6b62712313fa4d1698e34f51
parent 9e86c288
Loading
Loading
Loading
Loading
+146 −1
Original line number Diff line number Diff line
@@ -62,6 +62,7 @@ class JobConcurrencyManager {
            CONFIG_KEY_PREFIX_CONCURRENCY + "screen_off_adjustment_delay_ms";
    private static final long DEFAULT_SCREEN_OFF_ADJUSTMENT_DELAY_MS = 30_000;

    // Try to give higher priority types lower values.
    static final int WORK_TYPE_NONE = 0;
    static final int WORK_TYPE_TOP = 1 << 0;
    static final int WORK_TYPE_BG = 1 << 1;
@@ -520,6 +521,120 @@ class JobConcurrencyManager {
        }
    }

    void onJobCompletedLocked(@NonNull JobServiceContext worker, @NonNull JobStatus jobStatus,
            @WorkType final int workType) {
        mWorkCountTracker.onJobFinished(workType);
        mRunningJobs.remove(jobStatus);
        final List<JobStatus> pendingJobs = mService.mPendingJobs;
        if (worker.getPreferredUid() != JobServiceContext.NO_PREFERRED_UID) {
            updateCounterConfigLocked();
            // Preemption case needs special care.
            updateNonRunningPriorities(pendingJobs, false);

            JobStatus highestPriorityJob = null;
            int highPriWorkType = workType;
            JobStatus backupJob = null;
            int backupWorkType = WORK_TYPE_NONE;
            for (int i = 0; i < pendingJobs.size(); i++) {
                final JobStatus nextPending = pendingJobs.get(i);

                if (mRunningJobs.contains(nextPending)) {
                    continue;
                }

                if (worker.getPreferredUid() != nextPending.getUid()) {
                    if (backupJob == null) {
                        int workAsType =
                                mWorkCountTracker.canJobStart(getJobWorkTypes(nextPending));
                        if (workAsType != WORK_TYPE_NONE) {
                            backupJob = nextPending;
                            backupWorkType = workAsType;
                        }
                    }
                    continue;
                }

                if (highestPriorityJob == null
                        || highestPriorityJob.lastEvaluatedPriority
                        < nextPending.lastEvaluatedPriority) {
                    highestPriorityJob = nextPending;
                } else {
                    continue;
                }

                // In this path, we pre-empted an existing job. We don't fully care about the
                // reserved slots. We should just run the highest priority job we can find,
                // though it would be ideal to use an available WorkType slot instead of
                // overloading slots.
                final int workAsType = mWorkCountTracker.canJobStart(getJobWorkTypes(nextPending));
                if (workAsType == WORK_TYPE_NONE) {
                    // Just use the preempted job's work type since this new one is technically
                    // replacing it anyway.
                    highPriWorkType = workType;
                } else {
                    highPriWorkType = workAsType;
                }
            }
            if (highestPriorityJob != null) {
                if (DEBUG) {
                    Slog.d(TAG, "Running job " + jobStatus + " as preemption");
                }
                mWorkCountTracker.stageJob(highPriWorkType);
                startJobLocked(worker, highestPriorityJob, highPriWorkType);
            } else {
                if (DEBUG) {
                    Slog.d(TAG, "Couldn't find preemption job for uid " + worker.getPreferredUid());
                }
                worker.clearPreferredUid();
                if (backupJob != null) {
                    if (DEBUG) {
                        Slog.d(TAG, "Running job " + jobStatus + " instead");
                    }
                    mWorkCountTracker.stageJob(backupWorkType);
                    startJobLocked(worker, backupJob, backupWorkType);
                }
            }
        } else if (pendingJobs.size() > 0) {
            updateCounterConfigLocked();
            updateNonRunningPriorities(pendingJobs, false);

            // This slot is now free and we have pending jobs. Start the highest priority job we
            // find.
            JobStatus highestPriorityJob = null;
            int highPriWorkType = workType;
            for (int i = 0; i < pendingJobs.size(); i++) {
                final JobStatus nextPending = pendingJobs.get(i);

                if (mRunningJobs.contains(nextPending)) {
                    continue;
                }

                final int workAsType = mWorkCountTracker.canJobStart(getJobWorkTypes(nextPending));
                if (workAsType == WORK_TYPE_NONE) {
                    continue;
                }
                if (highestPriorityJob == null
                        || highestPriorityJob.lastEvaluatedPriority
                        < nextPending.lastEvaluatedPriority) {
                    highestPriorityJob = nextPending;
                    highPriWorkType = workAsType;
                }
            }

            if (highestPriorityJob != null) {
                // This slot is free, and we haven't yet hit the limit on
                // concurrent jobs...  we can just throw the job in to here.
                if (DEBUG) {
                    Slog.d(TAG, "About to run job: " + jobStatus);
                }
                mWorkCountTracker.stageJob(highPriWorkType);
                startJobLocked(worker, highestPriorityJob, highPriWorkType);
            }
        }

        noteConcurrency();
    }

    @GuardedBy("mLock")
    private String printPendingQueueLocked() {
        StringBuilder s = new StringBuilder("Pending queue: ");
@@ -855,9 +970,27 @@ class JobConcurrencyManager {
            if (numRemainingForType < mNumActuallyReservedSlots.get(workType)) {
                // We've run all jobs for this type. Let another type use it now.
                mNumActuallyReservedSlots.put(workType, numRemainingForType);
                int assignWorkType = WORK_TYPE_NONE;
                for (int i = 0; i < mNumActuallyReservedSlots.size(); ++i) {
                    int wt = mNumActuallyReservedSlots.keyAt(i);
                    if (assignWorkType == WORK_TYPE_NONE || wt < assignWorkType) {
                        // Try to give this slot to the highest priority one within its limits.
                        int total = mNumRunningJobs.get(wt) + mNumStartingJobs.get(wt)
                                + mNumPendingJobs.get(wt);
                        if (mNumActuallyReservedSlots.valueAt(i) < mConfigAbsoluteMaxSlots.get(wt)
                                && total > mNumActuallyReservedSlots.valueAt(i)) {
                            assignWorkType = wt;
                        }
                    }
                }
                if (assignWorkType != WORK_TYPE_NONE) {
                    mNumActuallyReservedSlots.put(assignWorkType,
                            mNumActuallyReservedSlots.get(assignWorkType) + 1);
                } else {
                    mNumUnspecializedRemaining++;
                }
            }
        }

        void onJobStarted(@WorkType int workType) {
            mNumRunningJobs.put(workType, mNumRunningJobs.get(workType) + 1);
@@ -871,6 +1004,18 @@ class JobConcurrencyManager {
            }
        }

        void onJobFinished(@WorkType int workType) {
            final int newNumRunningJobs = mNumRunningJobs.get(workType) - 1;
            if (newNumRunningJobs < 0) {
                // We are in a bad state. We will eventually recover when the pending list is
                // regenerated.
                Slog.e(TAG, "# running jobs for " + workType + " went negative.");
                return;
            }
            mNumRunningJobs.put(workType, newNumRunningJobs);
            maybeAdjustReservations(workType);
        }

        void onCountDone() {
            // Calculate how many slots to reserve for each work type. "Unspecialized" slots will
            // be reserved for higher importance types first (ie. top before bg).
+2 −6
Original line number Diff line number Diff line
@@ -1426,8 +1426,8 @@ public class JobSchedulerService extends com.android.server.SystemService
                // Create the "runners".
                for (int i = 0; i < MAX_JOB_CONTEXTS_COUNT; i++) {
                    mActiveServices.add(
                            new JobServiceContext(this, mBatteryStats, mJobPackageTracker,
                                    getContext().getMainLooper()));
                            new JobServiceContext(this, mConcurrencyManager, mBatteryStats,
                                    mJobPackageTracker, getContext().getMainLooper()));
                }
                // Attach jobs to their controllers.
                mJobs.forEachJob((job) -> {
@@ -1710,9 +1710,6 @@ public class JobSchedulerService extends com.android.server.SystemService
            if (DEBUG) {
                Slog.d(TAG, "Could not find job to remove. Was job removed while executing?");
            }
            // We still want to check for jobs to execute, because this job may have
            // scheduled a new job under the same job id, and now we can run it.
            mHandler.obtainMessage(MSG_CHECK_JOB_GREEDY).sendToTarget();
            return;
        }

@@ -1734,7 +1731,6 @@ public class JobSchedulerService extends com.android.server.SystemService
        }
        jobStatus.unprepareLocked();
        reportActiveLocked();
        mHandler.obtainMessage(MSG_CHECK_JOB_GREEDY).sendToTarget();
    }

    // StateChangedListener implementations.
+6 −2
Original line number Diff line number Diff line
@@ -107,6 +107,7 @@ public final class JobServiceContext implements ServiceConnection {
    private final Handler mCallbackHandler;
    /** Make callbacks to {@link JobSchedulerService} to inform on job completion status. */
    private final JobCompletedListener mCompletedListener;
    private final JobConcurrencyManager mJobConcurrencyManager;
    /** Used for service binding, etc. */
    private final Context mContext;
    private final Object mLock;
@@ -183,13 +184,14 @@ public final class JobServiceContext implements ServiceConnection {
        }
    }

    JobServiceContext(JobSchedulerService service, IBatteryStats batteryStats,
            JobPackageTracker tracker, Looper looper) {
    JobServiceContext(JobSchedulerService service, JobConcurrencyManager concurrencyManager,
            IBatteryStats batteryStats, JobPackageTracker tracker, Looper looper) {
        mContext = service.getContext();
        mLock = service.getLock();
        mBatteryStats = batteryStats;
        mJobPackageTracker = tracker;
        mCallbackHandler = new JobServiceHandler(looper);
        mJobConcurrencyManager = concurrencyManager;
        mCompletedListener = service;
        mAvailable = true;
        mVerb = VERB_FINISHED;
@@ -835,6 +837,7 @@ public final class JobServiceContext implements ServiceConnection {
        if (mWakeLock != null) {
            mWakeLock.release();
        }
        final int workType = mRunningJobWorkType;
        mContext.unbindService(JobServiceContext.this);
        mWakeLock = null;
        mRunningJob = null;
@@ -847,6 +850,7 @@ public final class JobServiceContext implements ServiceConnection {
        mAvailable = true;
        removeOpTimeOutLocked();
        mCompletedListener.onJobCompletedLocked(completedJob, reschedule);
        mJobConcurrencyManager.onJobCompletedLocked(this, completedJob, workType);
    }

    private void applyStoppedReasonLocked(String reason) {
+86 −4
Original line number Diff line number Diff line
@@ -77,18 +77,19 @@ public class WorkCountTrackerTest {
            for (int i = running.get(WORK_TYPE_BG); i > 0; i--) {
                if (mRandom.nextDouble() < stopRatio) {
                    running.put(WORK_TYPE_BG, running.get(WORK_TYPE_BG) - 1);
                    mWorkCountTracker.onJobFinished(WORK_TYPE_BG);
                }
            }
            for (int i = running.get(WORK_TYPE_TOP); i > 0; i--) {
                if (mRandom.nextDouble() < stopRatio) {
                    running.put(WORK_TYPE_TOP, running.get(WORK_TYPE_TOP) - 1);
                    mWorkCountTracker.onJobFinished(WORK_TYPE_TOP);
                }
            }
        }
    }


    private void startPendingJobs(Jobs jobs, int totalMax,
    private void recount(Jobs jobs, int totalMax,
            @NonNull List<Pair<Integer, Integer>> minLimits,
            @NonNull List<Pair<Integer, Integer>> maxLimits) {
        mWorkCountTracker.setConfig(new JobConcurrencyManager.WorkTypeConfig(
@@ -113,7 +114,9 @@ public class WorkCountTrackerTest {
        }

        mWorkCountTracker.onCountDone();
    }

    private void startPendingJobs(Jobs jobs) {
        while ((jobs.pending.get(WORK_TYPE_TOP) > 0
                && mWorkCountTracker.canJobStart(WORK_TYPE_TOP) != WORK_TYPE_NONE)
                || (jobs.pending.get(WORK_TYPE_BG) > 0
@@ -151,7 +154,8 @@ public class WorkCountTrackerTest {
            jobs.maybeFinishJobs(stopRatio);
            jobs.maybeEnqueueJobs(startRatio, fgJobRatio);

            startPendingJobs(jobs, totalMax, minLimits, maxLimits);
            recount(jobs, totalMax, minLimits, maxLimits);
            startPendingJobs(jobs);

            int totalRunning = 0;
            for (int r = 0; r < jobs.running.size(); ++r) {
@@ -316,7 +320,8 @@ public class WorkCountTrackerTest {
            jobs.pending.put(pend.first, pend.second);
        }

        startPendingJobs(jobs, totalMax, minLimits, maxLimits);
        recount(jobs, totalMax, minLimits, maxLimits);
        startPendingJobs(jobs);

        for (Pair<Integer, Integer> run : resultRunning) {
            assertWithMessage("Incorrect running result for work type " + run.first)
@@ -421,4 +426,81 @@ public class WorkCountTrackerTest {
                /* resRun */ List.of(Pair.create(WORK_TYPE_BG, 6)),
                /* resPen */ List.of(Pair.create(WORK_TYPE_TOP, 10), Pair.create(WORK_TYPE_BG, 3)));
    }

    /** Tests that the counter updates properly when jobs are stopped. */
    @Test
    public void testJobLifecycleLoop() {
        final Jobs jobs = new Jobs();
        jobs.pending.put(WORK_TYPE_TOP, 11);
        jobs.pending.put(WORK_TYPE_BG, 10);

        final int totalMax = 6;
        final List<Pair<Integer, Integer>> minLimits = List.of(Pair.create(WORK_TYPE_BG, 1));
        final List<Pair<Integer, Integer>> maxLimits = List.of(Pair.create(WORK_TYPE_BG, 5));

        recount(jobs, totalMax, minLimits, maxLimits);

        startPendingJobs(jobs);

        assertThat(jobs.running.get(WORK_TYPE_TOP)).isEqualTo(5);
        assertThat(jobs.running.get(WORK_TYPE_BG)).isEqualTo(1);
        assertThat(jobs.pending.get(WORK_TYPE_TOP)).isEqualTo(6);
        assertThat(jobs.pending.get(WORK_TYPE_BG)).isEqualTo(9);

        // Stop all jobs
        jobs.maybeFinishJobs(1);

        assertThat(mWorkCountTracker.canJobStart(WORK_TYPE_TOP)).isEqualTo(WORK_TYPE_TOP);
        assertThat(mWorkCountTracker.canJobStart(WORK_TYPE_BG)).isEqualTo(WORK_TYPE_BG);

        startPendingJobs(jobs);

        assertThat(jobs.running.get(WORK_TYPE_TOP)).isEqualTo(5);
        assertThat(jobs.running.get(WORK_TYPE_BG)).isEqualTo(1);
        assertThat(jobs.pending.get(WORK_TYPE_TOP)).isEqualTo(1);
        assertThat(jobs.pending.get(WORK_TYPE_BG)).isEqualTo(8);

        // Stop only a bg job and make sure the counter only allows another bg job to start.
        jobs.running.put(WORK_TYPE_BG, jobs.running.get(WORK_TYPE_BG) - 1);
        mWorkCountTracker.onJobFinished(WORK_TYPE_BG);

        assertThat(mWorkCountTracker.canJobStart(WORK_TYPE_TOP)).isEqualTo(WORK_TYPE_NONE);
        assertThat(mWorkCountTracker.canJobStart(WORK_TYPE_BG)).isEqualTo(WORK_TYPE_BG);

        startPendingJobs(jobs);

        assertThat(jobs.running.get(WORK_TYPE_TOP)).isEqualTo(5);
        assertThat(jobs.running.get(WORK_TYPE_BG)).isEqualTo(1);
        assertThat(jobs.pending.get(WORK_TYPE_TOP)).isEqualTo(1);
        assertThat(jobs.pending.get(WORK_TYPE_BG)).isEqualTo(7);

        // Stop only a top job and make sure the counter only allows another top job to start.
        jobs.running.put(WORK_TYPE_TOP, jobs.running.get(WORK_TYPE_TOP) - 1);
        mWorkCountTracker.onJobFinished(WORK_TYPE_TOP);

        assertThat(mWorkCountTracker.canJobStart(WORK_TYPE_TOP)).isEqualTo(WORK_TYPE_TOP);
        assertThat(mWorkCountTracker.canJobStart(WORK_TYPE_BG)).isEqualTo(WORK_TYPE_NONE);

        startPendingJobs(jobs);

        assertThat(jobs.running.get(WORK_TYPE_TOP)).isEqualTo(5);
        assertThat(jobs.running.get(WORK_TYPE_BG)).isEqualTo(1);
        assertThat(jobs.pending.get(WORK_TYPE_TOP)).isEqualTo(0);
        assertThat(jobs.pending.get(WORK_TYPE_BG)).isEqualTo(7);

        // Now that there are no more TOP jobs pending, BG should be able to start when TOP stops.
        for (int i = jobs.running.get(WORK_TYPE_TOP); i > 0; --i) {
            jobs.running.put(WORK_TYPE_TOP, jobs.running.get(WORK_TYPE_TOP) - 1);
            mWorkCountTracker.onJobFinished(WORK_TYPE_TOP);

            assertThat(mWorkCountTracker.canJobStart(WORK_TYPE_BG)).isEqualTo(WORK_TYPE_BG);
        }

        startPendingJobs(jobs);

        assertThat(jobs.running.get(WORK_TYPE_TOP)).isEqualTo(0);
        assertThat(jobs.running.get(WORK_TYPE_BG)).isEqualTo(5);
        assertThat(jobs.pending.get(WORK_TYPE_TOP)).isEqualTo(0);
        assertThat(jobs.pending.get(WORK_TYPE_BG)).isEqualTo(3);
    }
}