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

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

Try to avoid excessive delays for UI jobs. 

Try to avoid "starvation" of background user-initiated jobs by creating
an additional context if they're going to be waiting too long for a new
slot.

Also, enforce that we only ever create one additional slot for a job
type so that we don't keep creating new JobServiceContexts just because
another job became eligible to run before executing jobs finished.

Bug: 248386641
Bug: 261999509
Test: atest FrameworksMockingServicesTests:JobConcurrencyManagerTest
Change-Id: I5d143b14422cec8f98812390bb306acf0026baff
parent c0bcd9af

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

+66 −10
Original line number Diff line number Diff line
@@ -129,6 +129,10 @@ class JobConcurrencyManager { 
    static final String KEY_ENABLE_MAX_WAIT_TIME_BYPASS =

            CONFIG_KEY_PREFIX_CONCURRENCY + "enable_max_wait_time_bypass";

    private static final boolean DEFAULT_ENABLE_MAX_WAIT_TIME_BYPASS = true;

    @VisibleForTesting

    static final String KEY_MAX_WAIT_UI_MS = CONFIG_KEY_PREFIX_CONCURRENCY + "max_wait_ui_ms";

    @VisibleForTesting

    static final long DEFAULT_MAX_WAIT_UI_MS = 5 * MINUTE_IN_MILLIS;

    private static final String KEY_MAX_WAIT_EJ_MS =

            CONFIG_KEY_PREFIX_CONCURRENCY + "max_wait_ej_ms";

    @VisibleForTesting
@@ -432,6 +436,12 @@ class JobConcurrencyManager { 


    private boolean mMaxWaitTimeBypassEnabled = DEFAULT_ENABLE_MAX_WAIT_TIME_BYPASS;



    /**

     * The maximum time a user-initiated job would have to be potentially waiting for an available

     * slot before we would consider creating a new slot for it.

     */

    private long mMaxWaitUIMs = DEFAULT_MAX_WAIT_UI_MS;



    /**

     * The maximum time an expedited job would have to be potentially waiting for an available

     * slot before we would consider creating a new slot for it.
@@ -825,6 +835,13 @@ class JobConcurrencyManager { 
                if (js.startedWithImmediacyPrivilege) {

                    info.numRunningImmediacyPrivileged++;

                }

                if (js.shouldTreatAsUserInitiatedJob()) {

                    info.numRunningUi++;

                } else if (js.startedAsExpeditedJob) {

                    info.numRunningEj++;

                } else {

                    info.numRunningReg++;

                }

            }



            assignment.preferredUid = jsc.getPreferredUid();
@@ -880,6 +897,13 @@ class JobConcurrencyManager { 
        JobStatus nextPending;

        int projectedRunningCount = activeServices.size();

        long minChangedWaitingTimeMs = Long.MAX_VALUE;

        // Only allow the Context creation bypass for each type if one of that type isn't already

        // running. That way, we don't run into issues (creating too many additional contexts)

        // if new jobs become ready to run in rapid succession and we end up going through this

        // loop many times before running jobs have had a decent chance to finish.

        boolean allowMaxWaitContextBypassUi = info.numRunningUi == 0;

        boolean allowMaxWaitContextBypassEj = info.numRunningEj == 0;

        boolean allowMaxWaitContextBypassOthers = info.numRunningReg == 0;

        while ((nextPending = pendingJobQueue.next()) != null) {

            if (mRunningJobs.contains(nextPending)) {

                // Should never happen.
@@ -957,7 +981,9 @@ class JobConcurrencyManager { 
                                        > (mWorkTypeConfig.getMaxTotal() / 2);

                    }

                    if (!canReplace && mMaxWaitTimeBypassEnabled) { // Case 5

                        if (nextPending.shouldTreatAsExpeditedJob()) {

                        if (nextPending.shouldTreatAsUserInitiatedJob()) {

                            canReplace = minWaitingTimeMs >= mMaxWaitUIMs;

                        } else if (nextPending.shouldTreatAsExpeditedJob()) {

                            canReplace = minWaitingTimeMs >= mMaxWaitEjMs;

                        } else {

                            canReplace = minWaitingTimeMs >= mMaxWaitRegularMs;
@@ -1055,9 +1081,25 @@ class JobConcurrencyManager { 
                            (workType != WORK_TYPE_NONE) ? workType : WORK_TYPE_TOP;

                }

            } else if (selectedContext == null && mMaxWaitTimeBypassEnabled) {

                final boolean wouldBeWaitingTooLong = nextPending.shouldTreatAsExpeditedJob()

                        ? minWaitingTimeMs >= mMaxWaitEjMs

                        : minWaitingTimeMs >= mMaxWaitRegularMs;

                final boolean wouldBeWaitingTooLong;

                if (nextPending.shouldTreatAsUserInitiatedJob() && allowMaxWaitContextBypassUi) {

                    wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitUIMs;

                    // We want to create at most one additional context for each type.

                    allowMaxWaitContextBypassUi = !wouldBeWaitingTooLong;

                } else if (nextPending.shouldTreatAsExpeditedJob() && allowMaxWaitContextBypassEj) {

                    wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitEjMs;

                    // We want to create at most one additional context for each type.

                    allowMaxWaitContextBypassEj = !wouldBeWaitingTooLong;

                } else if (allowMaxWaitContextBypassOthers) {

                    // The way things are set up a UIJ or EJ could end up here and create a 2nd

                    // context as if it were a "regular" job. That's fine for now since they would

                    // still be subject to the higher waiting time threshold here.

                    wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitRegularMs;

                    // We want to create at most one additional context for each type.

                    allowMaxWaitContextBypassOthers = !wouldBeWaitingTooLong;

                } else {

                    wouldBeWaitingTooLong = false;

                }

                if (wouldBeWaitingTooLong) {

                    if (DEBUG) {

                        Slog.d(TAG, "Allowing additional context because job would wait too long");
@@ -1490,10 +1532,14 @@ class JobConcurrencyManager { 
                    minWaitingTimeMs = Math.min(minWaitingTimeMs,

                            mActiveServices.get(i).getRemainingGuaranteedTimeMs(nowElapsed));

                }

                final boolean wouldBeWaitingTooLong =

                        mWorkCountTracker.getPendingJobCount(WORK_TYPE_EJ) > 0

                                ? minWaitingTimeMs >= mMaxWaitEjMs

                                : minWaitingTimeMs >= mMaxWaitRegularMs;

                final boolean wouldBeWaitingTooLong;

                if (mWorkCountTracker.getPendingJobCount(WORK_TYPE_UI) > 0) {

                    wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitUIMs;

                } else if (mWorkCountTracker.getPendingJobCount(WORK_TYPE_EJ) > 0) {

                    wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitEjMs;

                } else {

                    wouldBeWaitingTooLong = minWaitingTimeMs >= mMaxWaitRegularMs;

                }

                respectConcurrencyLimit = !wouldBeWaitingTooLong;

            }

            if (respectConcurrencyLimit) {
@@ -1908,8 +1954,11 @@ class JobConcurrencyManager { 


        mMaxWaitTimeBypassEnabled = properties.getBoolean(

                KEY_ENABLE_MAX_WAIT_TIME_BYPASS, DEFAULT_ENABLE_MAX_WAIT_TIME_BYPASS);

        // EJ max wait must be in the range [0, infinity).

        mMaxWaitEjMs = Math.max(0, properties.getLong(KEY_MAX_WAIT_EJ_MS, DEFAULT_MAX_WAIT_EJ_MS));

        // UI max wait must be in the range [0, infinity).

        mMaxWaitUIMs = Math.max(0, properties.getLong(KEY_MAX_WAIT_UI_MS, DEFAULT_MAX_WAIT_UI_MS));

        // EJ max wait must be in the range [UI max wait, infinity).

        mMaxWaitEjMs = Math.max(mMaxWaitUIMs,

                properties.getLong(KEY_MAX_WAIT_EJ_MS, DEFAULT_MAX_WAIT_EJ_MS));

        // Regular max wait must be in the range [EJ max wait, infinity).

        mMaxWaitRegularMs = Math.max(mMaxWaitEjMs,

                properties.getLong(KEY_MAX_WAIT_REGULAR_MS, DEFAULT_MAX_WAIT_REGULAR_MS));
@@ -1928,6 +1977,7 @@ class JobConcurrencyManager { 
            pw.print(KEY_PKG_CONCURRENCY_LIMIT_EJ, mPkgConcurrencyLimitEj).println();

            pw.print(KEY_PKG_CONCURRENCY_LIMIT_REGULAR, mPkgConcurrencyLimitRegular).println();

            pw.print(KEY_ENABLE_MAX_WAIT_TIME_BYPASS, mMaxWaitTimeBypassEnabled).println();

            pw.print(KEY_MAX_WAIT_UI_MS, mMaxWaitUIMs).println();

            pw.print(KEY_MAX_WAIT_EJ_MS, mMaxWaitEjMs).println();

            pw.print(KEY_MAX_WAIT_REGULAR_MS, mMaxWaitRegularMs).println();

            pw.println();
@@ -2790,10 +2840,16 @@ class JobConcurrencyManager { 
    static final class AssignmentInfo {

        public long minPreferredUidOnlyWaitingTimeMs;

        public int numRunningImmediacyPrivileged;

        public int numRunningUi;

        public int numRunningEj;

        public int numRunningReg;



        void clear() {

            minPreferredUidOnlyWaitingTimeMs = 0;

            numRunningImmediacyPrivileged = 0;

            numRunningUi = 0;

            numRunningEj = 0;

            numRunningReg = 0;

        }

    }

services/tests/mockingservicestests/src/com/android/server/job/JobConcurrencyManagerTest.java

+120 −20
Original line number Diff line number Diff line
@@ -335,13 +335,14 @@ public final class JobConcurrencyManagerTest { 
        mConfigBuilder.setBoolean(JobConcurrencyManager.KEY_ENABLE_MAX_WAIT_TIME_BYPASS, true);

        setConcurrencyConfig(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT,

                new TypeConfig(WORK_TYPE_BG, 0, JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT));

        for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 2; ++i) {

        for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 3; ++i) {

            final int uid = mDefaultUserId * UserHandle.PER_USER_RANGE + i;

            final String sourcePkgName = "com.source.package." + UserHandle.getAppId(uid);

            setPackageUid(sourcePkgName, uid);

            final JobStatus job = createJob(uid, sourcePkgName);

            spyOn(job);

            doReturn(i % 2 == 0).when(job).shouldTreatAsExpeditedJob();

            doReturn(i % 3 == 0).when(job).shouldTreatAsUserInitiatedJob();

            doReturn(i % 3 == 1).when(job).shouldTreatAsExpeditedJob();

            if (i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) {

                mJobConcurrencyManager.addRunningJobForTesting(job);

            } else {
@@ -350,7 +351,7 @@ public final class JobConcurrencyManagerTest { 
        }



        // Waiting time is too short, so we shouldn't create any extra contexts.

        final long remainingTimeMs = JobConcurrencyManager.DEFAULT_MAX_WAIT_EJ_MS / 2;

        final long remainingTimeMs = JobConcurrencyManager.DEFAULT_MAX_WAIT_UI_MS / 2;

        for (int i = 0; i < mInjector.contexts.size(); ++i) {

            doReturn(true).when(mInjector.contexts.keyAt(i)).isWithinExecutionGuaranteeTime();

            doReturn(remainingTimeMs)
@@ -378,19 +379,97 @@ public final class JobConcurrencyManagerTest { 
    }



    @Test

    public void testDetermineAssignments_allPreferredUidOnly_mediumTimeLeft() throws Exception {

    public void testDetermineAssignments_allPreferredUidOnly_mediumTimeLeft_onlyRegRunning()

            throws Exception {

        mConfigBuilder.setBoolean(JobConcurrencyManager.KEY_ENABLE_MAX_WAIT_TIME_BYPASS, true);

        // Set the waiting time to be less than an EJ's min execution time.

        mConfigBuilder.setLong(JobConcurrencyManager.KEY_MAX_WAIT_UI_MS, 2 * 60_000L);

        setConcurrencyConfig(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT,

                new TypeConfig(WORK_TYPE_BG, 0, JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT));

        final ArraySet<JobStatus> jobs = new ArraySet<>();

        for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 2; ++i) {

        for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 4; ++i) {

            final int uid = mDefaultUserId * UserHandle.PER_USER_RANGE + i;

            final String sourcePkgName = "com.source.package." + UserHandle.getAppId(uid);

            setPackageUid(sourcePkgName, uid);

            final JobStatus job = createJob(uid, sourcePkgName);

            spyOn(job);

            doReturn(i % 2 == 0).when(job).shouldTreatAsExpeditedJob();

            if (i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) {

            doReturn(i % 3 == 0).when(job).shouldTreatAsUserInitiatedJob();

            doReturn(i % 3 == 1).when(job).shouldTreatAsExpeditedJob();

            if (i % 3 == 2

                    && mJobConcurrencyManager.mActiveServices.size()

                            < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) {

                mJobConcurrencyManager.addRunningJobForTesting(job);

            } else {

                mPendingJobQueue.add(job);

                jobs.add(job);

            }

        }



        // Waiting time is longer than the EJ & UI waiting time, but shorter than regular job

        // waiting time, so we should only create 2 extra contexts (one for EJ, one for UIJ).

        final long remainingTimeMs = (JobConcurrencyManager.DEFAULT_MAX_WAIT_EJ_MS

                + JobConcurrencyManager.DEFAULT_MAX_WAIT_REGULAR_MS) / 2;

        for (int i = 0; i < mInjector.contexts.size(); ++i) {

            doReturn(true).when(mInjector.contexts.keyAt(i)).isWithinExecutionGuaranteeTime();

            doReturn(remainingTimeMs)

                    .when(mInjector.contexts.keyAt(i)).getRemainingGuaranteedTimeMs(anyLong());

        }



        final ArraySet<JobConcurrencyManager.ContextAssignment> changed = new ArraySet<>();

        final ArraySet<JobConcurrencyManager.ContextAssignment> idle = new ArraySet<>();

        final List<JobConcurrencyManager.ContextAssignment> preferredUidOnly = new ArrayList<>();

        final List<JobConcurrencyManager.ContextAssignment> stoppable = new ArrayList<>();

        final JobConcurrencyManager.AssignmentInfo assignmentInfo =

                new JobConcurrencyManager.AssignmentInfo();



        mJobConcurrencyManager.prepareForAssignmentDeterminationLocked(

                idle, preferredUidOnly, stoppable, assignmentInfo);

        assertEquals(remainingTimeMs, assignmentInfo.minPreferredUidOnlyWaitingTimeMs);

        assertEquals(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT, preferredUidOnly.size());



        mJobConcurrencyManager

                .determineAssignmentsLocked(changed, idle, preferredUidOnly, stoppable,

                        assignmentInfo);



        assertEquals(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT, preferredUidOnly.size());

        for (int i = changed.size() - 1; i >= 0; --i) {

            jobs.remove(changed.valueAt(i).newJob);

        }

        // 1 EJ & 1 UIJ removed from the pending list.

        assertEquals(3 * JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT - 2, jobs.size());

        assertEquals(2, changed.size());

        JobStatus assignedJob1 = changed.valueAt(0).newJob;

        JobStatus assignedJob2 = changed.valueAt(1).newJob;

        boolean ejFirst = assignedJob1.shouldTreatAsExpeditedJob();

        if (ejFirst) {

            assertTrue(assignedJob1.shouldTreatAsExpeditedJob());

            assertTrue(assignedJob2.shouldTreatAsUserInitiatedJob());

        } else {

            assertTrue(assignedJob1.shouldTreatAsUserInitiatedJob());

            assertTrue(assignedJob2.shouldTreatAsExpeditedJob());

        }

    }



    @Test

    public void testDetermineAssignments_allPreferredUidOnly_mediumTimeLeft_onlyUiRunning()

            throws Exception {

        mConfigBuilder.setBoolean(JobConcurrencyManager.KEY_ENABLE_MAX_WAIT_TIME_BYPASS, true);

        // Set the waiting time to be less than an EJ's min execution time.

        mConfigBuilder.setLong(JobConcurrencyManager.KEY_MAX_WAIT_UI_MS, 2 * 60_000L);

        setConcurrencyConfig(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT,

                new TypeConfig(WORK_TYPE_BG, 0, JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT));

        final ArraySet<JobStatus> jobs = new ArraySet<>();

        for (int i = 0; i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT * 4; ++i) {

            final int uid = mDefaultUserId * UserHandle.PER_USER_RANGE + i;

            final String sourcePkgName = "com.source.package." + UserHandle.getAppId(uid);

            setPackageUid(sourcePkgName, uid);

            final JobStatus job = createJob(uid, sourcePkgName);

            spyOn(job);

            doReturn(i % 3 == 0).when(job).shouldTreatAsUserInitiatedJob();

            doReturn(i % 3 == 1).when(job).shouldTreatAsExpeditedJob();

            if (i % 3 == 0

                    && mJobConcurrencyManager.mActiveServices.size()

                            < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) {

                mJobConcurrencyManager.addRunningJobForTesting(job);

            } else {

                mPendingJobQueue.add(job);
@@ -398,8 +477,8 @@ public final class JobConcurrencyManagerTest { 
            }

        }



        // Waiting time is longer than the EJ waiting time, but shorter than regular job waiting

        // time, so we should only create an extra context for an EJ.

        // Waiting time is longer than the EJ & UI waiting time, but shorter than regular job

        // waiting time, so we should only create 2 extra contexts (one for EJ, one for UIJ).

        final long remainingTimeMs = (JobConcurrencyManager.DEFAULT_MAX_WAIT_EJ_MS

                + JobConcurrencyManager.DEFAULT_MAX_WAIT_REGULAR_MS) / 2;

        for (int i = 0; i < mInjector.contexts.size(); ++i) {
@@ -428,7 +507,9 @@ public final class JobConcurrencyManagerTest { 
        for (int i = changed.size() - 1; i >= 0; --i) {

            jobs.remove(changed.valueAt(i).newJob);

        }

        assertEquals(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT - 1, jobs.size());

        // There are already UIJs running, and wait time is too long for regular jobs, so

        // only 1 EJ removed from the pending list.

        assertEquals(3 * JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT - 1, jobs.size());

        assertEquals(1, changed.size());

        JobStatus assignedJob = changed.valueAt(0).newJob;

        assertTrue(assignedJob.shouldTreatAsExpeditedJob());
@@ -446,7 +527,8 @@ public final class JobConcurrencyManagerTest { 
            setPackageUid(sourcePkgName, uid);

            final JobStatus job = createJob(uid, sourcePkgName);

            spyOn(job);

            doReturn(i % 2 == 0).when(job).shouldTreatAsExpeditedJob();

            doReturn(i % 3 == 0).when(job).shouldTreatAsUserInitiatedJob();

            doReturn(i % 3 == 1).when(job).shouldTreatAsExpeditedJob();

            if (i < JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT) {

                mJobConcurrencyManager.addRunningJobForTesting(job);

            } else {
@@ -481,21 +563,39 @@ public final class JobConcurrencyManagerTest { 
                        assignmentInfo);



        assertEquals(JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT, preferredUidOnly.size());

        // Depending on iteration order, we may create 1 or 2 contexts.

        // Depending on iteration order, we may create 1-3 contexts.

        final long numAssignedJobs = changed.size();

        assertTrue(numAssignedJobs > 0);

        assertTrue(numAssignedJobs <= 2);

        assertTrue(numAssignedJobs <= 3);

        int numUi = 0, numEj = 0, numReg = 0;

        for (int i = 0; i < numAssignedJobs; ++i) {

            jobs.remove(changed.valueAt(i).newJob);

            JobStatus assignedJob = changed.valueAt(i).newJob;

            jobs.remove(assignedJob);

            if (assignedJob.shouldTreatAsUserInitiatedJob()) {

                numUi++;

            } else if (assignedJob.shouldTreatAsExpeditedJob()) {

                numEj++;

            } else {

                numReg++;

            }

        }

        assertEquals(numAssignedJobs,

                JobConcurrencyManager.DEFAULT_CONCURRENCY_LIMIT - jobs.size());

        JobStatus firstAssignedJob = changed.valueAt(0).newJob;

        if (!firstAssignedJob.shouldTreatAsExpeditedJob()) {

            assertEquals(2, numAssignedJobs);

            assertTrue(changed.valueAt(1).newJob.shouldTreatAsExpeditedJob());

        } else if (numAssignedJobs == 2) {

            assertFalse(changed.valueAt(1).newJob.shouldTreatAsExpeditedJob());

        if (numReg > 0) {

            assertEquals(1, numReg);

            assertEquals(1, numEj);

            assertEquals(1, numUi);

            assertEquals(3, numAssignedJobs);

        } else {

            if (numEj > 0) {

                assertEquals(1, numEj);

            }

            // If the manager looks at an EJ before a UIJ, the waiting time for the UIJ will drop

            // to 3 minutes and be below the threshold to create a new context.

            if (numUi > 0) {

                assertEquals(1, numUi);

            }

            assertEquals(numEj + numUi, numAssignedJobs);

        }

    }