perf(model-client): unnecessary overhead of BulkQuery

The ModelClientV2 uses a map based implementation of IKeyValueStore that doesn't benefit from
batching of requests. This overhead can be removed.

Author: slisson

model-datastructure/src/commonMain/kotlin/org/modelix/model/IKeyValueStore.kt

@@ -15,7 +15,12 @@
 
 package org.modelix.model
 
+import org.modelix.model.lazy.BulkQuery
+import org.modelix.model.lazy.IBulkQuery
+import org.modelix.model.lazy.IDeserializingKeyValueStore
+
 interface IKeyValueStore {
+    fun newBulkQuery(deserializingCache: IDeserializingKeyValueStore): IBulkQuery = BulkQuery(deserializingCache)
     operator fun get(key: String): String?
     fun put(key: String, value: String?)
     fun getAll(keys: Iterable<String>): Map<String, String?>

model-datastructure/src/commonMain/kotlin/org/modelix/model/lazy/CLTree.kt

@@ -93,7 +93,7 @@ class CLTree : ITree, IBulkTree {
     }
 
     fun getSize(): Long {
-        return (nodesMap ?: return 0L).calculateSize(BulkQuery(store)).execute()
+        return (nodesMap ?: return 0L).calculateSize(store.newBulkQuery()).execute()
     }
 
     fun prefetchAll() {
@@ -286,24 +286,24 @@ class CLTree : ITree, IBulkTree {
     }
 
     override fun getAllChildren(parentId: Long): Iterable<Long> {
-        val children = getChildren(resolveElement(parentId)!!, BulkQuery(store)).execute()
+        val children = getChildren(resolveElement(parentId)!!, store.newBulkQuery()).execute()
         return children.map { it.id }
     }
 
     override fun getDescendants(root: Long, includeSelf: Boolean): Iterable<CLNode> {
         val parent = resolveElement(root)
-        return getDescendants(parent!!, BulkQuery(store), includeSelf).execute().map { CLNode(this, it) }
+        return getDescendants(parent!!, store.newBulkQuery(), includeSelf).execute().map { CLNode(this, it) }
     }
 
     override fun getDescendants(rootIds: Iterable<Long>, includeSelf: Boolean): Iterable<CLNode> {
-        val bulkQuery = BulkQuery(store)
+        val bulkQuery = store.newBulkQuery()
         val roots: IBulkQuery.Value<List<CPNode>> = resolveElements(rootIds.toList(), bulkQuery)
         val descendants = roots.mapBulk { bulkQuery.map(it) { getDescendants(it, bulkQuery, includeSelf) } }
         return descendants.execute().flatten().map { CLNode(this, it) }
     }
 
     override fun getAncestors(nodeIds: Iterable<Long>, includeSelf: Boolean): Set<Long> {
-        val bulkQuery = BulkQuery(store)
+        val bulkQuery = store.newBulkQuery()
         val nodes: IBulkQuery.Value<List<CPNode>> = resolveElements(nodeIds, bulkQuery)
         val ancestors = nodes.mapBulk { bulkQuery.map(it) { getAncestors(it, bulkQuery, includeSelf) } }
         val result = HashSet<Long>()
@@ -314,15 +314,15 @@ class CLTree : ITree, IBulkTree {
     override fun getChildren(parentId: Long, role: String?): Iterable<Long> {
         checkChildRoleId(parentId, role)
         val parent = resolveElement(parentId)
-        val children = getChildren(parent!!, BulkQuery(store)).execute()
+        val children = getChildren(parent!!, store.newBulkQuery()).execute()
         return children
             .filter { it.roleInParent == role }
             .map { it.id }
     }
 
     override fun getChildRoles(sourceId: Long): Iterable<String?> {
         val parent = resolveElement(sourceId)
-        val children: Iterable<CPNode> = getChildren(parent!!, BulkQuery(store)).execute()
+        val children: Iterable<CPNode> = getChildren(parent!!, store.newBulkQuery()).execute()
         return children.map { it.roleInParent }.distinct()
     }
 
@@ -412,9 +412,9 @@ class CLTree : ITree, IBulkTree {
     }
 
     override fun visitChanges(oldVersion: ITree, visitor: ITreeChangeVisitor) {
-        val bulkQuery = BulkQuery(store)
+        val bulkQuery = store.newBulkQuery()
         visitChanges(oldVersion, visitor, bulkQuery)
-        bulkQuery.process()
+        (bulkQuery as? BulkQuery)?.process()
     }
 
     fun visitChanges(oldVersion: ITree, visitor: ITreeChangeVisitor, bulkQuery: IBulkQuery) {

model-datastructure/src/commonMain/kotlin/org/modelix/model/lazy/CLVersion.kt

@@ -325,7 +325,7 @@ private fun computeDelta(keyValueStore: IKeyValueStore, versionHash: String, bas
 //        }
 
         val history = LinearHistory(baseVersionHash).load(version)
-        val bulkQuery = BulkQuery(store)
+        val bulkQuery = store.newBulkQuery()
         var v1 = baseVersion
         for (v2 in history) {
             v2.operations // include them in the result
@@ -342,7 +342,7 @@ private fun computeDelta(keyValueStore: IKeyValueStore, versionHash: String, bas
                     override fun visitChangesOnly(): Boolean = false
                     override fun entryAdded(key: Long, value: KVEntryReference<CPNode>?) {
                         changedNodeIds += key
-                        if (value != null) bulkQuery.query(value, {})
+                        if (value != null) bulkQuery.get(value)
                     }
 
                     override fun entryRemoved(key: Long, value: KVEntryReference<CPNode>?) {
@@ -355,14 +355,14 @@ private fun computeDelta(keyValueStore: IKeyValueStore, versionHash: String, bas
                         newValue: KVEntryReference<CPNode>?,
                     ) {
                         changedNodeIds += key
-                        if (newValue != null) bulkQuery.query(newValue, {})
+                        if (newValue != null) bulkQuery.get(newValue)
                     }
                 },
                 bulkQuery,
             )
             v1 = v2
         }
-        bulkQuery.process()
+        (bulkQuery as? BulkQuery)?.process()
     }
     val oldEntries: Map<String, String?> = trackAccessedEntries(keyValueStore) { store ->
         if (baseVersionHash == null) return@trackAccessedEntries
@@ -372,16 +372,16 @@ private fun computeDelta(keyValueStore: IKeyValueStore, versionHash: String, bas
         baseVersion.operations
 
         val oldTree = baseVersion.getTree()
-        val bulkQuery = BulkQuery(store)
+        val bulkQuery = store.newBulkQuery()
 
         val nodesMap = oldTree.nodesMap!!
         changedNodeIds.forEach { changedNodeId ->
             nodesMap.get(changedNodeId, 0, bulkQuery).onSuccess { nodeRef: KVEntryReference<CPNode>? ->
-                if (nodeRef != null) bulkQuery.query(nodeRef) { a: CPNode? -> }
+                if (nodeRef != null) bulkQuery.get(nodeRef)
             }
         }
 
-        bulkQuery.process()
+        (bulkQuery as? BulkQuery)?.process()
     }
     return oldAndNewEntries - oldEntries.keys
 }
@@ -396,6 +396,10 @@ private fun trackAccessedEntries(store: IKeyValueStore, body: (IDeserializingKey
 private class AccessTrackingStore(val store: IKeyValueStore) : IKeyValueStore {
     val accessedEntries: MutableMap<String, String?> = HashMap()
 
+    override fun newBulkQuery(deserializingCache: IDeserializingKeyValueStore): IBulkQuery {
+        return store.newBulkQuery(deserializingCache)
+    }
+
     override fun get(key: String): String? {
         val value = store.get(key)
         accessedEntries.put(key, value)

model-datastructure/src/commonMain/kotlin/org/modelix/model/lazy/IDeserializingKeyValueStore.kt

@@ -18,6 +18,8 @@ package org.modelix.model.lazy
 import org.modelix.model.IKeyValueStore
 
 interface IDeserializingKeyValueStore {
+    fun newBulkQuery(): IBulkQuery = newBulkQuery(this)
+    fun newBulkQuery(wrapper: IDeserializingKeyValueStore): IBulkQuery = keyValueStore.newBulkQuery(wrapper)
     val keyValueStore: IKeyValueStore
     operator fun <T> get(hash: String, deserializer: (String) -> T): T?
     fun <T> getAll(hash: Iterable<String>, deserializer: (String, String) -> T): Iterable<T>

model-datastructure/src/commonMain/kotlin/org/modelix/model/lazy/NonBulkQuery.kt

@@ -19,7 +19,7 @@ import org.modelix.model.persistent.IKVValue
 
 class NonBulkQuery(private val store: IDeserializingKeyValueStore) : IBulkQuery {
     override fun <I, O> map(input_: Iterable<I>, f: (I) -> IBulkQuery.Value<O>): IBulkQuery.Value<List<O>> {
-        val list = input_.map(f).map { it.execute() }.toList()
+        val list = input_.asSequence().map(f).map { it.execute() }.toList()
         return Value(list)
     }
 

model-datastructure/src/commonMain/kotlin/org/modelix/model/persistent/MapBasedStore.kt

@@ -17,6 +17,9 @@ package org.modelix.model.persistent
 
 import org.modelix.model.IKeyListener
 import org.modelix.model.IKeyValueStore
+import org.modelix.model.lazy.IBulkQuery
+import org.modelix.model.lazy.IDeserializingKeyValueStore
+import org.modelix.model.lazy.NonBulkQuery
 
 @Deprecated("Use MapBasedStore, without a typo.", ReplaceWith("MapBasedStore"))
 open class MapBaseStore : MapBasedStore()
@@ -27,6 +30,11 @@ open class MapBasedStore : IKeyValueStore {
         return map[key]
     }
 
+    override fun newBulkQuery(deserializingCache: IDeserializingKeyValueStore): IBulkQuery {
+        // This implementation doesn't benefit from bulk queries. The NonBulkQuery has a lower performance overhead.
+        return NonBulkQuery(deserializingCache)
+    }
+
     override fun getPendingSize(): Int = 0
 
     override fun put(key: String, value: String?) {