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

Commit 19cf55b1 authored by Garfield Tan's avatar Garfield Tan Committed by Android (Google) Code Review
Browse files

Merge "Prioritize delete jobs by adding them to a separate thread pool." into nyc-andromeda-dev

parents 261aae12 810fb82f
Loading
Loading
Loading
Loading
+2 −10
Original line number Diff line number Diff line
@@ -869,12 +869,7 @@ public class DirectoryFragment extends Fragment
                        (TextView) mInflater.inflate(R.layout.dialog_delete_confirmation, null);
                message.setText(generateDeleteMessage(docs));

                // This "insta-hides" files that are being deleted, because
                // the delete operation may be not execute immediately (it
                // may be queued up on the FileOperationService.)
                // To hide the files locally, we call the hide method on the adapter
                // ...which a live object...cannot be parceled.
                // For that reason, for now, we implement this dialog NOT
                // For now, we implement this dialog NOT
                // as a fragment (which can survive rotation and have its own state),
                // but as a simple runtime dialog. So rotating a device with an
                // active delete dialog...results in that dialog disappearing.
@@ -895,10 +890,7 @@ public class DirectoryFragment extends Fragment
                                } else {
                                    Log.w(TAG, "Action mode is null before deleting documents.");
                                }
                                // Hide the files in the UI...since the operation
                                // might be queued up on FileOperationService.
                                // We're walking a line here.
                                mAdapter.hide(selected.getAll());

                                FileOperations.delete(
                                        getActivity(), docs, srcParent, getDisplayState().stack);
                            }
+0 −10
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@ import android.database.Cursor;
import android.provider.DocumentsContract.Document;
import android.support.v7.widget.GridLayoutManager;
import android.support.v7.widget.RecyclerView;
import android.util.SparseArray;

import com.android.documentsui.State;

@@ -64,15 +63,6 @@ abstract class DocumentsAdapter
     */
    abstract String getModelId(int position);

    /**
     * Hides a set of items from the associated RecyclerView.
     *
     * @param ids The Model IDs of the items to hide.
     * @return A SparseArray that maps the hidden IDs to their old positions. This can be used
     *         to {@link #unhide} the items if necessary.
     */
    abstract public SparseArray<String> hide(String... ids);

    /**
     * Returns a class that yields the span size for a particular element. This is
     * primarily useful in {@link SectionBreakDocumentsAdapterWrapper} where
+0 −22
Original line number Diff line number Diff line
@@ -25,11 +25,9 @@ import static com.android.documentsui.model.DocumentInfo.getCursorString;
import android.database.Cursor;
import android.provider.DocumentsContract.Document;
import android.util.Log;
import android.util.SparseArray;
import android.view.ViewGroup;

import com.android.documentsui.State;
import com.google.common.collect.Sets;

import java.util.ArrayList;
import java.util.HashSet;
@@ -165,26 +163,6 @@ final class ModelBackedDocumentsAdapter extends DocumentsAdapter {
        return mModelIds.get(adapterPosition);
    }

    @Override
    public SparseArray<String> hide(String... ids) {
        if (DEBUG) Log.d(TAG, "Hiding ids: " + ids);
        Set<String> toHide = Sets.newHashSet(ids);

        // Proceed backwards through the list of items, because each removal causes the
        // positions of all subsequent items to change.
        SparseArray<String> hiddenItems = new SparseArray<>();
        for (int i = mModelIds.size() - 1; i >= 0; --i) {
            String id = mModelIds.get(i);
            if (toHide.contains(id)) {
                mHiddenIds.add(id);
                hiddenItems.put(i, mModelIds.remove(i));
                notifyItemRemoved(i);
            }
        }

        return hiddenItems;
    }

    @Override
    public List<String> getModelIds() {
        return mModelIds;
+0 −8
Original line number Diff line number Diff line
@@ -20,7 +20,6 @@ import android.content.Context;
import android.database.Cursor;
import android.support.v7.widget.GridLayoutManager;
import android.support.v7.widget.RecyclerView.AdapterDataObserver;
import android.util.SparseArray;
import android.view.ViewGroup;
import android.widget.Space;

@@ -161,13 +160,6 @@ final class SectionBreakDocumentsAdapterWrapper extends DocumentsAdapter {
        return (mBreakPosition != -1 && p >= mBreakPosition) ? p + 1 : p;
    }

    @Override
    public SparseArray<String> hide(String... ids) {
        // NOTE: We hear about these changes and adjust break position
        // in our AdapterDataObserver.
        return mDelegate.hide(ids);
    }

    @Override
    List<String> getModelIds() {
        return mDelegate.getModelIds();
+28 −24
Original line number Diff line number Diff line
@@ -38,17 +38,14 @@ import java.lang.annotation.RetentionPolicy;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

import javax.annotation.concurrent.GuardedBy;

public class FileOperationService extends Service implements Job.Listener {

    private static final int DEFAULT_DELAY = 0;
    private static final int MAX_DELAY = 10 * 1000;  // ten seconds
    private static final int POOL_SIZE = 2;  // "pool size", not *max* "pool size".
    private static final int NOTIFICATION_ID_PROGRESS = 0;
    private static final int NOTIFICATION_ID_FAILURE = 1;
@@ -57,7 +54,6 @@ public class FileOperationService extends Service implements Job.Listener {
    public static final String TAG = "FileOperationService";

    public static final String EXTRA_JOB_ID = "com.android.documentsui.JOB_ID";
    public static final String EXTRA_DELAY = "com.android.documentsui.DELAY";
    public static final String EXTRA_OPERATION = "com.android.documentsui.OPERATION";
    public static final String EXTRA_CANCEL = "com.android.documentsui.CANCEL";
    public static final String EXTRA_SRC_LIST = "com.android.documentsui.SRC_LIST";
@@ -87,7 +83,10 @@ public class FileOperationService extends Service implements Job.Listener {
    // The executor and job factory are visible for testing and non-final
    // so we'll have a way to inject test doubles from the test. It's
    // a sub-optimal arrangement.
    @VisibleForTesting ScheduledExecutorService executor;
    @VisibleForTesting ExecutorService executor;

    // Use a separate thread pool to prioritize deletions
    @VisibleForTesting ExecutorService deletionExecutor;
    @VisibleForTesting Factory jobFactory;

    private PowerManager mPowerManager;
@@ -103,7 +102,11 @@ public class FileOperationService extends Service implements Job.Listener {
    public void onCreate() {
        // Allow tests to pre-set these with test doubles.
        if (executor == null) {
            executor = new ScheduledThreadPoolExecutor(POOL_SIZE);
            executor = Executors.newFixedThreadPool(POOL_SIZE);
        }

        if (deletionExecutor == null) {
            deletionExecutor = Executors.newCachedThreadPool();
        }

        if (jobFactory == null) {
@@ -171,11 +174,8 @@ public class FileOperationService extends Service implements Job.Listener {
        }

        assert(job != null);
        int delay = intent.getIntExtra(EXTRA_DELAY, DEFAULT_DELAY);
        assert(delay <= MAX_DELAY);
        if (DEBUG) Log.d(
                TAG, "Scheduling job " + job.id + " to run in " + delay + " milliseconds.");
        ScheduledFuture<?> future = executor.schedule(job, delay, TimeUnit.MILLISECONDS);
        if (DEBUG) Log.d(TAG, "Scheduling job " + job.id + ".");
        Future<?> future = getExecutorService(operationType).submit(job);
        mRunning.put(jobId, new JobRecord(job, future));
    }

@@ -200,14 +200,6 @@ public class FileOperationService extends Service implements Job.Listener {
            JobRecord record = mRunning.get(jobId);
            if (record != null) {
                record.job.cancel();

                // If the job hasn't been started, cancel it and explicitly clean up.
                // If it *has* been started, we wait for it to recognize this, then
                // allow it stop working in an orderly fashion.
                if (record.future.getDelay(TimeUnit.MILLISECONDS) > 0) {
                    record.future.cancel(false);
                    onFinished(record.job);
                }
            }
        }

@@ -257,6 +249,18 @@ public class FileOperationService extends Service implements Job.Listener {
        }
    }

    private ExecutorService getExecutorService(@OpType int operationType) {
        switch (operationType) {
            case OPERATION_COPY:
            case OPERATION_MOVE:
                return executor;
            case OPERATION_DELETE:
                return deletionExecutor;
            default:
                throw new UnsupportedOperationException();
        }
    }

    @GuardedBy("mRunning")
    private void deleteJob(Job job) {
        if (DEBUG) Log.d(TAG, "deleteJob: " + job.id);
@@ -333,9 +337,9 @@ public class FileOperationService extends Service implements Job.Listener {

    private static final class JobRecord {
        private final Job job;
        private final ScheduledFuture<?> future;
        private final Future<?> future;

        public JobRecord(Job job, ScheduledFuture<?> future) {
        public JobRecord(Job job, Future<?> future) {
            this.job = job;
            this.future = future;
        }
Loading