Skip to content

Commit

Permalink
Merge pull request #6372 from seadowg/optimize-download
Browse files Browse the repository at this point in the history
Optimize form download with entities
  • Loading branch information
grzesiek2010 authored Aug 29, 2024
2 parents 2dff662 + 8f21378 commit f38ba7e
Show file tree
Hide file tree
Showing 10 changed files with 199 additions and 94 deletions.
2 changes: 2 additions & 0 deletions collect_app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ 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', '')
def entitiesFilterSearchTestProjectUrl = secrets.getProperty('ENTITIES_FILTER_SEARCH_TEST_PROJECT_URL', '')

android {
compileSdk Versions.android_compile_sdk
Expand Down Expand Up @@ -146,6 +147,7 @@ android {
resValue("string", "GOOGLE_MAPS_API_KEY", googleMapsApiKey)
resValue("string", "mapbox_access_token", mapboxAccessToken)
buildConfigField("String", "ENTITIES_FILTER_TEST_PROJECT_URL", "\"$entitiesFilterTestProjectUrl\"")
buildConfigField("String", "ENTITIES_FILTER_SEARCH_TEST_PROJECT_URL", "\"$entitiesFilterSearchTestProjectUrl\"")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,25 @@ 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.benchmark.support.Benchmarker
import org.odk.collect.android.benchmark.support.benchmark
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.
* Benchmarks the performance of entity follow up forms. [ENTITIES_FILTER_TEST_PROJECT_URL] should
* be set to a project that contains the "100k Entities Filter" form.
*
* Devices that currently pass:
* - Pixel 4a
* - Fairphone 3
*
*/
Expand Down Expand Up @@ -68,13 +66,13 @@ class EntitiesBenchmarkTest {
.clickOKOnDialog(MainMenuPage())

.clickGetBlankForm()
.benchmark("Downloading form with http cache", 75, benchmarker) {
.benchmark("Downloading form with http cache", 25, benchmarker) {
it.clickGetSelected()
}

.clickOK(MainMenuPage())
.clickGetBlankForm()
.benchmark("Downloading form second time with http cache", 90, benchmarker) {
.benchmark("Downloading form second time with http cache", 75, benchmarker) {
it.clickGetSelected()
}

Expand All @@ -97,67 +95,10 @@ class EntitiesBenchmarkTest {

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")
}
}
private fun clearAndroidCache() {
val application = ApplicationProvider.getApplicationContext<Application>()
application.cacheDir.deleteRecursively()
application.cacheDir.mkdir()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package org.odk.collect.android.benchmark

import androidx.test.ext.junit.runners.AndroidJUnit4
import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.blankOrNullString
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.benchmark.support.Benchmarker
import org.odk.collect.android.benchmark.support.benchmark
import org.odk.collect.android.support.TestDependencies
import org.odk.collect.android.support.pages.MainMenuPage
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_SEARCH_TEST_PROJECT_URL

/**
* Benchmarks the performance of search() forms. [ENTITIES_FILTER_SEARCH_TEST_PROJECT_URL] should be
* set to a project that contains the "100k Entities Filter search()" form.
*
* Devices that currently pass:
* - Fairphone 3
*
*/

@RunWith(AndroidJUnit4::class)
class SearchBenchmarkTest {

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_SEARCH_TEST_PROJECT_URL before running!",
ENTITIES_FILTER_SEARCH_TEST_PROJECT_URL,
not(blankOrNullString())
)

val benchmarker = Benchmarker()

rule.startAtFirstLaunch()
.clickManuallyEnterProjectDetails()
.inputUrl(ENTITIES_FILTER_SEARCH_TEST_PROJECT_URL)
.addProject()

.clickGetBlankForm()
.clickGetSelected()
.clickOK(MainMenuPage())

.clickFillBlankForm()
.benchmark("Loading form first time", 20, benchmarker) {
it.clickOnForm("100k Entities Filter search()")
}

.pressBackAndDiscardForm()
.clickFillBlankForm()
.benchmark("Loading form second time", 2, benchmarker) {
it.clickOnForm("100k Entities Filter search()")
}

.answerQuestion("Which value do you want to filter by?", "1024")
.benchmark("Filtering select", 2, benchmarker) {
it.swipeToNextQuestion("Filtered select")
}

benchmarker.assertResults()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package org.odk.collect.android.benchmark.support

import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.lessThan
import org.odk.collect.android.support.pages.Page
import org.odk.collect.shared.TimeInMs

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")
}
}
}

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 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]!!
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ import androidx.core.database.sqlite.transaction
import org.odk.collect.db.sqlite.CursorExt.first
import org.odk.collect.db.sqlite.CursorExt.foldAndClose
import org.odk.collect.db.sqlite.CursorExt.getInt
import org.odk.collect.db.sqlite.CursorExt.getIntOrNull
import org.odk.collect.db.sqlite.CursorExt.getString
import org.odk.collect.db.sqlite.CursorExt.getStringOrNull
import org.odk.collect.db.sqlite.CursorExt.rowToMap
import org.odk.collect.db.sqlite.DatabaseConnection
import org.odk.collect.db.sqlite.DatabaseMigrator
import org.odk.collect.db.sqlite.SQLiteColumns.ROW_ID
Expand Down Expand Up @@ -111,7 +110,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
it.put(EntitiesTable.COLUMN_STATE, convertStateToInt(entity.state))

entity.properties.forEach { (name, value) ->
it.put(name, value)
it.put("\"$name\"", value)
}
}

Expand Down Expand Up @@ -348,7 +347,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
try {
databaseConnection.writeableDatabase.execSQL(
"""
ALTER TABLE ${entity.list} ADD $it text NOT NULL DEFAULT "";
ALTER TABLE ${entity.list} ADD "$it" text NOT NULL DEFAULT "";
""".trimIndent()
)
} catch (e: SQLiteException) {
Expand All @@ -362,7 +361,9 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
cursor: Cursor,
rowId: Int
): Entity.Saved {
val propertyColumns = cursor.columnNames.filter {
val map = cursor.rowToMap()

val propertyColumns = map.keys.filter {
!listOf(
_ID,
EntitiesTable.COLUMN_ID,
Expand All @@ -377,25 +378,25 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep

val properties =
propertyColumns.fold(emptyList<Pair<String, String>>()) { accum, property ->
accum + Pair(property, cursor.getStringOrNull(property) ?: "")
accum + Pair(property, map[property] ?: "")
}

val state = if (cursor.getInt(EntitiesTable.COLUMN_STATE) == 0) {
val state = if (map[EntitiesTable.COLUMN_STATE]!!.toInt() == 0) {
Entity.State.OFFLINE
} else {
Entity.State.ONLINE
}

return Entity.Saved(
list,
cursor.getString(EntitiesTable.COLUMN_ID),
cursor.getStringOrNull(EntitiesTable.COLUMN_LABEL),
cursor.getInt(EntitiesTable.COLUMN_VERSION),
map[EntitiesTable.COLUMN_ID]!!,
map[EntitiesTable.COLUMN_LABEL],
map[EntitiesTable.COLUMN_VERSION]!!.toInt(),
properties,
state,
rowId - 1,
cursor.getIntOrNull(EntitiesTable.COLUMN_TRUNK_VERSION),
cursor.getString(EntitiesTable.COLUMN_BRANCH_ID)
map[EntitiesTable.COLUMN_TRUNK_VERSION]?.toInt(),
map[EntitiesTable.COLUMN_BRANCH_ID]!!
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,19 @@ abstract class EntitiesRepositoryTest {
assertThat(repository.getLists(), containsInAnyOrder("wines", "blah"))
}

@Test
fun `#save supports properties with dots and dashes`() {
val repository = buildSubject()
val entity = Entity.New(
"things",
"1",
"One",
properties = listOf(Pair("a.property", "value"), Pair("a-property", "value"))
)
repository.save(entity)
assertThat(repository.getEntities("things")[0], sameEntityAs(entity))
}

@Test
fun `#clear deletes all entities`() {
val repository = buildSubject()
Expand Down
13 changes: 13 additions & 0 deletions db/src/main/java/org/odk/collect/db/sqlite/CursorExt.kt
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,17 @@ object CursorExt {
val columnIndex = this.getColumnIndex(column)
return this.getIntOrNull(columnIndex)
}

/**
* Prevents doing repeated [Cursor.getColumnIndex] lookups and also works around the lack of
* support for column names including a "." there (due to the mysterious bug 903852).
*
* @see [SQLiteCursor source](https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/database/sqlite/SQLiteCursor.java;l=178?q=sqlitecursor)
*/
fun Cursor.rowToMap(): Map<String, String> {
return this.columnNames.foldIndexed(mutableMapOf()) { index, map, column ->
map[column] = this.getString(index)
map
}
}
}
Loading

0 comments on commit f38ba7e

Please sign in to comment.