From 578d72e4706ddff05467326a2a10d38b384b6e04 Mon Sep 17 00:00:00 2001 From: Danielle Voznyy Date: Thu, 30 May 2024 23:08:52 -0400 Subject: [PATCH] chore: Consistently use T.(T) for supporting both shorthand and object queries perf: Avoid going through delegate entirely in shorthand queries --- .../geary/serialization/formats/Format.kt | 6 +- .../benchmarks/VelocitySystemBenchmark.kt | 14 +-- .../geary/systems/accessors/Accessor.kt | 22 ++++- .../accessors/type/ComponentAccessor.kt | 12 +-- .../type/ComponentOrDefaultAccessor.kt | 6 +- .../systems/accessors/type/MappedAccessor.kt | 5 +- .../accessors/type/RelationsAccessor.kt | 5 +- .../type/RelationsWithDataAccessor.kt | 7 +- .../geary/systems/builders/SystemBuilder.kt | 6 +- .../geary/systems/query/CachedQuery.kt | 6 +- .../geary/systems/query/QueryShorthands.kt | 92 +++++++++---------- .../geary/queries/SimpleQueryTest.kt | 34 ++++--- .../geary/systems/FamilyMatchingTest.kt | 2 +- 13 files changed, 112 insertions(+), 105 deletions(-) diff --git a/addons/geary-serialization/src/commonMain/kotlin/com/mineinabyss/geary/serialization/formats/Format.kt b/addons/geary-serialization/src/commonMain/kotlin/com/mineinabyss/geary/serialization/formats/Format.kt index c90e3144..002b0dad 100644 --- a/addons/geary-serialization/src/commonMain/kotlin/com/mineinabyss/geary/serialization/formats/Format.kt +++ b/addons/geary-serialization/src/commonMain/kotlin/com/mineinabyss/geary/serialization/formats/Format.kt @@ -1,8 +1,10 @@ package com.mineinabyss.geary.serialization.formats -import kotlinx.serialization.DeserializationStrategy -import kotlinx.serialization.SerializationStrategy +import kotlinx.serialization.* +import kotlinx.serialization.json.Json import kotlinx.serialization.modules.SerializersModule +import kotlinx.serialization.modules.polymorphic +import kotlinx.serialization.modules.subclass import okio.Path interface Format { diff --git a/geary-benchmarks/src/main/kotlin/com/mineinabyss/geary/benchmarks/VelocitySystemBenchmark.kt b/geary-benchmarks/src/main/kotlin/com/mineinabyss/geary/benchmarks/VelocitySystemBenchmark.kt index e9905f8a..ba925a76 100644 --- a/geary-benchmarks/src/main/kotlin/com/mineinabyss/geary/benchmarks/VelocitySystemBenchmark.kt +++ b/geary-benchmarks/src/main/kotlin/com/mineinabyss/geary/benchmarks/VelocitySystemBenchmark.kt @@ -7,6 +7,7 @@ import com.mineinabyss.geary.modules.TestEngineModule import com.mineinabyss.geary.modules.geary import com.mineinabyss.geary.systems.query.Query import com.mineinabyss.geary.systems.builders.system +import com.mineinabyss.geary.systems.query.query import org.openjdk.jmh.annotations.Benchmark import org.openjdk.jmh.annotations.Scope import org.openjdk.jmh.annotations.Setup @@ -21,16 +22,15 @@ class VelocitySystemBenchmark { val velocity by get() var position by get() }).exec { + it.position.x += it.velocity.x + it.position.y += it.velocity.y + } + fun createVelocitySystemNoDelegates() = geary.system( + query() + ).exec { (velocity, position) -> position.x += velocity.x position.y += velocity.y } - fun createVelocitySystemNoDelegates() = geary.system(object : Query() { - val velocity = get() - var position = get() - }).exec { - position().x += velocity().x - position().y += velocity().y - } val velocities = Array(tenMil) { Velocity(it.toFloat() / oneMil, it.toFloat() / oneMil) } val positions = Array(tenMil) { Position(0f, 0f) } diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/Accessor.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/Accessor.kt index ea5137f6..9d789b01 100644 --- a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/Accessor.kt +++ b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/Accessor.kt @@ -3,11 +3,29 @@ package com.mineinabyss.geary.systems.accessors import com.mineinabyss.geary.systems.query.Query import kotlin.properties.ReadOnlyProperty import kotlin.properties.ReadWriteProperty +import kotlin.reflect.KProperty interface Accessor { val originalAccessor: Accessor? } -interface ReadOnlyAccessor : Accessor, ReadOnlyProperty +interface ReadOnlyAccessor : Accessor, ReadOnlyProperty { + fun get(query: Query): T -interface ReadWriteAccessor : ReadOnlyAccessor, ReadWriteProperty + + override fun getValue(thisRef: Query, property: KProperty<*>): T { + return get(thisRef) + } +} + +interface ReadWriteAccessor : ReadOnlyAccessor, ReadWriteProperty { + fun set(query: Query, value: T) + + override fun getValue(thisRef: Query, property: KProperty<*>): T { + return get(thisRef) + } + + override fun setValue(thisRef: Query, property: KProperty<*>, value: T) { + return set(thisRef, value) + } +} diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/ComponentAccessor.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/ComponentAccessor.kt index 3e0a0ab0..58823cfa 100644 --- a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/ComponentAccessor.kt +++ b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/ComponentAccessor.kt @@ -28,19 +28,11 @@ class ComponentAccessor( if (cachedIndex != -1) cachedDataArray = archetype.componentData[cachedIndex] as MutableObjectList } - fun get(query: Query): T { + override fun get(query: Query): T { return cachedDataArray[query.row] } - fun set(query: Query, value: T) { + override fun set(query: Query, value: T) { cachedDataArray[query.row] = value } - - override fun getValue(thisRef: Query, property: KProperty<*>): T { - return get(thisRef) - } - - override fun setValue(thisRef: Query, property: KProperty<*>, value: T) { - return set(thisRef, value) - } } diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/ComponentOrDefaultAccessor.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/ComponentOrDefaultAccessor.kt index ac2853fa..9839977f 100644 --- a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/ComponentOrDefaultAccessor.kt +++ b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/ComponentOrDefaultAccessor.kt @@ -17,14 +17,14 @@ class ComponentOrDefaultAccessor( private var cachedArchetype: Archetype? = null @OptIn(UnsafeAccessors::class) - override fun getValue(thisRef: Query, property: KProperty<*>): T { - val archetype = thisRef.archetype + override fun get(query: Query): T { + val archetype = query.archetype if (archetype !== cachedArchetype) { cachedArchetype = archetype cachedIndex = archetype.indexOf(id) } if (cachedIndex == -1) return default() @Suppress("UNCHECKED_CAST") - return archetype.componentData[cachedIndex][thisRef.row] as T + return archetype.componentData[cachedIndex][query.row] as T } } diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/MappedAccessor.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/MappedAccessor.kt index 8df8b579..521ceadf 100644 --- a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/MappedAccessor.kt +++ b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/MappedAccessor.kt @@ -2,14 +2,13 @@ package com.mineinabyss.geary.systems.accessors.type import com.mineinabyss.geary.systems.accessors.ReadOnlyAccessor import com.mineinabyss.geary.systems.query.Query -import kotlin.reflect.KProperty class MappedAccessor( override val originalAccessor: ReadOnlyAccessor, val mapping: (T) -> U, ) : ReadOnlyAccessor { - override fun getValue(thisRef: Query, property: KProperty<*>): U { - val value = originalAccessor.getValue(thisRef, property) + override fun get(query: Query): U { + val value = originalAccessor.get(query) return mapping(value) } } diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/RelationsAccessor.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/RelationsAccessor.kt index 8c1638e2..80f590b0 100644 --- a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/RelationsAccessor.kt +++ b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/RelationsAccessor.kt @@ -10,7 +10,6 @@ import com.mineinabyss.geary.systems.accessors.Accessor import com.mineinabyss.geary.systems.accessors.FamilyMatching import com.mineinabyss.geary.systems.accessors.ReadOnlyAccessor import com.mineinabyss.geary.systems.query.Query -import kotlin.reflect.KProperty class RelationsAccessor( override val originalAccessor: Accessor?, @@ -23,8 +22,8 @@ class RelationsAccessor( private var cachedArchetype: Archetype? = null @OptIn(UnsafeAccessors::class) - override fun getValue(thisRef: Query, property: KProperty<*>): List { - val archetype = thisRef.archetype + override fun get(query: Query): List { + val archetype = query.archetype if (archetype != cachedArchetype) { cachedArchetype = archetype cachedRelations = archetype.getRelations(kind, target) diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/RelationsWithDataAccessor.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/RelationsWithDataAccessor.kt index f9527435..9508c23e 100644 --- a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/RelationsWithDataAccessor.kt +++ b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/accessors/type/RelationsWithDataAccessor.kt @@ -11,7 +11,6 @@ import com.mineinabyss.geary.systems.accessors.FamilyMatching import com.mineinabyss.geary.systems.accessors.ReadOnlyAccessor import com.mineinabyss.geary.systems.accessors.RelationWithData import com.mineinabyss.geary.systems.query.Query -import kotlin.reflect.KProperty @OptIn(UnsafeAccessors::class) class RelationsWithDataAccessor( @@ -24,8 +23,8 @@ class RelationsWithDataAccessor( private var cachedRelations = emptyList() private var cachedArchetype: Archetype? = null - override fun getValue(thisRef: Query, property: KProperty<*>): List> { - val archetype = thisRef.archetype + override fun get(query: Query): List> { + val archetype = query.archetype if (archetype != cachedArchetype) { cachedArchetype = archetype cachedRelations = archetype.getRelations(kind, target) @@ -33,7 +32,7 @@ class RelationsWithDataAccessor( @Suppress("UNCHECKED_CAST") return archetype.readRelationDataFor( - thisRef.row, + query.row, kind, target, cachedRelations diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/builders/SystemBuilder.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/builders/SystemBuilder.kt index 9c62775b..6af90a63 100644 --- a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/builders/SystemBuilder.kt +++ b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/builders/SystemBuilder.kt @@ -22,14 +22,14 @@ data class SystemBuilder( } inline fun exec(crossinline run: T.(T) -> Unit): TrackedSystem<*> { - val onTick: CachedQuery.() -> Unit = { forEach(run) } + val onTick: CachedQuery.() -> Unit = { forEach { run(it) } } val system = System(name, query, onTick, interval) return pipeline.addSystem(system) } - inline fun defer(crossinline run: T.() -> R): DeferredSystemBuilder { + inline fun defer(crossinline run: T.(T) -> R): DeferredSystemBuilder { val onTick: CachedQuery.() -> List> = { - mapWithEntity { run() } + mapWithEntity { run(it) } } val system = DeferredSystemBuilder(this, onTick) return system diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/query/CachedQuery.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/query/CachedQuery.kt index 09f51325..8f5c402e 100644 --- a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/query/CachedQuery.kt +++ b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/query/CachedQuery.kt @@ -123,13 +123,13 @@ class CachedQuery internal constructor(val query: T) { return collected } - inline fun map(crossinline run: (T) -> R): List { + inline fun map(crossinline run: T.(T) -> R): List { val deferred = mutableListOf() forEach { deferred.add(run(it)) } return deferred } - inline fun mapNotNull(crossinline run: (T) -> R?): List { + inline fun mapNotNull(crossinline run: T.(T) -> R?): List { val deferred = mutableListOf() forEach { query -> run(query).let { if (it != null) deferred.add(it) } } return deferred @@ -171,7 +171,7 @@ class CachedQuery internal constructor(val query: T) { ) @OptIn(UnsafeAccessors::class) - inline fun mapWithEntity(crossinline run: T.() -> R): List> { + inline fun mapWithEntity(crossinline run: T.(T) -> R): List> { val deferred = mutableListOf>() forEach { deferred.add(Deferred(run(it), it.unsafeEntity)) diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/query/QueryShorthands.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/query/QueryShorthands.kt index 8895818e..fd4748f7 100644 --- a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/query/QueryShorthands.kt +++ b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/systems/query/QueryShorthands.kt @@ -13,23 +13,23 @@ abstract class ShorthandQuery : Query() { } abstract class ShorthandQuery1 : ShorthandQuery() { - abstract val comp1: A + val comp1 get() = component1() abstract operator fun component1(): A } abstract class ShorthandQuery2 : ShorthandQuery() { - abstract val comp1: A - abstract val comp2: B + val comp1 get() = component1() + val comp2 get() = component2() abstract operator fun component1(): A abstract operator fun component2(): B } abstract class ShorthandQuery3 : ShorthandQuery() { - abstract val comp1: A - abstract val comp2: B - abstract val comp3: C + val comp1 get() = component1() + val comp2 get() = component2() + val comp3 get() = component3() abstract operator fun component1(): A abstract operator fun component2(): B @@ -37,10 +37,10 @@ abstract class ShorthandQuery3 : ShorthandQuery() { } abstract class ShorthandQuery4 : ShorthandQuery() { - abstract val comp1: A - abstract val comp2: B - abstract val comp3: C - abstract val comp4: D + val comp1 get() = component1() + val comp2 get() = component2() + val comp3 get() = component3() + val comp4 get() = component4() abstract operator fun component1(): A abstract operator fun component2(): B @@ -49,11 +49,11 @@ abstract class ShorthandQuery4 : ShorthandQuery() { } abstract class ShorthandQuery5 : ShorthandQuery() { - abstract val comp1: A - abstract val comp2: B - abstract val comp3: C - abstract val comp4: D - abstract val comp5: E + val comp1 get() = component1() + val comp2 get() = component2() + val comp3 get() = component3() + val comp4 get() = component4() + val comp5 get() = component5() abstract operator fun component1(): A abstract operator fun component2(): B @@ -78,9 +78,9 @@ inline fun query( filterFamily?.let { this { it() } } } - override val comp1 by getPotentiallyNullable() + private val accessor1 = getPotentiallyNullable() - override fun component1() = comp1 + override fun component1() = accessor1.get(this) } inline fun query( @@ -92,11 +92,11 @@ inline fun query( filterFamily?.let { this { it() } } } - override val comp1 by getPotentiallyNullable() - override val comp2 by getPotentiallyNullable() + private val accessor1 = getPotentiallyNullable() + private val accessor2 = getPotentiallyNullable() - override fun component1(): A = comp1 - override fun component2(): B = comp2 + override fun component1(): A = accessor1.get(this) + override fun component2(): B = accessor2.get(this) } @@ -109,13 +109,13 @@ inline fun query( filterFamily?.let { this { it() } } } - override val comp1 by getPotentiallyNullable() - override val comp2 by getPotentiallyNullable() - override val comp3 by getPotentiallyNullable() + private val accessor1 = getPotentiallyNullable() + private val accessor2 = getPotentiallyNullable() + private val accessor3 = getPotentiallyNullable() - override fun component1(): A = comp1 - override fun component2(): B = comp2 - override fun component3(): C = comp3 + override fun component1(): A = accessor1.get(this) + override fun component2(): B = accessor2.get(this) + override fun component3(): C = accessor3.get(this) } inline fun query( @@ -127,15 +127,15 @@ inline fun query( filterFamily?.let { this { it() } } } - override val comp1 by getPotentiallyNullable() - override val comp2 by getPotentiallyNullable() - override val comp3 by getPotentiallyNullable() - override val comp4 by getPotentiallyNullable() + private val accessor1 = getPotentiallyNullable() + private val accessor2 = getPotentiallyNullable() + private val accessor3 = getPotentiallyNullable() + private val accessor4 = getPotentiallyNullable() - override fun component1(): A = comp1 - override fun component2(): B = comp2 - override fun component3(): C = comp3 - override fun component4(): D = comp4 + override fun component1(): A = accessor1.get(this) + override fun component2(): B = accessor2.get(this) + override fun component3(): C = accessor3.get(this) + override fun component4(): D = accessor4.get(this) } inline fun query( @@ -147,17 +147,17 @@ inline fun query( filterFamily?.let { this { it() } } } - override val comp1 by getPotentiallyNullable() - override val comp2 by getPotentiallyNullable() - override val comp3 by getPotentiallyNullable() - override val comp4 by getPotentiallyNullable() - override val comp5 by getPotentiallyNullable() - - override fun component1(): A = comp1 - override fun component2(): B = comp2 - override fun component3(): C = comp3 - override fun component4(): D = comp4 - override fun component5(): E = comp5 + private val accessor1 = getPotentiallyNullable() + private val accessor2 = getPotentiallyNullable() + private val accessor3 = getPotentiallyNullable() + private val accessor4 = getPotentiallyNullable() + private val accessor5 = getPotentiallyNullable() + + override fun component1(): A = accessor1.get(this) + override fun component2(): B = accessor2.get(this) + override fun component3(): C = accessor3.get(this) + override fun component4(): D = accessor4.get(this) + override fun component5(): E = accessor5.get(this) } @JvmName("toList1") diff --git a/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/queries/SimpleQueryTest.kt b/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/queries/SimpleQueryTest.kt index f4292505..a59d2db0 100644 --- a/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/queries/SimpleQueryTest.kt +++ b/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/queries/SimpleQueryTest.kt @@ -19,9 +19,7 @@ import kotlin.test.Test @OptIn(ExperimentalGearyApi::class) class SimpleQueryTest : GearyTest() { - class MyQuery : Query() { - val int by get() - } + val myQuery = query() @BeforeAll fun ensureTestEntitiesExist() { @@ -37,10 +35,10 @@ class SimpleQueryTest : GearyTest() { @Test fun `forEach should allow reading data`() { - val query = geary.queryManager.trackQuery(MyQuery()) + val query = geary.queryManager.trackQuery(myQuery) val nums = mutableListOf() - query.forEach { + query.forEach { (int) -> nums.add(int) } nums.sorted() shouldBe (0..9).toList() @@ -48,7 +46,7 @@ class SimpleQueryTest : GearyTest() { @Test fun `entities should return matched entities correctly`() { - val query = geary.queryManager.trackQuery(MyQuery()) + val query = geary.queryManager.trackQuery(myQuery) val nums = mutableListOf() query.entities().forEach { @@ -60,45 +58,45 @@ class SimpleQueryTest : GearyTest() { @Test fun `first should correctly return first matched entity`() { - val query = geary.queryManager.trackQuery(MyQuery()) + val query = geary.queryManager.trackQuery(myQuery) - query.find({ int }, { int == 5 }) shouldBe 5 + query.find({ it.comp1 }, { it.comp1 == 5 }) shouldBe 5 } @Test fun `any should correctly check for matches if matched`() { - val query = geary.queryManager.trackQuery(MyQuery()) + val query = geary.queryManager.trackQuery(myQuery) - query.any { int == 5 } shouldBe true - query.any { int == 100 } shouldBe false + query.any { it.comp1 == 5 } shouldBe true + query.any { it.comp1 == 100 } shouldBe false } @Test fun `should be able to collect query as sequence`() { - val query = geary.queryManager.trackQuery(MyQuery()) + val query = geary.queryManager.trackQuery(myQuery) query.collect { - filter { it.int % 2 == 0 }.map { it.int.toString() }.toList() + filter { it.comp1 % 2 == 0 }.map { it.comp1.toString() }.toList() } shouldBe (0..9 step 2).map { it.toString() } } @Test fun `should allow collecting fancier sequences`() { - val query = geary.queryManager.trackQuery(MyQuery()) + val query = geary.queryManager.trackQuery(myQuery) query.collect { - filter { it.int % 2 == 0 }.map { it.int }.sortedByDescending { it }.take(3).toList() + filter { it.comp1 % 2 == 0 }.map { it.comp1 }.sortedByDescending { it }.take(3).toList() } shouldBe listOf(8, 6, 4) } @Test fun `should not allow working on sequence outside collect block`() { - val query = geary.queryManager.trackQuery(MyQuery()) + val query = geary.queryManager.trackQuery(myQuery) shouldThrow { query.collect { - filter { it.int % 2 == 0 } - }.map { it.int.toString() }.toList() + filter { it.comp1 % 2 == 0 } + }.map { it.comp1.toString() }.toList() } } diff --git a/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/systems/FamilyMatchingTest.kt b/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/systems/FamilyMatchingTest.kt index f5fa7c60..8ba6c499 100644 --- a/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/systems/FamilyMatchingTest.kt +++ b/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/systems/FamilyMatchingTest.kt @@ -23,7 +23,7 @@ class FamilyMatchingTest : GearyTest() { val system = geary.system(object : Query() { val string by get() override fun ensure() = this { has() } - }).defer { string }.onFinish { data, entity -> + }).defer { it.string }.onFinish { data, entity -> data shouldBe entity.get() entity.has() shouldBe true }