From b37a2ce21a6330e2616d072f555d43f5aef3d45a Mon Sep 17 00:00:00 2001 From: matt-ramotar Date: Sun, 21 Jul 2024 10:10:24 -0400 Subject: [PATCH] Refactor from ConcurrentOperationManager to OperationPipeline Signed-off-by: matt-ramotar --- .../storex/paging/runtime/OperationManager.kt | 10 -- .../storex/paging/runtime/PagingScope.kt | 1 - .../pager/impl/ConcurrentOperationApplier.kt | 8 +- .../pager/impl/ConcurrentOperationManager.kt | 112 ------------------ .../runtime/internal/pager/impl/RealPager.kt | 8 +- .../pagingScope/impl/PagingScopeBuilder.kt | 9 +- .../pagingScope/impl/RealPagingScope.kt | 6 - 7 files changed, 11 insertions(+), 143 deletions(-) delete mode 100644 paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/OperationManager.kt delete mode 100644 paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/ConcurrentOperationManager.kt diff --git a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/OperationManager.kt b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/OperationManager.kt deleted file mode 100644 index df73f4f..0000000 --- a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/OperationManager.kt +++ /dev/null @@ -1,10 +0,0 @@ -package org.mobilenativefoundation.storex.paging.runtime - -interface OperationManager { - suspend fun add(operation: Operation) - suspend fun remove(operation: Operation) - suspend fun removeAll(predicate: (Operation) -> Boolean) - suspend fun clear() - fun get(): List> - fun get(predicate: (Operation) -> Boolean): List> -} \ No newline at end of file diff --git a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/PagingScope.kt b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/PagingScope.kt index 8bfd083..fe7aae3 100644 --- a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/PagingScope.kt +++ b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/PagingScope.kt @@ -12,7 +12,6 @@ import org.mobilenativefoundation.storex.paging.runtime.internal.pagingScope.imp interface PagingScope { fun getPager(): Pager - fun getOperationManager(): OperationManager fun getDispatcher(): Dispatcher fun getUpdatingItemProvider(): UpdatingItemProvider diff --git a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/ConcurrentOperationApplier.kt b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/ConcurrentOperationApplier.kt index 41d47ca..5cf7065 100644 --- a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/ConcurrentOperationApplier.kt +++ b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/ConcurrentOperationApplier.kt @@ -5,9 +5,8 @@ import kotlinx.coroutines.sync.withLock import org.mobilenativefoundation.storex.paging.runtime.FetchingState import org.mobilenativefoundation.storex.paging.runtime.ItemSnapshotList import org.mobilenativefoundation.storex.paging.runtime.Operation -import org.mobilenativefoundation.storex.paging.runtime.OperationManager +import org.mobilenativefoundation.storex.paging.runtime.OperationPipeline import org.mobilenativefoundation.storex.paging.runtime.PagingState -import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.MutableOperationPipeline import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.OperationApplier /** @@ -21,8 +20,7 @@ import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.Opera * @param ItemValue The type of the item value. */ class ConcurrentOperationApplier( - private val operationManager: OperationManager, - private val mutableOperationPipeline: MutableOperationPipeline + private val operationPipeline: OperationPipeline ) : OperationApplier { // Mutex for ensuring thread-safe access to shared resources @@ -49,7 +47,7 @@ class ConcurrentOperationApplier, fetchingState: FetchingState ): ItemSnapshotList = mutex.withLock { - operationManager.get().fold(snapshot) { acc, operation -> + operationPipeline.fold(snapshot) { acc, operation -> if (operation.shouldApply(key, pagingState, fetchingState)) { val cacheKey = CacheKey(operation, acc, key, pagingState, fetchingState) operationCache.getOrPut(cacheKey) { diff --git a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/ConcurrentOperationManager.kt b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/ConcurrentOperationManager.kt deleted file mode 100644 index 002b7d3..0000000 --- a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/ConcurrentOperationManager.kt +++ /dev/null @@ -1,112 +0,0 @@ -package org.mobilenativefoundation.storex.paging.runtime.internal.pager.impl - -import kotlinx.atomicfu.atomic -import kotlinx.atomicfu.updateAndGet -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock -import org.mobilenativefoundation.storex.paging.runtime.Operation -import org.mobilenativefoundation.storex.paging.runtime.OperationManager - -/** - * This file contains a thread-safe implementation of OperationManager for Kotlin Multiplatform. - * - * Key design decisions: - * 1. Use of atomicfu for lock-free reads and atomic updates. - * 2. Mutex for synchronizing write operations. - * 3. Immutable list snapshots for consistent reads without locking. - * 4. Suspension on write operations to work well with coroutines. - * - * Trade-offs: - * - Reads are very fast and non-blocking, but they may not always reflect the absolute latest state. - * - Writes are serialized and may be slower, but they ensure consistency. - * - * Note: This implementation requires the kotlinx-atomicfu dependency. - */ - -/** - * A thread-safe implementation of OperationManager that allows for non-blocking reads - * and synchronized writes. - * - * @param ItemId The type of the item identifier. - * @param PageRequestKey The type of the paging key. - * @param ItemValue The type of the item value. - */ -class ConcurrentOperationManager : - OperationManager { - - // Atomic reference to the list of operations, allowing for lock-free reads - private val operationsAtomic = atomic(listOf>()) - - // Mutex for synchronizing write operations - private val writeMutex = Mutex() - - /** - * Adds an operation to the manager if it's not already present. - * - * @param operation The operation to add. - */ - override suspend fun add(operation: Operation) { - writeMutex.withLock { - // Atomic update ensures that the change is visible to all threads immediately - operationsAtomic.updateAndGet { current -> - if (operation !in current) current + operation else current - } - } - } - - /** - * Removes a specific operation from the manager. - * - * @param operation The operation to remove. - */ - override suspend fun remove(operation: Operation) { - writeMutex.withLock { - operationsAtomic.updateAndGet { it - operation } - } - } - - /** - * Removes all operations that match the given predicate. - * - * @param predicate A function that determines which operations to remove. - */ - override suspend fun removeAll(predicate: (Operation) -> Boolean) { - writeMutex.withLock { - operationsAtomic.updateAndGet { it.filterNot(predicate) } - } - } - - /** - * Clears all operations from the manager. - */ - override suspend fun clear() { - writeMutex.withLock { - operationsAtomic.value = emptyList() - } - } - - /** - * Retrieves a snapshot of all current operations. - * - * This is a non-blocking operation and returns an immutable list. - * - * @return A list of all current operations. - */ - override fun get(): List> { - // Lock-free read of the current state - return operationsAtomic.value - } - - /** - * Retrieves a filtered snapshot of operations that match the given predicate. - * - * This is a non-blocking operation and returns an immutable list. - * - * @param predicate A function that determines which operations to include. - * @return A filtered list of operations. - */ - override fun get(predicate: (Operation) -> Boolean): List> { - // Lock-free read and filter - return operationsAtomic.value.filter(predicate) - } -} \ No newline at end of file diff --git a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/RealPager.kt b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/RealPager.kt index 4355f64..f13fe99 100644 --- a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/RealPager.kt +++ b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pager/impl/RealPager.kt @@ -24,6 +24,7 @@ import org.mobilenativefoundation.storex.paging.runtime.internal.logger.api.Pagi import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.FetchingStateHolder import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.LoadParamsQueue import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.LoadingHandler +import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.MutableOperationPipeline import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.PagingStateManager import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.QueueManager import org.mobilenativefoundation.storex.paging.runtime.internal.store.api.NormalizedStore @@ -46,6 +47,7 @@ internal class RealPager( private val queueManager: QueueManager, private val loadingHandler: LoadingHandler, private val coroutineScope: CoroutineScope, + private val mutableOperationPipeline: MutableOperationPipeline ) : Pager { init { @@ -143,9 +145,9 @@ internal class RealPager( is Action.Enqueue -> handleEnqueueAction(action) Action.Invalidate -> handleInvalidateAction() - is Action.AddOperation -> TODO() - Action.ClearOperations -> TODO() - is Action.RemoveOperation -> TODO() + is Action.AddOperation -> mutableOperationPipeline.add(action.operation) + Action.ClearOperations -> mutableOperationPipeline.clear() + is Action.RemoveOperation -> mutableOperationPipeline.remove(action.operation) } } } diff --git a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pagingScope/impl/PagingScopeBuilder.kt b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pagingScope/impl/PagingScopeBuilder.kt index 3d4e49e..eeba50a 100644 --- a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pagingScope/impl/PagingScopeBuilder.kt +++ b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pagingScope/impl/PagingScopeBuilder.kt @@ -32,13 +32,11 @@ import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.Fetch import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.LinkedHashMapManager import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.ListSortAnalyzer import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.LoadingHandler -import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.MutableOperationPipeline import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.OperationApplier import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.PagingStateManager import org.mobilenativefoundation.storex.paging.runtime.internal.pager.api.QueueManager import org.mobilenativefoundation.storex.paging.runtime.internal.pager.impl.ConcurrentFetchingStateHolder import org.mobilenativefoundation.storex.paging.runtime.internal.pager.impl.ConcurrentOperationApplier -import org.mobilenativefoundation.storex.paging.runtime.internal.pager.impl.ConcurrentOperationManager import org.mobilenativefoundation.storex.paging.runtime.internal.pager.impl.DefaultFetchingStrategy import org.mobilenativefoundation.storex.paging.runtime.internal.pager.impl.DefaultListSortAnalyzer import org.mobilenativefoundation.storex.paging.runtime.internal.pager.impl.RealLinkedHashMapManager @@ -136,7 +134,6 @@ class PagingScopeBuilder( } override fun build(): PagingScope { - val operationManager = ConcurrentOperationManager() val fetchingStateHolder = ConcurrentFetchingStateHolder(initialFetchingState, itemIdComparator, pageRequestKeyComparator) val dataPersistence = RealDataPersistence(itemPersistence, pagePersistence) @@ -162,7 +159,7 @@ class PagingScopeBuilder( val pagingStateManager = RealPagingStateManager(initialState, logger) val queueManager = RealQueueManager(logger, pageRequestKeyComparator) val mutableOperationPipeline = RealMutableOperationPipeline(initialOperations) - val operationApplier = ConcurrentOperationApplier(operationManager, mutableOperationPipeline) + val operationApplier = ConcurrentOperationApplier(mutableOperationPipeline) val loadingHandler = createLoadingHandler( store = store, pagingStateManager = pagingStateManager, @@ -184,12 +181,12 @@ class PagingScopeBuilder( pagingStateManager = pagingStateManager, queueManager = queueManager, loadingHandler = loadingHandler, - coroutineScope = coroutineScope + coroutineScope = coroutineScope, + mutableOperationPipeline = mutableOperationPipeline ) return RealPagingScope( pager = pager, - operationManager = operationManager, dispatcher = dispatcher, updatingItemProvider = updatingItemProvider ) diff --git a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pagingScope/impl/RealPagingScope.kt b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pagingScope/impl/RealPagingScope.kt index 6525b0e..27ba8c7 100644 --- a/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pagingScope/impl/RealPagingScope.kt +++ b/paging-runtime/src/commonMain/kotlin/org/mobilenativefoundation/storex/paging/runtime/internal/pagingScope/impl/RealPagingScope.kt @@ -1,14 +1,12 @@ package org.mobilenativefoundation.storex.paging.runtime.internal.pagingScope.impl import org.mobilenativefoundation.storex.paging.runtime.Dispatcher -import org.mobilenativefoundation.storex.paging.runtime.OperationManager import org.mobilenativefoundation.storex.paging.runtime.Pager import org.mobilenativefoundation.storex.paging.runtime.PagingScope import org.mobilenativefoundation.storex.paging.runtime.UpdatingItemProvider class RealPagingScope( private val pager: Pager, - private val operationManager: OperationManager, private val dispatcher: Dispatcher, private val updatingItemProvider: UpdatingItemProvider ) : PagingScope { @@ -16,10 +14,6 @@ class RealPagingScope( return pager } - override fun getOperationManager(): OperationManager { - return operationManager - } - override fun getDispatcher(): Dispatcher { return dispatcher }