From 4792dc1172246c57b0ad14de3ee4649f5e4dafdd Mon Sep 17 00:00:00 2001 From: Daan Vanden Bosch Date: Fri, 3 Dec 2021 22:08:04 +0100 Subject: [PATCH] Slightly optimized bindChildrenTo for the frequent case where all list cell elements have been replaced. --- .../cell/list/AbstractDependentListCell.kt | 19 +++++-- .../observable/cell/list/AbstractListCell.kt | 9 ++-- .../observable/cell/list/FilteredListCell.kt | 4 ++ .../observable/cell/list/ImmutableListCell.kt | 10 +++- .../observable/cell/list/ListObserver.kt | 5 +- .../observable/cell/list/SimpleListCell.kt | 54 ++++++++++++++----- .../kotlin/world/phantasmal/webui/dom/Dom.kt | 8 ++- 7 files changed, 86 insertions(+), 23 deletions(-) diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractDependentListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractDependentListCell.kt index 3934ed6d..f2483204 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractDependentListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractDependentListCell.kt @@ -65,9 +65,12 @@ abstract class AbstractDependentListCell : observer( ListChangeEvent( value, - listOf( - ListChange.Structural(index = 0, removed = emptyList(), inserted = value), - ), + listOf(ListChange.Structural( + index = 0, + prevSize = 0, + removed = emptyList(), + inserted = value, + )), ) ) } @@ -81,7 +84,15 @@ abstract class AbstractDependentListCell : computeElements() emitDependencyChanged( - ListChangeEvent(elements, listOf(ListChange.Structural(0, oldElements, elements))) + ListChangeEvent( + elements, + listOf(ListChange.Structural( + index = 0, + prevSize = oldElements.size, + removed = oldElements, + inserted = elements, + )), + ) ) } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractListCell.kt index 275c614e..dcf5b337 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractListCell.kt @@ -49,9 +49,12 @@ abstract class AbstractListCell : AbstractCell>(), ListCell { observer( ListChangeEvent( value, - listOf( - ListChange.Structural(index = 0, removed = emptyList(), inserted = value), - ), + listOf(ListChange.Structural( + index = 0, + prevSize = 0, + removed = emptyList(), + inserted = value + )), ) ) } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FilteredListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FilteredListCell.kt index 7761cb31..a2c2c1fc 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FilteredListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FilteredListCell.kt @@ -48,6 +48,7 @@ class FilteredListCell( override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { if (event is ListChangeEvent<*>) { + val prevSize = elements.size val filteredChanges = mutableListOf>() for (change in event.changes) { @@ -95,6 +96,7 @@ class FilteredListCell( filteredChanges.add( ListChange.Structural( eventIndex, + prevSize, removed, inserted ) @@ -140,6 +142,7 @@ class FilteredListCell( filteredChanges.add( ListChange.Structural( insertIndex, + prevSize, removed = emptyList(), inserted = listOf(change.updated), ) @@ -167,6 +170,7 @@ class FilteredListCell( filteredChanges.add( ListChange.Structural( index, + prevSize, removed = listOf(change.updated), inserted = emptyList(), ) diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ImmutableListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ImmutableListCell.kt index b1883d76..14c9ff2a 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ImmutableListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ImmutableListCell.kt @@ -32,7 +32,15 @@ class ImmutableListCell(private val elements: List) : AbstractDependency() override fun observeList(callNow: Boolean, observer: ListObserver): Disposable { if (callNow) { - observer(ListChangeEvent(value, listOf(ListChange.Structural(0, emptyList(), value)))) + observer(ListChangeEvent( + value, + listOf(ListChange.Structural( + index = 0, + prevSize = 0, + removed = emptyList(), + inserted = value, + )), + )) } return nopDisposable() diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListObserver.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListObserver.kt index 37584fd5..20a06b3f 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListObserver.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListObserver.kt @@ -13,6 +13,7 @@ sealed class ListChange { */ class Structural( val index: Int, + val prevSize: Int, /** * The elements that were removed from the list at [index]. * @@ -27,7 +28,9 @@ sealed class ListChange { * be mutated when the originating [ListCell] is mutated. */ val inserted: List, - ) : ListChange() + ) : ListChange() { + val allRemoved: Boolean get() = removed.size == prevSize + } /** * Represents a change to an element in a list cell. Will only be emitted if the list is diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleListCell.kt index f7697e96..85b974be 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleListCell.kt @@ -50,7 +50,12 @@ class SimpleListCell( elementDependents[index] = ElementDependent(index, element) } - changes.add(ListChange.Structural(index, listOf(removed), listOf(element))) + changes.add(ListChange.Structural( + index, + prevSize = elements.size, + removed = listOf(removed), + inserted = listOf(element), + )) ChangeManager.changed(this) return removed @@ -63,17 +68,23 @@ class SimpleListCell( copyAndResetWrapper() elements.add(element) - finalizeStructuralChange(index, emptyList(), listOf(element)) + finalizeStructuralChange( + index, + prevSize = index, + removed = emptyList(), + inserted = listOf(element), + ) } override fun add(index: Int, element: E) { - checkIndex(index, elements.size) + val prevSize = elements.size + checkIndex(index, prevSize) emitMightChange() copyAndResetWrapper() elements.add(index, element) - finalizeStructuralChange(index, emptyList(), listOf(element)) + finalizeStructuralChange(index, prevSize, removed = emptyList(), inserted = listOf(element)) } override fun remove(element: E): Boolean { @@ -91,34 +102,41 @@ class SimpleListCell( checkIndex(index, elements.lastIndex) emitMightChange() + val prevSize = elements.size + copyAndResetWrapper() val removed = elements.removeAt(index) - finalizeStructuralChange(index, listOf(removed), emptyList()) + finalizeStructuralChange(index, prevSize, removed = listOf(removed), inserted = emptyList()) return removed } override fun replaceAll(elements: Iterable) { emitMightChange() + val prevSize = this.elements.size val removed = elementsWrapper + copyAndResetWrapper() this.elements.replaceAll(elements) - finalizeStructuralChange(0, removed, elementsWrapper) + finalizeStructuralChange(index = 0, prevSize, removed, inserted = elementsWrapper) } override fun replaceAll(elements: Sequence) { emitMightChange() + val prevSize = this.elements.size val removed = elementsWrapper + copyAndResetWrapper() this.elements.replaceAll(elements) - finalizeStructuralChange(0, removed, elementsWrapper) + finalizeStructuralChange(index = 0, prevSize, removed, inserted = elementsWrapper) } override fun splice(fromIndex: Int, removeCount: Int, newElement: E) { + val prevSize = elements.size val removed = ArrayList(removeCount) for (i in fromIndex until (fromIndex + removeCount)) { @@ -131,17 +149,19 @@ class SimpleListCell( repeat(removeCount) { elements.removeAt(fromIndex) } elements.add(fromIndex, newElement) - finalizeStructuralChange(fromIndex, removed, listOf(newElement)) + finalizeStructuralChange(fromIndex, prevSize, removed, inserted = listOf(newElement)) } override fun clear() { emitMightChange() + val prevSize = elements.size val removed = elementsWrapper + copyAndResetWrapper() elements.clear() - finalizeStructuralChange(0, removed, emptyList()) + finalizeStructuralChange(index = 0, prevSize, removed, inserted = emptyList()) } override fun sortWith(comparator: Comparator) { @@ -157,7 +177,12 @@ class SimpleListCell( throwable = e } - finalizeStructuralChange(0, removed, elementsWrapper) + finalizeStructuralChange( + index = 0, + prevSize = elements.size, + removed, + inserted = elementsWrapper, + ) if (throwable != null) { throw throwable @@ -200,7 +225,12 @@ class SimpleListCell( } } - private fun finalizeStructuralChange(index: Int, removed: List, inserted: List) { + private fun finalizeStructuralChange( + index: Int, + prevSize: Int, + removed: List, + inserted: List, + ) { if (dependents.isNotEmpty() && extractDependencies != null) { repeat(removed.size) { elementDependents.removeAt(index).dispose() @@ -218,7 +248,7 @@ class SimpleListCell( } } - changes.add(ListChange.Structural(index, removed, inserted)) + changes.add(ListChange.Structural(index, prevSize, removed, inserted)) ChangeManager.changed(this) } diff --git a/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt b/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt index 95081fa4..95e75f56 100644 --- a/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt +++ b/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt @@ -273,8 +273,12 @@ private fun bindChildrenTo( list.observeList(callNow = true) { event: ListChangeEvent -> for (change in event.changes) { if (change is ListChange.Structural) { - repeat(change.removed.size) { - parent.removeChild(parent.childNodes[change.index].unsafeCast()) + if (change.allRemoved) { + parent.innerHTML = "" + } else { + repeat(change.removed.size) { + parent.removeChild(parent.childNodes[change.index].unsafeCast()) + } } childrenRemoved(change.index, change.removed.size)