Optimize entity form load times

.circleci/config.yml

@@ -336,7 +336,7 @@ jobs:
               --device model=MediumPhone.arm,version=34,locale=en,orientation=portrait \
               --results-bucket opendatakit-collect-test-results \
               --directories-to-pull /sdcard --timeout 20m \
-              --test-targets "notPackage org.odk.collect.android.regression"
+              --test-targets "notPackage org.odk.collect.android.regression" "notPackage org.odk.collect.android.benchmark"
             fi
           no_output_timeout: 25m
 

README.md

@@ -262,6 +262,7 @@ Maintainers keep a folder with a clean checkout of the code and use [jenv.be](ht
 - make sure CI is green for the chosen commit
 - run `./gradlew releaseCheck`. If successful, a signed release will be at `collect_app/build/outputs/apk` (with an old version name)
 - verify a basic "happy path": scan a QR code to configure a new project, get a blank form, fill it, open the form map (confirms that the Google Maps key is correct), send form
+- run `./benchmark.sh` with a real device connected to verify performance
 - verify new APK can be installed as update to previous version and that above "happy path" works in that case also
 - create and publish scheduled forum post with release description
 - write Play Store release notes, include link to forum post

benchmark.sh

@@ -0,0 +1 @@
+./gradlew collect_app:connectedAndroidTest -Pandroid.testInstrumentationRunnerArguments.package=org.odk.collect.android.benchmark

build.gradle

@@ -106,7 +106,8 @@ tasks.register('testLab') {
                     '--device', 'model=MediumPhone.arm,version=34,locale=en,orientation=portrait',
                     '--timeout', '10m',
                     '--directories-to-pull', '/sdcard',
-                    '--test-targets', "notPackage org.odk.collect.android.regression"
+                    '--test-targets', "notPackage org.odk.collect.android.regression",
+                    '--test-targets', "notPackage org.odk.collect.android.benchmark"
             )
         }
     }

collect_app/build.gradle

@@ -64,6 +64,7 @@ def getVersionName = { ->
 def secrets = getSecrets()
 def googleMapsApiKey = secrets.getProperty('GOOGLE_MAPS_API_KEY', '')
 def mapboxAccessToken = secrets.getProperty('MAPBOX_ACCESS_TOKEN', '')
+def entitiesFilterTestProjectUrl = secrets.getProperty('ENTITIES_FILTER_TEST_PROJECT_URL', '')
 
 android {
     compileSdk Versions.android_compile_sdk
@@ -144,6 +145,7 @@ android {
             debuggable(true)
             resValue("string", "GOOGLE_MAPS_API_KEY", googleMapsApiKey)
             resValue("string", "mapbox_access_token", mapboxAccessToken)
+            buildConfigField("String", "ENTITIES_FILTER_TEST_PROJECT_URL", "\"$entitiesFilterTestProjectUrl\"")
         }
     }
 

collect_app/src/androidTest/java/org/odk/collect/android/benchmark/EntitiesBenchmarkTest.kt

@@ -0,0 +1,163 @@
+package org.odk.collect.android.benchmark
+
+import android.app.Application
+import androidx.test.core.app.ApplicationProvider
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import org.hamcrest.MatcherAssert.assertThat
+import org.hamcrest.Matchers.blankOrNullString
+import org.hamcrest.Matchers.lessThan
+import org.hamcrest.Matchers.not
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.RuleChain
+import org.junit.runner.RunWith
+import org.odk.collect.android.support.TestDependencies
+import org.odk.collect.android.support.pages.MainMenuPage
+import org.odk.collect.android.support.pages.Page
+import org.odk.collect.android.support.rules.CollectTestRule
+import org.odk.collect.android.support.rules.TestRuleChain.chain
+import org.odk.collect.android.test.BuildConfig.ENTITIES_FILTER_TEST_PROJECT_URL
+import org.odk.collect.shared.TimeInMs
+import org.odk.collect.strings.R
+
+/**
+ * Benchmarks the performance of entity follow up forms. [PROJECT_URL] should be set to a project
+ * that contains the "100k Entities Filter" form.
+ *
+ * Devices that currently pass:
+ * - Pixel 4a
+ * - Fairphone 3
+ *
+ */
+
+@RunWith(AndroidJUnit4::class)
+class EntitiesBenchmarkTest {
+
+    private val rule = CollectTestRule(useDemoProject = false)
+
+    @get:Rule
+    var chain: RuleChain = chain(TestDependencies(true)).around(rule)
+
+    @Test
+    fun run() {
+        assertThat(
+            "Need to set ENTITIES_FILTER_TEST_PROJECT_URL before running!",
+            ENTITIES_FILTER_TEST_PROJECT_URL,
+            not(blankOrNullString())
+        )
+        clearAndroidCache()
+
+        val benchmarker = Benchmarker()
+
+        rule.startAtFirstLaunch()
+            .clickManuallyEnterProjectDetails()
+            .inputUrl(ENTITIES_FILTER_TEST_PROJECT_URL)
+            .addProject()
+
+            // Populate http cache and clear out form/entities
+            .clickGetBlankForm()
+            .clickGetSelected()
+            .clickOK(MainMenuPage())
+            .openProjectSettingsDialog()
+            .clickSettings()
+            .clickProjectManagement()
+            .clickOnResetProject()
+            .clickOnString(R.string.reset_blank_forms)
+            .clickOnString(R.string.reset_saved_forms)
+            .clickOnString(R.string.reset_settings_button_reset)
+            .clickOKOnDialog(MainMenuPage())
+
+            .clickGetBlankForm()
+            .benchmark("Downloading form with http cache", 75, benchmarker) {
+                it.clickGetSelected()
+            }
+
+            .clickOK(MainMenuPage())
+            .clickGetBlankForm()
+            .benchmark("Downloading form second time with http cache", 90, benchmarker) {
+                it.clickGetSelected()
+            }
+
+            .clickOK(MainMenuPage())
+            .clickFillBlankForm()
+            .benchmark("Loading form first time", 5, benchmarker) {
+                it.clickOnForm("100k Entities Filter")
+            }
+
+            .pressBackAndDiscardForm()
+            .clickFillBlankForm()
+            .benchmark("Loading form second time", 5, benchmarker) {
+                it.clickOnForm("100k Entities Filter")
+            }
+
+            .answerQuestion("Which value do you want to filter by?", "1024")
+            .benchmark("Filtering select", 5, benchmarker) {
+                it.swipeToNextQuestion("Filtered select")
+            }
+
+        benchmarker.assertResults()
+    }
+
+    private fun clearAndroidCache() {
+        val application = ApplicationProvider.getApplicationContext<Application>()
+        application.cacheDir.deleteRecursively()
+        application.cacheDir.mkdir()
+    }
+}
+
+private class Stopwatch {
+
+    private val times = mutableMapOf<String, Long>()
+
+    fun <T> time(name: String, action: () -> T): T {
+        val startTime = System.currentTimeMillis()
+        val result = action()
+        val endTime = System.currentTimeMillis()
+
+        times[name] = (endTime - startTime) / TimeInMs.ONE_SECOND
+        return result
+    }
+
+    fun getTime(name: String): Long {
+        return times[name]!!
+    }
+}
+
+private fun <T : Page<T>, Y : Page<Y>> Y.benchmark(
+    name: String,
+    target: Long,
+    benchmarker: Benchmarker,
+    action: (Y) -> T
+): T {
+    return benchmarker.benchmark(name, target) {
+        action(this)
+    }
+}
+
+private class Benchmarker {
+    private val stopwatch = Stopwatch()
+    private val targets = mutableMapOf<String, Long>()
+
+    fun <T> benchmark(name: String, target: Long, action: () -> T): T {
+        targets[name] = target
+        return stopwatch.time(name) {
+            action()
+        }
+    }
+
+    fun assertResults() {
+        printResults()
+
+        targets.entries.forEach {
+            val time = stopwatch.getTime(it.key)
+            assertThat("\"${it.key}\" took ${time}s!", time, lessThan(it.value))
+        }
+    }
+
+    private fun printResults() {
+        println("Benchmark results:")
+        targets.keys.forEach {
+            println("$it: ${stopwatch.getTime(it)}s")
+        }
+    }
+}

collect_app/src/androidTest/java/org/odk/collect/android/support/TestDependencies.java

@@ -22,10 +22,23 @@ public class TestDependencies extends AppDependencyModule {
     public final TestScheduler scheduler = new TestScheduler(networkStateProvider);
     public final StoragePathProvider storagePathProvider = new StoragePathProvider();
     public final StubBarcodeViewDecoder stubBarcodeViewDecoder = new StubBarcodeViewDecoder();
+    private final boolean useRealServer;
+
+    public TestDependencies() {
+        this(false);
+    }
+
+    public TestDependencies(boolean useRealServer) {
+        this.useRealServer = useRealServer;
+    }
 
     @Override
     public OpenRosaHttpInterface provideHttpInterface(MimeTypeMap mimeTypeMap, UserAgentProvider userAgentProvider, Application application, VersionInformation versionInformation) {
-        return server;
+        if (useRealServer) {
+            return super.provideHttpInterface(mimeTypeMap, userAgentProvider, application, versionInformation);
+        } else {
+            return server;
+        }
     }
 
     @Override

collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt

@@ -138,14 +138,29 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
             return emptyList()
         }
 
-        return queryWithAttachedRowId(list)
-            .foldAndClose { cursor ->
-                mapCursorRowToEntity(
-                    list,
-                    cursor,
-                    cursor.getInt(ROW_ID)
-                )
-            }
+        return queryWithAttachedRowId(list).foldAndClose {
+            mapCursorRowToEntity(
+                list,
+                it,
+                it.getInt(ROW_ID)
+            )
+        }
+    }
+
+    override fun getCount(list: String): Int {
+        if (!listExists(list)) {
+            return 0
+        }
+
+        return databaseConnection.readableDatabase.rawQuery(
+            """
+            SELECT COUNT(*)
+            FROM $list
+            """.trimIndent(),
+            null
+        ).first {
+            it.getInt(0)
+        }!!
     }
 
     override fun clear() {
@@ -207,6 +222,24 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
         }
     }
 
+    override fun getByIndex(list: String, index: Int): Entity.Saved? {
+        if (!listExists(list)) {
+            return null
+        }
+
+        return databaseConnection.readableDatabase
+            .rawQuery(
+                """
+                SELECT *, i.$ROW_ID
+                FROM $list e, ${getRowIdTableName(list)} i
+                WHERE e._id = i._id AND i.$ROW_ID = ?
+                """.trimIndent(),
+                arrayOf((index + 1).toString())
+            ).first {
+                mapCursorRowToEntity(list, it, it.getInt(ROW_ID))
+            }
+    }
+
     private fun queryWithAttachedRowId(list: String): Cursor {
         return databaseConnection.readableDatabase
             .rawQuery(

collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt

@@ -337,7 +337,10 @@ abstract class EntitiesRepositoryTest {
 
         repository.delete("1")
 
-        assertThat(repository.getEntities("wines"), containsInAnyOrder(sameEntityAs(canet)))
+        assertThat(
+            repository.getEntities("wines"),
+            containsInAnyOrder(sameEntityAs(canet))
+        )
     }
 
     @Test
@@ -482,4 +485,59 @@ abstract class EntitiesRepositoryTest {
         val repository = buildSubject()
         assertThat(repository.getAllByProperty("wines", "vintage", "1983"), equalTo(emptyList()))
     }
+
+    @Test
+    fun `#getCount returns 0 when a list is empty`() {
+        val repository = buildSubject()
+        repository.addList("wines")
+
+        assertThat(repository.getCount("wines"), equalTo(0))
+    }
+
+    @Test
+    fun `#getCount returns 0 when a list does not exist`() {
+        val repository = buildSubject()
+        assertThat(repository.getCount("wines"), equalTo(0))
+    }
+
+    @Test
+    fun `#getCount returns number of entities in list`() {
+        val repository = buildSubject()
+
+        val leoville = Entity.New("wines", "1", "Léoville Barton 2008")
+        val dows = Entity.New("wines", "2", "Dow's 1983")
+        repository.save(leoville, dows)
+
+        val springbank = Entity.New("whiskys", "1", "Springbank 10")
+        repository.save(springbank)
+
+        assertThat(repository.getCount("wines"), equalTo(2))
+        assertThat(repository.getCount("whiskys"), equalTo(1))
+    }
+
+    @Test
+    fun `#getByIndex returns matching entity`() {
+        val repository = buildSubject()
+
+        val springbank = Entity.New("whiskys", "1", "Springbank 10")
+        val aultmore = Entity.New("whiskys", "2", "Aultmore 12")
+        repository.save(springbank, aultmore)
+
+        val aultmoreIndex = repository.getEntities("whiskys").first { it.id == aultmore.id }.index
+        assertThat(repository.getByIndex("whiskys", aultmoreIndex), sameEntityAs(aultmore))
+    }
+
+    @Test
+    fun `#getByIndex returns null when the list does not exist`() {
+        val repository = buildSubject()
+        assertThat(repository.getByIndex("wine", 0), equalTo(null))
+    }
+
+    @Test
+    fun `#getByIndex returns null when the list is empty`() {
+        val repository = buildSubject()
+        repository.addList("wine")
+
+        assertThat(repository.getByIndex("wine", 0), equalTo(null))
+    }
 }

entities/src/main/java/org/odk/collect/entities/javarosa/intance/LocalEntitiesInstanceAdapter.kt

@@ -15,12 +15,24 @@ class LocalEntitiesInstanceAdapter(private val entitiesRepository: EntitiesRepos
     }
 
     fun getAll(instanceId: String, partial: Boolean): List<TreeElement> {
-        return entitiesRepository.getEntities(instanceId).mapIndexed { index, entity ->
-            if (partial && index == 0) {
-                convertToElement(entity, true)
-            } else if (partial) {
-                TreeElement("item", entity.index, true)
+        return if (partial) {
+            val count = entitiesRepository.getCount(instanceId)
+
+            if (count > 0) {
+                val first = entitiesRepository.getByIndex(instanceId, 0)!!
+
+                0.until(count).map {
+                    if (it == 0) {
+                        convertToElement(first, true)
+                    } else {
+                        TreeElement("item", it, true)
+                    }
+                }
             } else {
+                emptyList()
+            }
+        } else {
+            entitiesRepository.getEntities(instanceId).map { entity ->
                 convertToElement(entity, false)
             }
         }