Skip to content

Commit

Permalink
Eliminate boxing of protocol types for serialization (#2330)
Browse files Browse the repository at this point in the history 
* Eliminate boxing of protocol types for serialization

We still have a few cases of use within a Zipline interface that causes it, but those are rarely invoked (protocol mismatch handlers).

* Fix test

* WHOOPS

* Push initialization of subtree synthesization to base

This deduplicates things, but it also prevents an initialization cycle which was causing the value to always be false.
  • Loading branch information
@JakeWharton
JakeWharton authored Sep 27, 2024
1 parent 3fe178c commit ebdc0b1
Show file tree
Hide file tree
Showing 13 changed files with 271 additions and 142 deletions.
17 changes: 11 additions & 6 deletions redwood-protocol-guest/api/redwood-protocol-guest.api
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public final class app/cash/redwood/protocol/guest/DefaultGuestProtocolAdapter :
public fun emitChanges ()V
public fun getJson ()Lkotlinx/serialization/json/Json;
public fun getRoot ()Lapp/cash/redwood/widget/Widget$Children;
public fun getSynthesizeSubtreeRemoval ()Z
public fun getWidgetSystem ()Lapp/cash/redwood/widget/WidgetSystem;
public fun initChangesSink (Lapp/cash/redwood/protocol/ChangesSink;)V
public fun nextId-0HhLjSo ()I
Expand All @@ -22,7 +21,9 @@ public final class app/cash/redwood/protocol/guest/DefaultGuestProtocolAdapter :
public final fun takeChanges ()Ljava/util/List;
}

public abstract interface class app/cash/redwood/protocol/guest/GuestProtocolAdapter : app/cash/redwood/protocol/EventSink {
public abstract class app/cash/redwood/protocol/guest/GuestProtocolAdapter : app/cash/redwood/protocol/EventSink {
public static final field $stable I
public synthetic fun <init> (Ljava/lang/String;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public abstract fun appendAdd-ARs5Qwk (IIILapp/cash/redwood/protocol/guest/ProtocolWidget;)V
public abstract fun appendCreate-kyz2zXs (II)V
public abstract fun appendModifierChange-z3jyS0k (ILapp/cash/redwood/Modifier;)V
Expand All @@ -31,11 +32,11 @@ public abstract interface class app/cash/redwood/protocol/guest/GuestProtocolAda
public abstract fun appendPropertyChange-M7EZMwg (III)V
public abstract fun appendPropertyChange-e3iP1vo (IIZ)V
public abstract fun appendRemove-HpxY78w (IIIILjava/util/List;)V
public static synthetic fun appendRemove-HpxY78w$default (Lapp/cash/redwood/protocol/guest/GuestProtocolAdapter;IIIILjava/util/List;ILjava/lang/Object;)V
public abstract fun emitChanges ()V
public final fun getChildrenVisitor ()Lapp/cash/redwood/protocol/guest/ProtocolWidget$ChildrenVisitor;
public abstract fun getJson ()Lkotlinx/serialization/json/Json;
public abstract fun getRoot ()Lapp/cash/redwood/widget/Widget$Children;
public abstract fun getSynthesizeSubtreeRemoval ()Z
public final fun getSynthesizeSubtreeRemoval ()Z
public abstract fun getWidgetSystem ()Lapp/cash/redwood/widget/WidgetSystem;
public abstract fun initChangesSink (Lapp/cash/redwood/protocol/ChangesSink;)V
public abstract fun nextId-0HhLjSo ()I
Expand All @@ -58,18 +59,22 @@ public final class app/cash/redwood/protocol/guest/ProtocolRedwoodCompositionKt
}

public abstract interface class app/cash/redwood/protocol/guest/ProtocolWidget : app/cash/redwood/widget/Widget {
public abstract fun depthFirstWalk (Lkotlin/jvm/functions/Function3;)V
public abstract fun depthFirstWalk (Lapp/cash/redwood/protocol/guest/ProtocolWidget$ChildrenVisitor;)V
public abstract fun getId-0HhLjSo ()I
public abstract fun getTag-BlhN7y0 ()I
public synthetic fun getValue ()Ljava/lang/Object;
public fun getValue ()Lkotlin/Unit;
public abstract fun sendEvent (Lapp/cash/redwood/protocol/Event;)V
}

public abstract interface class app/cash/redwood/protocol/guest/ProtocolWidget$ChildrenVisitor {
public abstract fun visit-KUkifdM (Lapp/cash/redwood/protocol/guest/ProtocolWidget;ILapp/cash/redwood/protocol/guest/ProtocolWidgetChildren;)V
}

public final class app/cash/redwood/protocol/guest/ProtocolWidgetChildren : app/cash/redwood/widget/Widget$Children {
public static final field $stable I
public synthetic fun <init> (IILapp/cash/redwood/protocol/guest/GuestProtocolAdapter;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun depthFirstWalk (Lapp/cash/redwood/protocol/guest/ProtocolWidget;Lkotlin/jvm/functions/Function3;)V
public final fun depthFirstWalk (Lapp/cash/redwood/protocol/guest/ProtocolWidget;Lapp/cash/redwood/protocol/guest/ProtocolWidget$ChildrenVisitor;)V
public fun detach ()V
public fun getWidgets ()Ljava/util/List;
public fun insert (ILapp/cash/redwood/widget/Widget;)V
Expand Down
64 changes: 36 additions & 28 deletions redwood-protocol-guest/api/redwood-protocol-guest.klib.api
View file

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions ...uest/src/commonMain/kotlin/app/cash/redwood/protocol/guest/DefaultGuestProtocolAdapter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class DefaultGuestProtocolAdapter(
hostVersion: RedwoodVersion,
private val widgetSystemFactory: ProtocolWidgetSystemFactory,
private val mismatchHandler: ProtocolMismatchHandler = ProtocolMismatchHandler.Throwing,
) : GuestProtocolAdapter {
) : GuestProtocolAdapter(hostVersion) {
private var nextValue = Id.Root.value + 1
private val widgets = mutableMapOf<Int, ProtocolWidget>()
private val changes = mutableListOf<Change>()
Expand All @@ -56,8 +56,6 @@ public class DefaultGuestProtocolAdapter(
public override val root: Widget.Children<Unit> =
ProtocolWidgetChildren(Id.Root, ChildrenTag.Root, this)

public override val synthesizeSubtreeRemoval: Boolean = hostVersion < RedwoodVersion("0.10.0-SNAPSHOT")

override fun sendEvent(event: Event) {
val node = widgets[event.id.value]
if (node != null) {
Expand Down
69 changes: 51 additions & 18 deletions ...tocol-guest/src/commonMain/kotlin/app/cash/redwood/protocol/guest/GuestProtocolAdapter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import app.cash.redwood.protocol.ChildrenTag
import app.cash.redwood.protocol.EventSink
import app.cash.redwood.protocol.Id
import app.cash.redwood.protocol.PropertyTag
import app.cash.redwood.protocol.RedwoodVersion
import app.cash.redwood.protocol.WidgetTag
import app.cash.redwood.widget.Widget
import app.cash.redwood.widget.WidgetSystem
Expand All @@ -36,54 +37,56 @@ import kotlinx.serialization.json.Json
*
* This interface is for generated code use only.
*/
public interface GuestProtocolAdapter : EventSink {
public abstract class GuestProtocolAdapter(
hostVersion: RedwoodVersion,
) : EventSink {
@RedwoodCodegenApi
public val json: Json
public abstract val json: Json

/**
* Host versions prior to 0.10.0 contained a bug where they did not recursively remove widgets
* from the protocol map which leaked any child views of a removed node. We can work around this
* on the guest side by synthesizing removes for every node in the subtree.
*/
@RedwoodCodegenApi
public val synthesizeSubtreeRemoval: Boolean
public val synthesizeSubtreeRemoval: Boolean = hostVersion < RedwoodVersion("0.10.0-SNAPSHOT")

/**
* The provider of factories of widgets which record property changes and whose children changes
* are also recorded. You **must** attach returned widgets to [root] or the children of a widget
* in the tree beneath [root] in order for it to be tracked.
*/
public val widgetSystem: WidgetSystem<Unit>
public abstract val widgetSystem: WidgetSystem<Unit>

/**
* The root of the widget tree onto which [widgetSystem]-produced widgets can be added. Changes to
* this instance are recorded as changes to [Id.Root] and [ChildrenTag.Root].
*/
public val root: Widget.Children<Unit>
public abstract val root: Widget.Children<Unit>

public fun initChangesSink(changesSink: ChangesSink)
public abstract fun initChangesSink(changesSink: ChangesSink)

public fun emitChanges()
public abstract fun emitChanges()

@RedwoodCodegenApi
public fun nextId(): Id
public abstract fun nextId(): Id

@RedwoodCodegenApi
public fun appendCreate(
public abstract fun appendCreate(
id: Id,
tag: WidgetTag,
)

@RedwoodCodegenApi
public fun <T> appendPropertyChange(
public abstract fun <T> appendPropertyChange(
id: Id,
tag: PropertyTag,
serializer: KSerializer<T>,
value: T,
)

@RedwoodCodegenApi
public fun appendPropertyChange(
public abstract fun appendPropertyChange(
id: Id,
tag: PropertyTag,
value: Boolean,
Expand All @@ -97,28 +100,28 @@ public interface GuestProtocolAdapter : EventSink {
* https://github.com/Kotlin/kotlinx.serialization/issues/2713
*/
@RedwoodCodegenApi
public fun appendPropertyChange(
public abstract fun appendPropertyChange(
id: Id,
tag: PropertyTag,
value: UInt,
)

@RedwoodCodegenApi
public fun appendModifierChange(
public abstract fun appendModifierChange(
id: Id,
value: Modifier,
)

@RedwoodCodegenApi
public fun appendAdd(
public abstract fun appendAdd(
id: Id,
tag: ChildrenTag,
index: Int,
child: ProtocolWidget,
)

@RedwoodCodegenApi
public fun appendMove(
public abstract fun appendMove(
id: Id,
tag: ChildrenTag,
fromIndex: Int,
Expand All @@ -127,14 +130,44 @@ public interface GuestProtocolAdapter : EventSink {
)

@RedwoodCodegenApi
public fun appendRemove(
public abstract fun appendRemove(
id: Id,
tag: ChildrenTag,
index: Int,
count: Int,
removedIds: List<Id> = listOf(),
removedIds: List<Id>,
)

@RedwoodCodegenApi
public fun removeWidget(id: Id)
public abstract fun removeWidget(id: Id)

@RedwoodCodegenApi
public val childrenVisitor: ProtocolWidget.ChildrenVisitor = if (synthesizeSubtreeRemoval) {
object : ProtocolWidget.ChildrenVisitor {
override fun visit(
parent: ProtocolWidget,
childrenTag: ChildrenTag,
children: ProtocolWidgetChildren,
) {
// This boxes Id values. Don't bother optimizing since it only serves very old hosts.
val childIds = children.widgets.map(ProtocolWidget::id)
for (childId in childIds) {
removeWidget(childId)
}
appendRemove(parent.id, childrenTag, 0, childIds.size, childIds)
}
}
} else {
object : ProtocolWidget.ChildrenVisitor {
override fun visit(
parent: ProtocolWidget,
childrenTag: ChildrenTag,
children: ProtocolWidgetChildren,
) {
for (childWidget in children.widgets) {
removeWidget(childWidget.id)
}
}
}
}
}
12 changes: 7 additions & 5 deletions ...od-protocol-guest/src/commonMain/kotlin/app/cash/redwood/protocol/guest/ProtocolWidget.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,19 @@ public interface ProtocolWidget : Widget<Unit> {
* }
* }
* ```
* You will see the following argument values passed to [block] if invoked on the `Split`:
* You will see the following argument values passed to [visitor] if invoked on the `Split`:
* 1. parent: `Row`, childrenTag: 1, children: `[Text+Button]`
* 2. parent: `Split`, childrenTag: 1, children: `[Row]`
* 3. parent: `Column`, childrenTag: 1, children: `[Button+Text]`
* 4. parent: `Split`, childrenTag: 2, children: `[Column]`
*/
public fun depthFirstWalk(
block: (
public fun depthFirstWalk(visitor: ChildrenVisitor)

public fun interface ChildrenVisitor {
public fun visit(
parent: ProtocolWidget,
childrenTag: ChildrenTag,
children: ProtocolWidgetChildren,
) -> Unit,
)
)
}
}
23 changes: 7 additions & 16 deletions ...col-guest/src/commonMain/kotlin/app/cash/redwood/protocol/guest/ProtocolWidgetChildren.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,32 +38,23 @@ public class ProtocolWidgetChildren(

override fun remove(index: Int, count: Int) {
if (guestAdapter.synthesizeSubtreeRemoval) {
// This boxes Id values. Don't bother optimizing since it only serves very old hosts.
val removedIds = ArrayList<Id>(count)
for (i in index until index + count) {
val widget = _widgets[i]
removedIds += widget.id
guestAdapter.removeWidget(widget.id)

widget.depthFirstWalk { parent, childrenTag, children ->
val childIds = children.widgets.map(ProtocolWidget::id)
for (childId in childIds) {
guestAdapter.removeWidget(childId)
}
guestAdapter.appendRemove(parent.id, childrenTag, 0, childIds.size, childIds)
}
widget.depthFirstWalk(guestAdapter.childrenVisitor)
}
guestAdapter.appendRemove(id, tag, index, count, removedIds)
} else {
for (i in index until index + count) {
val widget = _widgets[i]
guestAdapter.removeWidget(widget.id)
widget.depthFirstWalk { _, _, children ->
for (childWidget in children.widgets) {
guestAdapter.removeWidget(childWidget.id)
}
}
widget.depthFirstWalk(guestAdapter.childrenVisitor)
}
guestAdapter.appendRemove(id, tag, index, count)
guestAdapter.appendRemove(id, tag, index, count, emptyList())
}

_widgets.remove(index, count)
Expand All @@ -79,12 +70,12 @@ public class ProtocolWidgetChildren(

public fun depthFirstWalk(
parent: ProtocolWidget,
block: (ProtocolWidget, ChildrenTag, ProtocolWidgetChildren) -> Unit,
visitor: ProtocolWidget.ChildrenVisitor,
) {
for (widget in widgets) {
widget.depthFirstWalk(block)
widget.depthFirstWalk(visitor)
}
block(parent, tag, this)
visitor.visit(parent, tag, this)
}

override fun detach() {
Expand Down
15 changes: 11 additions & 4 deletions redwood-protocol/api/redwood-protocol.api
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public synthetic class app/cash/redwood/protocol/ChildrenChange$Add$$serializer
}

public final class app/cash/redwood/protocol/ChildrenChange$Add$Companion {
public final fun invoke-11bgaWM (IIII)Lapp/cash/redwood/protocol/ChildrenChange$Add;
public final fun serializer ()Lkotlinx/serialization/KSerializer;
}

Expand Down Expand Up @@ -72,12 +73,12 @@ public synthetic class app/cash/redwood/protocol/ChildrenChange$Move$$serializer
}

public final class app/cash/redwood/protocol/ChildrenChange$Move$Companion {
public final fun invoke-HpxY78w (IIIII)Lapp/cash/redwood/protocol/ChildrenChange$Move;
public final fun serializer ()Lkotlinx/serialization/KSerializer;
}

public final class app/cash/redwood/protocol/ChildrenChange$Remove : app/cash/redwood/protocol/ChildrenChange {
public static final field Companion Lapp/cash/redwood/protocol/ChildrenChange$Remove$Companion;
public synthetic fun <init> (IIIILjava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (IIIILjava/util/List;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun equals (Ljava/lang/Object;)Z
public final fun getCount ()I
Expand All @@ -101,6 +102,8 @@ public synthetic class app/cash/redwood/protocol/ChildrenChange$Remove$$serializ
}

public final class app/cash/redwood/protocol/ChildrenChange$Remove$Companion {
public final fun invoke-HpxY78w (IIIILjava/util/List;)Lapp/cash/redwood/protocol/ChildrenChange$Remove;
public static synthetic fun invoke-HpxY78w$default (Lapp/cash/redwood/protocol/ChildrenChange$Remove$Companion;IIIILjava/util/List;ILjava/lang/Object;)Lapp/cash/redwood/protocol/ChildrenChange$Remove;
public final fun serializer ()Lkotlinx/serialization/KSerializer;
}

Expand Down Expand Up @@ -157,12 +160,12 @@ public synthetic class app/cash/redwood/protocol/Create$$serializer : kotlinx/se
}

public final class app/cash/redwood/protocol/Create$Companion {
public final fun invoke-kyz2zXs (II)Lapp/cash/redwood/protocol/Create;
public final fun serializer ()Lkotlinx/serialization/KSerializer;
}

public final class app/cash/redwood/protocol/Event {
public static final field Companion Lapp/cash/redwood/protocol/Event$Companion;
public synthetic fun <init> (IILjava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (IILjava/util/List;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun equals (Ljava/lang/Object;)Z
public final fun getArgs ()Ljava/util/List;
Expand All @@ -184,6 +187,8 @@ public synthetic class app/cash/redwood/protocol/Event$$serializer : kotlinx/ser
}

public final class app/cash/redwood/protocol/Event$Companion {
public final fun invoke-OtSvs1c (IILjava/util/List;)Lapp/cash/redwood/protocol/Event;
public static synthetic fun invoke-OtSvs1c$default (Lapp/cash/redwood/protocol/Event$Companion;IILjava/util/List;ILjava/lang/Object;)Lapp/cash/redwood/protocol/Event;
public final fun serializer ()Lkotlinx/serialization/KSerializer;
}

Expand Down Expand Up @@ -254,7 +259,6 @@ public final class app/cash/redwood/protocol/Id$Companion {

public final class app/cash/redwood/protocol/ModifierChange : app/cash/redwood/protocol/ValueChange {
public static final field Companion Lapp/cash/redwood/protocol/ModifierChange$Companion;
public synthetic fun <init> (ILjava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (ILjava/util/List;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun equals (Ljava/lang/Object;)Z
public final fun getElements ()Ljava/util/List;
Expand All @@ -275,6 +279,8 @@ public synthetic class app/cash/redwood/protocol/ModifierChange$$serializer : ko
}

public final class app/cash/redwood/protocol/ModifierChange$Companion {
public final fun invoke-z3jyS0k (ILjava/util/List;)Lapp/cash/redwood/protocol/ModifierChange;
public static synthetic fun invoke-z3jyS0k$default (Lapp/cash/redwood/protocol/ModifierChange$Companion;ILjava/util/List;ILjava/lang/Object;)Lapp/cash/redwood/protocol/ModifierChange;
public final fun serializer ()Lkotlinx/serialization/KSerializer;
}

Expand Down Expand Up @@ -320,7 +326,6 @@ public final class app/cash/redwood/protocol/ModifierTag$Companion {

public final class app/cash/redwood/protocol/PropertyChange : app/cash/redwood/protocol/ValueChange {
public static final field Companion Lapp/cash/redwood/protocol/PropertyChange$Companion;
public synthetic fun <init> (IILkotlinx/serialization/json/JsonElement;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (IILkotlinx/serialization/json/JsonElement;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun equals (Ljava/lang/Object;)Z
public fun getId-0HhLjSo ()I
Expand All @@ -342,6 +347,8 @@ public synthetic class app/cash/redwood/protocol/PropertyChange$$serializer : ko
}

public final class app/cash/redwood/protocol/PropertyChange$Companion {
public final fun invoke-e3iP1vo (IILkotlinx/serialization/json/JsonElement;)Lapp/cash/redwood/protocol/PropertyChange;
public static synthetic fun invoke-e3iP1vo$default (Lapp/cash/redwood/protocol/PropertyChange$Companion;IILkotlinx/serialization/json/JsonElement;ILjava/lang/Object;)Lapp/cash/redwood/protocol/PropertyChange;
public final fun serializer ()Lkotlinx/serialization/KSerializer;
}

Expand Down
Loading

0 comments on commit ebdc0b1

Please sign in to comment.