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

Commit cccdcae1 authored by Chalard Jean's avatar Chalard Jean
Browse files

Make the memory store operations serial.

Bug: 128499160
Test: new test in this patch

Change-Id: I6ccdc801e3888a61b22272c8ce9480f45fa26df2
(cherry picked from commit 3074f10b41fd1b622dc698d89e4c9e1bcb5b05cf)
Merged-In: I10b5c5cd85fcb76924ba96c8c379be677774705d
parent c6e7d008
Loading
Loading
Loading
Loading
+11 −18
Original line number Diff line number Diff line
@@ -60,7 +60,6 @@ import java.util.concurrent.Executors;
 */
public class IpMemoryStoreService extends IIpMemoryStore.Stub {
    private static final String TAG = IpMemoryStoreService.class.getSimpleName();
    private static final int MAX_CONCURRENT_THREADS = 4;
    private static final int DATABASE_SIZE_THRESHOLD = 10 * 1024 * 1024; //10MB
    private static final int MAX_DROP_RECORD_TIMES = 500;
    private static final int MIN_DELETE_NUM = 5;
@@ -107,23 +106,17 @@ public class IpMemoryStoreService extends IIpMemoryStore.Stub {
            db = null;
        }
        mDb = db;
        // The work-stealing thread pool executor will spawn threads as needed up to
        // the max only when there is no free thread available. This generally behaves
        // exactly like one would expect it intuitively :
        // - When work arrives, it will spawn a new thread iff there are no available threads
        // - When there is no work to do it will shutdown threads after a while (the while
        //   being equal to 2 seconds (not configurable) when max threads are spun up and
        //   twice as much for every one less thread)
        // - When all threads are busy the work is enqueued and waits for any worker
        //   to become available.
        // Because the stealing pool is made for very heavily parallel execution of
        // small tasks that spawn others, it creates a queue per thread that in this
        // case is overhead. However, the three behaviors above make it a superior
        // choice to cached or fixedThreadPoolExecutor, neither of which can actually
        // enqueue a task waiting for a thread to be free. This can probably be solved
        // with judicious subclassing of ThreadPoolExecutor, but that's a lot of dangerous
        // complexity for little benefit in this case.
        mExecutor = Executors.newWorkStealingPool(MAX_CONCURRENT_THREADS);
        // The single thread executor guarantees that all work is executed sequentially on the
        // same thread, and no two tasks can be active at the same time. This is required to
        // ensure operations from multiple clients don't interfere with each other (in particular,
        // operations involving a transaction must not run concurrently with other operations
        // as the other operations might be taken as part of the transaction). By default, the
        // single thread executor runs off an unbounded queue.
        // TODO : investigate replacing this scheme with a scheme where each thread has its own
        // instance of the database, as it may be faster. It is likely however that IpMemoryStore
        // operations are mostly IO-bound anyway, and additional contention is unlikely to bring
        // benefits. Alternatively, a read-write lock might increase throughput.
        mExecutor = Executors.newSingleThreadExecutor();
        RegularMaintenanceJobService.schedule(mContext, this);
    }

+21 −0
Original line number Diff line number Diff line
@@ -732,4 +732,25 @@ public class IpMemoryStoreServiceTest {
                            latch.countDown();
                        })));
    }

    public void testTasksAreSerial() {
        final long sleepTimeMs = 1000;
        final long startTime = System.currentTimeMillis();
        mService.retrieveNetworkAttributes("somekey", onNetworkAttributesRetrieved(
                (status, key, attr) -> {
                    assertTrue("Unexpected status : " + status.resultCode, status.isSuccess());
                    try {
                        Thread.sleep(sleepTimeMs);
                    } catch (InterruptedException e) {
                        fail("InterruptedException");
                    }
                }));
        doLatched("Serial tasks timing out", latch ->
                mService.retrieveNetworkAttributes("somekey", onNetworkAttributesRetrieved(
                        (status, key, attr) -> {
                            assertTrue("Unexpected status : " + status.resultCode,
                                    status.isSuccess());
                            assertTrue(System.currentTimeMillis() >= startTime + sleepTimeMs);
                        })), DEFAULT_TIMEOUT_MS);
    }
}