From 103ba463c38ec5532f8ebb28d09897db237d2f69 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 16 Mar 2021 11:34:27 +0100 Subject: [PATCH] Create getBetsChunkSize to avoid code duplication --- .../crypto/tasks/DownloadKeysForUsersTask.kt | 9 +-- .../internal/session/sync/RoomSyncHandler.kt | 18 ++--- .../android/sdk/internal/util/MathUtils.kt | 43 +++++++++++ .../android/sdk/internal/util/MathUtilTest.kt | 73 +++++++++++++++++++ 4 files changed, 129 insertions(+), 14 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/MathUtils.kt create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/MathUtilTest.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/DownloadKeysForUsersTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/DownloadKeysForUsersTask.kt index c78a5cb74c..3f1f5299d0 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/DownloadKeysForUsersTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/DownloadKeysForUsersTask.kt @@ -29,12 +29,12 @@ import org.matrix.android.sdk.internal.crypto.model.rest.RestKeyInfo import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.task.Task +import org.matrix.android.sdk.internal.util.getBetsChunkSize import javax.inject.Inject -import kotlin.math.ceil internal interface DownloadKeysForUsersTask : Task { data class Params( - // the list of users to get keys for. + // the list of users to get keys for. The list MUST NOT be empty val userIds: List, // the up-to token val token: String? @@ -47,8 +47,7 @@ internal class DefaultDownloadKeysForUsers @Inject constructor( ) : DownloadKeysForUsersTask { override suspend fun execute(params: DownloadKeysForUsersTask.Params): KeysQueryResponse { - val numberOfChunks = ceil(params.userIds.size / LIMIT.toDouble()).toInt().coerceAtLeast(1) - val chunkSize = params.userIds.size / numberOfChunks + val bestChunkSize = getBetsChunkSize(params.userIds.size, LIMIT) // Store server results in these mutable maps val deviceKeys = mutableMapOf>() @@ -62,7 +61,7 @@ internal class DefaultDownloadKeysForUsers @Inject constructor( // Split network request into smaller request (#2925) coroutineScope { params.userIds - .chunked(chunkSize) + .chunked(bestChunkSize.chunkSize) .map { KeysQueryBody( deviceKeys = it.associateWith { emptyList() }, diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt index 0f97d0cb39..4553ff90ed 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt @@ -64,9 +64,9 @@ import org.matrix.android.sdk.internal.session.sync.model.LazyRoomSyncEphemeral import org.matrix.android.sdk.internal.session.sync.model.RoomSync import org.matrix.android.sdk.internal.session.sync.model.RoomSyncAccountData import org.matrix.android.sdk.internal.session.sync.model.RoomsSyncResponse +import org.matrix.android.sdk.internal.util.getBetsChunkSize import timber.log.Timber import javax.inject.Inject -import kotlin.math.ceil internal class RoomSyncHandler @Inject constructor(private val readReceiptHandler: ReadReceiptHandler, private val roomSummaryUpdater: RoomSummaryUpdater, @@ -140,17 +140,17 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle syncLocalTimeStampMillis: Long, aggregator: SyncResponsePostTreatmentAggregator, reporter: ProgressReporter?) { - val maxSize = (initialSyncStrategy as? InitialSyncStrategy.Optimized)?.maxRoomsToInsert ?: Int.MAX_VALUE - val listSize = handlingStrategy.data.keys.size - val numberOfChunks = ceil(listSize / maxSize.toDouble()).toInt() + val bestChunkSize = getBetsChunkSize( + listSize = handlingStrategy.data.keys.size, + limit = (initialSyncStrategy as? InitialSyncStrategy.Optimized)?.maxRoomsToInsert ?: Int.MAX_VALUE + ) - if (numberOfChunks > 1) { - reportSubtask(reporter, InitSyncStep.ImportingAccountJoinedRooms, numberOfChunks, 0.6f) { - val chunkSize = listSize / numberOfChunks - Timber.d("INIT_SYNC $listSize rooms to insert, split into $numberOfChunks sublists of $chunkSize items") + if (bestChunkSize.shouldSplit()) { + reportSubtask(reporter, InitSyncStep.ImportingAccountJoinedRooms, bestChunkSize.numberOfChunks, 0.6f) { + Timber.d("INIT_SYNC ${handlingStrategy.data.keys.size} rooms to insert, split with $bestChunkSize") // I cannot find a better way to chunk a map, so chunk the keys and then create new maps handlingStrategy.data.keys - .chunked(chunkSize) + .chunked(bestChunkSize.chunkSize) .forEachIndexed { index, roomIds -> val roomEntities = roomIds .also { Timber.d("INIT_SYNC insert ${roomIds.size} rooms") } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/MathUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/MathUtils.kt new file mode 100644 index 0000000000..6abf917ab0 --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/MathUtils.kt @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2021 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.internal.util + +import kotlin.math.ceil + +internal data class BestChunkSize( + val numberOfChunks: Int, + val chunkSize: Int +) { + fun shouldSplit() = numberOfChunks > 1 +} + +internal fun getBetsChunkSize(listSize: Int, limit: Int): BestChunkSize { + return if (listSize <= limit) { + BestChunkSize( + numberOfChunks = 1, + chunkSize = listSize + ) + } else { + val numberOfChunks = ceil(listSize / limit.toDouble()).toInt() + val chunkSize = listSize / numberOfChunks + + BestChunkSize( + numberOfChunks = numberOfChunks, + chunkSize = chunkSize + ) + } +} diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/MathUtilTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/MathUtilTest.kt new file mode 100644 index 0000000000..7cb355a621 --- /dev/null +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/util/MathUtilTest.kt @@ -0,0 +1,73 @@ +/* + * Copyright 2020 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.internal.util + +import org.amshove.kluent.shouldBeEqualTo +import org.amshove.kluent.shouldHaveSize +import org.junit.FixMethodOrder +import org.junit.Test +import org.junit.runners.MethodSorters +import org.matrix.android.sdk.MatrixTest + +@FixMethodOrder(MethodSorters.JVM) +class MathUtilTest : MatrixTest { + + @Test + fun testGetBestChunkSize0() = doTest(0, 100, 1, 0) + + @Test + fun testGetBestChunkSize1() = doTest(1, 100, 1, 1) + + @Test + fun testGetBestChunkSize5() = doTest(5, 100, 1, 5) + + @Test + fun testGetBestChunkSize99() = doTest(99, 100, 1, 99) + + @Test + fun testGetBestChunkSize100() = doTest(100, 100, 1, 100) + + @Test + fun testGetBestChunkSize101() = doTest(101, 100, 2, 50) + + @Test + fun testGetBestChunkSize199() = doTest(199, 100, 2, 99) + + @Test + fun testGetBestChunkSize200() = doTest(200, 100, 2, 100) + + @Test + fun testGetBestChunkSize201() = doTest(201, 100, 3, 67) + + @Test + fun testGetBestChunkSize240() = doTest(240, 100, 3, 80) + + private fun doTest(listSize: Int, limit: Int, expectedNumberOfChunks: Int, expectedChunkSize: Int) { + val result = getBetsChunkSize(listSize, limit) + + result.numberOfChunks shouldBeEqualTo expectedNumberOfChunks + result.chunkSize shouldBeEqualTo expectedChunkSize + + // Test that the result make sense, when we use chunked() + if (result.chunkSize > 0) { + generateSequence { "a" } + .take(listSize) + .chunked(result.chunkSize) + .shouldHaveSize(result.numberOfChunks) + } + } +}