From 65af26993b609ea0e8b5e3fdfbb0121127ba25e7 Mon Sep 17 00:00:00 2001 From: Christophe Beyls Date: Wed, 10 Apr 2024 21:52:55 +0200 Subject: [PATCH] Optimize I/O code using Okio (#4366) This pull request takes advantage of the Okio library to simplify, fix or improve performance of some I/O related code in Tusky. - Return early or throw `FileNotFoundException` early in case `contentResolver.openInputStream()` returns `null` instead of throwing `NullPointerException` later. Change the signature of `Closeable.closeQuietly()` to only accept a non-null `Closeable`. - Reimplement `Uri.copyToFile()` using Okio. This takes advantage of the built-in high-performance buffers of the library so a buffer doesn't need to be allocated or managed manually. The new implementation also makes sure that the input and output streams are always closed, as the original code could in some cases return without properly closing a stream. - Reimplement `ProgressRequestBody` as `Uri.asRequestBody()` (adding to the existing extension functions available in the Okio library to create a `RequestBody`). The new implementation uses Okio's `Buffer` instead of a manually managed byte array, which allows to avoid copying bytes from one buffer to the next. The max number of bytes read at once was increased from 2K to 8K to improve performance. Avoid division by zero in case `contentLength` is `0`. Finally, this implementation now takes a `Uri` as input instead of an `InputStream`, because a `RequestBody` must be replayable in case Okio retries the request, and an `InputStream` can only be used once. --- .../components/compose/ImageDownsizer.kt | 4 +- .../tusky/components/compose/MediaUploader.kt | 19 +++---- .../tusky/network/ProgressRequestBody.kt | 55 ------------------ .../tusky/network/UriRequestBody.kt | 57 +++++++++++++++++++ .../com/keylesspalace/tusky/util/IOUtils.kt | 45 +++++---------- .../keylesspalace/tusky/util/MediaUtils.kt | 2 +- 6 files changed, 80 insertions(+), 102 deletions(-) delete mode 100644 app/src/main/java/com/keylesspalace/tusky/network/ProgressRequestBody.kt create mode 100644 app/src/main/java/com/keylesspalace/tusky/network/UriRequestBody.kt diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/ImageDownsizer.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/ImageDownsizer.kt index c45db990a..fd355ca51 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/ImageDownsizer.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/ImageDownsizer.kt @@ -42,7 +42,7 @@ fun downsizeImage( tempFile: File ): Boolean { val decodeBoundsInputStream = try { - contentResolver.openInputStream(uri) + contentResolver.openInputStream(uri) ?: return false } catch (e: FileNotFoundException) { return false } @@ -66,7 +66,7 @@ fun downsizeImage( return false } val decodeBitmapInputStream = try { - contentResolver.openInputStream(uri) + contentResolver.openInputStream(uri) ?: return false } catch (e: FileNotFoundException) { return false } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaUploader.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaUploader.kt index 03ac83227..b5ad6c622 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaUploader.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/MediaUploader.kt @@ -15,7 +15,6 @@ package com.keylesspalace.tusky.components.compose -import android.annotation.SuppressLint import android.content.ContentResolver import android.content.Context import android.media.MediaMetadataRetriever @@ -31,7 +30,7 @@ import com.keylesspalace.tusky.R import com.keylesspalace.tusky.components.compose.ComposeActivity.QueuedMedia import com.keylesspalace.tusky.components.instanceinfo.InstanceInfo import com.keylesspalace.tusky.network.MediaUploadApi -import com.keylesspalace.tusky.network.ProgressRequestBody +import com.keylesspalace.tusky.network.asRequestBody import com.keylesspalace.tusky.util.MEDIA_SIZE_UNKNOWN import com.keylesspalace.tusky.util.getImageSquarePixels import com.keylesspalace.tusky.util.getMediaSize @@ -41,7 +40,6 @@ import java.io.File import java.io.FileInputStream import java.io.FileOutputStream import java.io.IOException -import java.util.Date import javax.inject.Inject import javax.inject.Singleton import kotlinx.coroutines.CoroutineScope @@ -246,7 +244,6 @@ class MediaUploader @Inject constructor( private val contentResolver = context.contentResolver - @SuppressLint("Recycle") // stream is closed in ProgressRequestBody private suspend fun upload(media: QueuedMedia): Flow { return callbackFlow { var mimeType = contentResolver.getType(media.uri) @@ -265,22 +262,20 @@ class MediaUploader @Inject constructor( } val map = MimeTypeMap.getSingleton() val fileExtension = map.getExtensionFromMimeType(mimeType) - val filename = "%s_%s_%s.%s".format( + val filename = "%s_%d_%s.%s".format( context.getString(R.string.app_name), - Date().time.toString(), + System.currentTimeMillis(), randomAlphanumericString(10), fileExtension ) - val stream = contentResolver.openInputStream(media.uri) - if (mimeType == null) mimeType = "multipart/form-data" var lastProgress = -1 - val fileBody = ProgressRequestBody( - stream!!, - media.mediaSize, - mimeType.toMediaTypeOrNull()!! + val fileBody = media.uri.asRequestBody( + contentResolver, + requireNotNull(mimeType.toMediaTypeOrNull()) { "Invalid Content Type" }, + media.mediaSize ) { percentage -> if (percentage != lastProgress) { trySend(UploadEvent.ProgressEvent(percentage)) diff --git a/app/src/main/java/com/keylesspalace/tusky/network/ProgressRequestBody.kt b/app/src/main/java/com/keylesspalace/tusky/network/ProgressRequestBody.kt deleted file mode 100644 index 3a420139c..000000000 --- a/app/src/main/java/com/keylesspalace/tusky/network/ProgressRequestBody.kt +++ /dev/null @@ -1,55 +0,0 @@ -/* Copyright 2017 Andrew Dawson - * - * This file is a part of Tusky. - * - * This program is free software; you can redistribute it and/or modify it under the terms of the - * GNU General Public License as published by the Free Software Foundation; either version 3 of the - * License, or (at your option) any later version. - * - * Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even - * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General - * Public License for more details. - * - * You should have received a copy of the GNU General Public License along with Tusky; if not, - * see . */ -package com.keylesspalace.tusky.network - -import java.io.IOException -import java.io.InputStream -import okhttp3.MediaType -import okhttp3.RequestBody -import okio.BufferedSink - -class ProgressRequestBody(private val content: InputStream, private val contentLength: Long, private val mediaType: MediaType, private val uploadListener: UploadCallback) : RequestBody() { - fun interface UploadCallback { - fun onProgressUpdate(percentage: Int) - } - - override fun contentType(): MediaType { - return mediaType - } - - override fun contentLength(): Long { - return contentLength - } - - @Throws(IOException::class) - override fun writeTo(sink: BufferedSink) { - val buffer = ByteArray(DEFAULT_BUFFER_SIZE) - var uploaded: Long = 0 - - content.use { content -> - var read: Int - while (content.read(buffer).also { read = it } != -1) { - uploadListener.onProgressUpdate((100 * uploaded / contentLength).toInt()) - uploaded += read.toLong() - sink.write(buffer, 0, read) - } - uploadListener.onProgressUpdate((100 * uploaded / contentLength).toInt()) - } - } - - companion object { - private const val DEFAULT_BUFFER_SIZE = 2048 - } -} diff --git a/app/src/main/java/com/keylesspalace/tusky/network/UriRequestBody.kt b/app/src/main/java/com/keylesspalace/tusky/network/UriRequestBody.kt new file mode 100644 index 000000000..c751f2fc3 --- /dev/null +++ b/app/src/main/java/com/keylesspalace/tusky/network/UriRequestBody.kt @@ -0,0 +1,57 @@ +/* Copyright 2024 Tusky Contributors + * + * This file is a part of Tusky. + * + * This program is free software; you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation; either version 3 of the + * License, or (at your option) any later version. + * + * Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even + * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General + * Public License for more details. + * + * You should have received a copy of the GNU General Public License along with Tusky; if not, + * see . */ +package com.keylesspalace.tusky.network + +import android.content.ContentResolver +import android.net.Uri +import java.io.FileNotFoundException +import okhttp3.MediaType +import okhttp3.RequestBody +import okio.Buffer +import okio.BufferedSink +import okio.source + +private const val DEFAULT_CHUNK_SIZE = 8192L + +fun interface UploadCallback { + fun onProgressUpdate(percentage: Int) +} + +fun Uri.asRequestBody(contentResolver: ContentResolver, contentType: MediaType? = null, contentLength: Long = -1L, uploadListener: UploadCallback? = null): RequestBody { + return object : RequestBody() { + override fun contentType(): MediaType? = contentType + + override fun contentLength(): Long = contentLength + + override fun writeTo(sink: BufferedSink) { + val buffer = Buffer() + var uploaded: Long = 0 + val inputStream = contentResolver.openInputStream(this@asRequestBody) ?: throw FileNotFoundException("Unavailable ContentProvider") + + inputStream.source().use { source -> + while (true) { + val read = source.read(buffer, DEFAULT_CHUNK_SIZE) + if (read == -1L) { + break + } + sink.write(buffer, read) + uploaded += read + uploadListener?.let { if (contentLength > 0L) it.onProgressUpdate((100L * uploaded / contentLength).toInt()) } + } + uploadListener?.onProgressUpdate(100) + } + } + } +} diff --git a/app/src/main/java/com/keylesspalace/tusky/util/IOUtils.kt b/app/src/main/java/com/keylesspalace/tusky/util/IOUtils.kt index a955af255..f17e970ff 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/IOUtils.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/IOUtils.kt @@ -15,52 +15,33 @@ package com.keylesspalace.tusky.util -import android.annotation.SuppressLint import android.content.ContentResolver import android.net.Uri import java.io.Closeable import java.io.File -import java.io.FileNotFoundException -import java.io.FileOutputStream import java.io.IOException -import java.io.InputStream +import okio.buffer +import okio.sink +import okio.source -private const val DEFAULT_BLOCKSIZE = 16384 - -fun Closeable?.closeQuietly() { +fun Closeable.closeQuietly() { try { - this?.close() + close() } catch (e: IOException) { // intentionally unhandled } } -@SuppressLint("Recycle") // The linter can't tell that the stream gets closed by a helper method fun Uri.copyToFile(contentResolver: ContentResolver, file: File): Boolean { - val from: InputStream? - val to: FileOutputStream - - try { - from = contentResolver.openInputStream(this) - to = FileOutputStream(file) - } catch (e: FileNotFoundException) { - return false - } - - if (from == null) return false - - val chunk = ByteArray(DEFAULT_BLOCKSIZE) - try { - while (true) { - val bytes = from.read(chunk, 0, chunk.size) - if (bytes < 0) break - to.write(chunk, 0, bytes) + return try { + val inputStream = contentResolver.openInputStream(this) ?: return false + inputStream.source().use { source -> + file.sink().buffer().use { bufferedSink -> + bufferedSink.writeAll(source) + } } + true } catch (e: IOException) { - return false + false } - - from.closeQuietly() - to.closeQuietly() - return true } diff --git a/app/src/main/java/com/keylesspalace/tusky/util/MediaUtils.kt b/app/src/main/java/com/keylesspalace/tusky/util/MediaUtils.kt index c9c0a8b9f..ed4e71811 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/MediaUtils.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/MediaUtils.kt @@ -69,7 +69,7 @@ fun getMediaSize(contentResolver: ContentResolver, uri: Uri?): Long { @Throws(FileNotFoundException::class) fun getImageSquarePixels(contentResolver: ContentResolver, uri: Uri): Long { - val input = contentResolver.openInputStream(uri) + val input = contentResolver.openInputStream(uri) ?: throw FileNotFoundException("Unavailable ContentProvider") val options = BitmapFactory.Options() options.inJustDecodeBounds = true