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

Commit 750d0f0c authored by Daichi Hirono's avatar Daichi Hirono Committed by Android (Google) Code Review
Browse files

Merge "Reuse buffer when reading bytes from files."

parents 153d7ada 2f310f6d
Loading
Loading
Loading
Loading
+25 −14
Original line number Diff line number Diff line
@@ -50,7 +50,8 @@ constexpr size_t MAX_REQUEST_SIZE = sizeof(struct fuse_in_header) +

static jclass app_fuse_class;
static jmethodID app_fuse_get_file_size;
static jmethodID app_fuse_get_object_bytes;
static jmethodID app_fuse_read_object_bytes;
static jfieldID app_fuse_buffer;

// NOTE:
// FuseRequest and FuseResponse shares the same buffer to save memory usage, so the handlers must
@@ -282,7 +283,7 @@ private:
        out->prepare_buffer(0);
        const int64_t result = get_object_bytes(it->second, offset, size, out->data());
        if (result < 0) {
            return -EIO;
            return result;
        }
        out->set_size(result);
        return 0;
@@ -332,20 +333,24 @@ private:
            uint64_t offset,
            uint32_t size,
            void* buf) {
        ScopedLocalRef<jbyteArray> array(env_, static_cast<jbyteArray>(env_->CallObjectMethod(
        const jlong read_size = env_->CallLongMethod(
                self_,
                app_fuse_get_object_bytes,
                inode,
                offset,
                size)));
                app_fuse_read_object_bytes,
                static_cast<jint>(inode),
                static_cast<jlong>(offset),
                static_cast<jlong>(size));
        if (read_size <= 0) {
            return read_size;
        }
        ScopedLocalRef<jbyteArray> array(
                env_, static_cast<jbyteArray>(env_->GetObjectField(self_, app_fuse_buffer)));
        if (array.get() == nullptr) {
            return -1;
            return -EFAULT;
        }
        ScopedByteArrayRO bytes(env_, array.get());
        if (bytes.get() == nullptr) {
            return -1;
            return -ENOMEM;
        }
        const uint32_t read_size = std::min(static_cast<uint32_t>(bytes.size()), size);
        memcpy(buf, bytes.get(), read_size);
        return read_size;
    }
@@ -451,10 +456,16 @@ jint JNI_OnLoad(JavaVM* vm, void* /* reserved */) {
        return -1;
    }

    app_fuse_get_object_bytes = env->GetMethodID(
            app_fuse_class, "getObjectBytes", "(IJI)[B");
    if (app_fuse_get_object_bytes == nullptr) {
        ALOGE("Can't find getObjectBytes");
    app_fuse_read_object_bytes = env->GetMethodID(
            app_fuse_class, "readObjectBytes", "(IJJ)J");
    if (app_fuse_read_object_bytes == nullptr) {
        ALOGE("Can't find readObjectBytes");
        return -1;
    }

    app_fuse_buffer = env->GetFieldID(app_fuse_class, "mBuffer", "[B");
    if (app_fuse_buffer == nullptr) {
        ALOGE("Can't find mBuffer");
        return -1;
    }

+36 −7
Original line number Diff line number Diff line
@@ -16,9 +16,11 @@

package com.android.mtp;

import android.annotation.WorkerThread;
import android.os.ParcelFileDescriptor;
import android.os.Process;
import android.os.storage.StorageManager;
import android.system.OsConstants;
import android.util.Log;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.Preconditions;
@@ -40,6 +42,13 @@ public class AppFuse {

    private final String mName;
    private final Callback mCallback;

    /**
     * Buffer for read bytes request.
     * Don't use the buffer from the out of AppFuseMessageThread.
     */
    private byte[] mBuffer = new byte[MAX_READ];

    private Thread mMessageThread;
    private ParcelFileDescriptor mDeviceFd;

@@ -82,28 +91,48 @@ public class AppFuse {
    }

    static interface Callback {
        /**
         * Returns file size for the given inode.
         * @param inode
         * @return File size. Must not be negative.
         * @throws FileNotFoundException
         */
        long getFileSize(int inode) throws FileNotFoundException;
        byte[] getObjectBytes(int inode, long offset, int size) throws IOException;

        /**
         * Returns flie bytes for the give inode.
         * @param inode
         * @param offset Offset for file bytes.
         * @param size Size for file bytes.
         * @param bytes Buffer to store file bytes.
         * @return Number of read bytes. Must not be negative.
         * @throws IOException
         */
        long readObjectBytes(int inode, long offset, long size, byte[] bytes) throws IOException;
    }

    @UsedByNative("com_android_mtp_AppFuse.cpp")
    @WorkerThread
    private long getFileSize(int inode) {
        try {
            return mCallback.getFileSize(inode);
        } catch (IOException e) {
            return -1;
        } catch (FileNotFoundException e) {
            return -OsConstants.ENOENT;
        }
    }

    @UsedByNative("com_android_mtp_AppFuse.cpp")
    private byte[] getObjectBytes(int inode, long offset, int size) {
    @WorkerThread
    private long readObjectBytes(int inode, long offset, long size) {
        if (offset < 0 || size < 0 || size > MAX_READ) {
            return null;
            return -OsConstants.EINVAL;
        }
        try {
            return mCallback.getObjectBytes(inode, offset, size);
            // It's OK to share the same mBuffer among requests because the requests are processed
            // by AppFuseMessageThread sequentially.
            return mCallback.readObjectBytes(inode, offset, size, mBuffer);
        } catch (IOException e) {
            return null;
            return -OsConstants.EIO;
        }
    }

+4 −11
Original line number Diff line number Diff line
@@ -16,8 +16,6 @@

package com.android.mtp;

import static com.android.internal.util.Preconditions.checkArgument;

import android.content.ContentResolver;
import android.content.res.AssetFileDescriptor;
import android.content.res.Resources;
@@ -40,7 +38,6 @@ import com.android.internal.annotations.VisibleForTesting;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;

@@ -391,16 +388,12 @@ public class MtpDocumentsProvider extends DocumentsProvider {
    }

    private class AppFuseCallback implements AppFuse.Callback {
        final byte[] mBytes = new byte[AppFuse.MAX_READ];

        @Override
        public byte[] getObjectBytes(int inode, long offset, int size) throws IOException {
        public long readObjectBytes(
                int inode, long offset, long size, byte[] buffer) throws IOException {
            final Identifier identifier = mDatabase.createIdentifier(Integer.toString(inode));
            final long readSize = mMtpManager.getPartialObject(
                    identifier.mDeviceId, identifier.mObjectHandle, offset, size, mBytes);
            // TODO: Change signature so that getObjectBytes can return read size without copying
            // bytes.
            return Arrays.copyOf(mBytes, (int) readSize);
            return mMtpManager.getPartialObject(
                    identifier.mDeviceId, identifier.mObjectHandle, offset, size, buffer);
        }

        @Override
+15 −10
Original line number Diff line number Diff line
@@ -77,30 +77,35 @@ public class AppFuseTest extends AndroidTestCase {

    public void testReadFile() throws IOException {
        final StorageManager storageManager = getContext().getSystemService(StorageManager.class);
        final int INODE = 10;
        final byte[] BYTES = new byte[] { 'a', 'b', 'c', 'd', 'e' };
        final int fileInode = 10;
        final byte[] fileBytes = new byte[] { 'a', 'b', 'c', 'd', 'e' };
        final AppFuse appFuse = new AppFuse(
                "test",
                new TestCallback() {
                    @Override
                    public long getFileSize(int inode) throws FileNotFoundException {
                        if (inode == INODE) {
                            return BYTES.length;
                        if (inode == fileInode) {
                            return fileBytes.length;
                        }
                        return super.getFileSize(inode);
                    }

                    @Override
                    public byte[] getObjectBytes(int inode, long offset, int size)
                    public long readObjectBytes(int inode, long offset, long size, byte[] bytes)
                            throws IOException {
                        if (inode == INODE) {
                            return Arrays.copyOfRange(BYTES, (int) offset, (int) offset + size);
                        if (inode == fileInode) {
                            int i = 0;
                            while (i < size && i + offset < fileBytes.length)  {
                                bytes[i] = fileBytes[(int) (i + offset)];
                                i++;
                            }
                            return i;
                        }
                        return super.getObjectBytes(inode, offset, size);
                        return super.readObjectBytes(inode, offset, size, bytes);
                    }
                });
        appFuse.mount(storageManager);
        final ParcelFileDescriptor fd = appFuse.openFile(INODE);
        final ParcelFileDescriptor fd = appFuse.openFile(fileInode);
        try (final ParcelFileDescriptor.AutoCloseInputStream stream =
                new ParcelFileDescriptor.AutoCloseInputStream(fd)) {
            final byte[] buffer = new byte[1024];
@@ -117,7 +122,7 @@ public class AppFuseTest extends AndroidTestCase {
        }

        @Override
        public byte[] getObjectBytes(int inode, long offset, int size)
        public long readObjectBytes(int inode, long offset, long size, byte[] bytes)
                throws IOException {
            throw new IOException();
        }