From fbed5c7d6790dfe0c3527f0f2a32c1c4848e0a28 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Tue, 30 Jan 2024 11:55:59 +0100 Subject: [PATCH] WebDAV performance optimizations (#543) * Make ranged GET requests cancellable; reduce notification update frequency * Include original exception as a cause in WebDAV ErrnoException * Add KDoc for threading --- .../bitfire/davdroid/webdav/PagingReader.kt | 6 + .../davdroid/webdav/RandomAccessCallback.kt | 108 +++++++++++------- 2 files changed, 75 insertions(+), 39 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/PagingReader.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/PagingReader.kt index a8734450..0924cfd8 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/PagingReader.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/PagingReader.kt @@ -40,6 +40,9 @@ class PagingReader( * - a file seek + read or * - a ranged WebDAV request. * + * This function will not be called by multiple threads at the same time, so + * thread-safety is not required. + * * @param offset position to start * @param size number of bytes to load * @@ -97,6 +100,9 @@ class PagingReader( * This method will determine the page that contains [position] and read only * from this page. * + * This method is synchronized so that no concurrent modifications of [currentPage] + * and no concurrent calls to [loader] will be made. + * * @param position starting position * @param size number of bytes requested * @param dst destination where data are read into diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallback.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallback.kt index a703c910..143a4bde 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallback.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallback.kt @@ -26,6 +26,12 @@ import at.bitfire.davdroid.ui.NotificationUtils.notifyIfPossible import at.bitfire.davdroid.util.DavUtils import at.bitfire.davdroid.webdav.RandomAccessCallback.Wrapper.Companion.TIMEOUT_INTERVAL import at.bitfire.davdroid.webdav.cache.PageCacheBuilder +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.runInterruptible import okhttp3.Headers import okhttp3.HttpUrl import okhttp3.MediaType @@ -77,6 +83,16 @@ class RandomAccessCallback private constructor( private val pagingReader = PagingReader(fileSize, PageCacheBuilder.MAX_PAGE_SIZE, this) private val pageCache = PageCacheBuilder.getInstance() + private var loadPageJobs: Set> = emptySet() + + init { + cancellationSignal?.let { + Logger.log.info("Cancelling random access to $url") + for (job in loadPageJobs) + job.cancel() + } + } + override fun onFsync() { /* not used */ } @@ -90,17 +106,6 @@ class RandomAccessCallback private constructor( Logger.log.fine("onRead $url $offset $size") throwIfCancelled("onRead") - val progress = - if (fileSize == 0L) // avoid division by zero - 100 - else - (offset*100/fileSize).toInt() - notificationManager.notifyIfPossible( - notificationTag, - NotificationUtils.NOTIFY_WEBDAV_ACCESS, - notification.setProgress(100, progress, false).build() - ) - try { return pagingReader.read(offset, size, data) } catch (e: Exception) { @@ -123,30 +128,58 @@ class RandomAccessCallback private constructor( override fun loadPage(offset: Long, size: Int): ByteArray { Logger.log.fine("Loading page $url $offset/$size") - return pageCache.getOrPut(PageCacheBuilder.PageIdentifier(url, offset, size)) { - val ifMatch: Headers = - documentState.eTag?.let { eTag -> - Headers.headersOf("If-Match", "\"$eTag\"") - } ?: - documentState.lastModified?.let { lastModified -> - Headers.headersOf("If-Unmodified-Since", HttpUtils.formatDate(lastModified)) - } ?: throw IllegalStateException("ETag/Last-Modified required for random access") - var result: ByteArray? = null - dav.getRange( - DavUtils.acceptAnything(preferred = mimeType), - offset, - size, - ifMatch - ) { response -> - if (response.code == 200) // server doesn't support ranged requests - throw PartialContentNotSupportedException() - else if (response.code != 206) - throw HttpException(response) + // update notification + val progress = + if (fileSize == 0L) // avoid division by zero + 100 + else + (offset * 100 / fileSize).toInt() + notificationManager.notifyIfPossible( + notificationTag, + NotificationUtils.NOTIFY_WEBDAV_ACCESS, + notification.setProgress(100, progress, false).build() + ) - result = response.body?.bytes() + // create async job that can be cancelled (and cancellation interrupts I/O) + val job = CoroutineScope(Dispatchers.IO).async { + runInterruptible { + pageCache.getOrPut(PageCacheBuilder.PageIdentifier(url, offset, size)) { + val ifMatch: Headers = + documentState.eTag?.let { eTag -> + Headers.headersOf("If-Match", "\"$eTag\"") + } ?: documentState.lastModified?.let { lastModified -> + Headers.headersOf("If-Unmodified-Since", HttpUtils.formatDate(lastModified)) + } ?: throw IllegalStateException("ETag/Last-Modified required for random access") + + var result: ByteArray? = null + dav.getRange( + DavUtils.acceptAnything(preferred = mimeType), + offset, + size, + ifMatch + ) { response -> + if (response.code == 200) // server doesn't support ranged requests + throw PartialContentNotSupportedException() + else if (response.code != 206) + throw HttpException(response) + + result = response.body?.bytes() + } + return@getOrPut result ?: throw DavException("No response body") + } } - return@getOrPut result ?: throw DavException("No response body") + } + + try { + loadPageJobs += job + + // wait for result + return runBlocking { + job.await() + } + } finally { + loadPageJobs -= job } } @@ -158,7 +191,8 @@ class RandomAccessCallback private constructor( } private fun Exception.toErrNoException(functionName: String) = - ErrnoException(functionName, + ErrnoException( + functionName, when (this) { is HttpException -> when (code) { @@ -170,15 +204,11 @@ class RandomAccessCallback private constructor( is InterruptedIOException -> OsConstants.EINTR is PartialContentNotSupportedException -> OsConstants.EOPNOTSUPP else -> OsConstants.EIO - } + }, + this ) - data class DocumentKey( - val resource: HttpUrl, - val state: DocumentState - ) - class PartialContentNotSupportedException: Exception()