Repository: cashapp/redwood
Pull request #590 (2022-12-05 20:04:56)

Make ID wrap a signed value again
We do not care about the actual values and value classes wrapping value classes causes a serious allocation issue in kotlinx.serialization.

Refs #446 


Patch:
diff --git a/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolBridge.kt b/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolBridge.kt
index dcc3340af..2681439d8 100644
--- a/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolBridge.kt
+++ b/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolBridge.kt
@@ -25,7 +25,7 @@ import app.cash.redwood.protocol.PropertyDiff
 import app.cash.redwood.widget.Widget
 
 public class ProtocolBridge : EventSink {
-  private var nextValue = Id.Root.value + 1U
+  private var nextValue = Id.Root.value + 1L
   private val nodes = mutableMapOf<Id, DiffProducingWidget>()
 
   private var childrenDiffs = mutableListOf<ChildrenDiff>()
@@ -34,7 +34,7 @@ public class ProtocolBridge : EventSink {
 
   public fun nextId(): Id {
     val value = nextValue
-    nextValue = value + 1U
+    nextValue = value + 1L
     return Id(value)
   }
 
diff --git a/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt b/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt
index 6b73c1ab2..c5e332448 100644
--- a/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt
+++ b/redwood-protocol/src/commonMain/kotlin/app/cash/redwood/protocol/protocol.kt
@@ -33,9 +33,9 @@ public fun interface DiffSink {
 /** Identifies a widget instance. */
 @JvmInline
 @Serializable
-public value class Id(public val value: ULong) {
+public value class Id(public val value: Long) {
   public companion object {
-    public val Root: Id = Id(0UL)
+    public val Root: Id = Id(0L)
   }
 }
 


Test patch:
diff --git a/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/DiffProducingWidgetFactoryTest.kt b/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/DiffProducingWidgetFactoryTest.kt
index a5f202d01..842c4de22 100644
--- a/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/DiffProducingWidgetFactoryTest.kt
+++ b/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/DiffProducingWidgetFactoryTest.kt
@@ -49,7 +49,7 @@ class DiffProducingWidgetFactoryTest {
 
     val expected = Diff(
       propertyDiffs = listOf(
-        PropertyDiff(Id(1U), 2U, JsonPrimitive("PT10S")),
+        PropertyDiff(Id(1), 2U, JsonPrimitive("PT10S")),
       ),
     )
     assertEquals(expected, bridge.createDiffOrNull())
@@ -72,7 +72,7 @@ class DiffProducingWidgetFactoryTest {
     val expected = Diff(
       layoutModifiers = listOf(
         LayoutModifiers(
-          Id(1U),
+          Id(1),
           buildJsonArray {
             add(
               buildJsonArray {
@@ -108,7 +108,7 @@ class DiffProducingWidgetFactoryTest {
     val expected = Diff(
       layoutModifiers = listOf(
         LayoutModifiers(
-          Id(1U),
+          Id(1),
           buildJsonArray {
             add(
               buildJsonArray {
@@ -143,7 +143,7 @@ class DiffProducingWidgetFactoryTest {
       argument = it
     }
 
-    diffProducingWidget.sendEvent(Event(Id(1U), 4U, JsonPrimitive("PT10S")))
+    diffProducingWidget.sendEvent(Event(Id(1), 4U, JsonPrimitive("PT10S")))
 
     assertEquals(10.seconds, argument)
   }
@@ -152,7 +152,7 @@ class DiffProducingWidgetFactoryTest {
     val factory = DiffProducingExampleSchemaWidgetFactory(ProtocolBridge())
     val button = factory.Button() as DiffProducingWidget
 
-    val event = Event(Id(1U), 3456543U)
+    val event = Event(Id(1), 3456543U)
     val t = assertFailsWith<IllegalArgumentException> {
       button.sendEvent(event)
     }
@@ -165,7 +165,7 @@ class DiffProducingWidgetFactoryTest {
     val factory = DiffProducingExampleSchemaWidgetFactory(ProtocolBridge(), mismatchHandler = handler)
     val button = factory.Button() as DiffProducingWidget
 
-    button.sendEvent(Event(Id(1U), 3456543U))
+    button.sendEvent(Event(Id(1), 3456543U))
 
     assertEquals("Unknown event 3456543 for 4", handler.events.single())
   }
diff --git a/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/ProtocolTest.kt b/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/ProtocolTest.kt
index 5de2dd34a..e377bc0d1 100644
--- a/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/ProtocolTest.kt
+++ b/redwood-protocol-compose/src/commonTest/kotlin/app/cash/redwood/protocol/compose/ProtocolTest.kt
@@ -84,20 +84,20 @@ class ProtocolTest {
     assertEquals(
       Diff(
         childrenDiffs = listOf(
-          ChildrenDiff.Insert(Id.Root, RootChildrenTag, Id(1U), 1 /* row */, 0),
-          ChildrenDiff.Insert(Id(1U), 1U, Id(2U), 3 /* text */, 0),
-          ChildrenDiff.Insert(Id(1U), 1U, Id(3U), 1 /* row */, 1),
-          ChildrenDiff.Insert(Id(3U), 1U, Id(4U), 3 /* text */, 0),
+          ChildrenDiff.Insert(Id.Root, RootChildrenTag, Id(1), 1 /* row */, 0),
+          ChildrenDiff.Insert(Id(1), 1U, Id(2), 3 /* text */, 0),
+          ChildrenDiff.Insert(Id(1), 1U, Id(3), 1 /* row */, 1),
+          ChildrenDiff.Insert(Id(3), 1U, Id(4), 3 /* text */, 0),
         ),
         layoutModifiers = listOf(
-          LayoutModifiers(Id(1U), JsonArray(listOf())),
-          LayoutModifiers(Id(2U), JsonArray(listOf())),
-          LayoutModifiers(Id(3U), JsonArray(listOf())),
-          LayoutModifiers(Id(4U), JsonArray(listOf())),
+          LayoutModifiers(Id(1), JsonArray(listOf())),
+          LayoutModifiers(Id(2), JsonArray(listOf())),
+          LayoutModifiers(Id(3), JsonArray(listOf())),
+          LayoutModifiers(Id(4), JsonArray(listOf())),
         ),
         propertyDiffs = listOf(
-          PropertyDiff(Id(2U), 1U /* text */, JsonPrimitive("hey")),
-          PropertyDiff(Id(4U), 1U /* text */, JsonPrimitive("hello")),
+          PropertyDiff(Id(2), 1U /* text */, JsonPrimitive("hey")),
+          PropertyDiff(Id(4), 1U /* text */, JsonPrimitive("hello")),
         ),
       ),
       diffs.removeFirst(),
@@ -135,43 +135,43 @@ class ProtocolTest {
     assertEquals(
       Diff(
         childrenDiffs = listOf(
-          ChildrenDiff.Insert(Id.Root, RootChildrenTag, Id(1U), 4 /* button */, 0),
+          ChildrenDiff.Insert(Id.Root, RootChildrenTag, Id(1), 4 /* button */, 0),
         ),
         layoutModifiers = listOf(
-          LayoutModifiers(Id(1U), JsonArray(listOf())),
+          LayoutModifiers(Id(1), JsonArray(listOf())),
         ),
         propertyDiffs = listOf(
-          PropertyDiff(Id(1U), 1U /* text */, JsonPrimitive("state: 0")),
-          PropertyDiff(Id(1U), 2U /* onClick */, JsonPrimitive(true)),
+          PropertyDiff(Id(1), 1U /* text */, JsonPrimitive("state: 0")),
+          PropertyDiff(Id(1), 2U /* onClick */, JsonPrimitive(true)),
         ),
       ),
       diffs.removeFirst(),
     )
 
     // Invoke the onClick lambda to move the state from 0 to 1.
-    bridge.sendEvent(Event(Id(1U), 2U))
+    bridge.sendEvent(Event(Id(1), 2U))
     yield() // Allow state change to be handled.
 
     clock.awaitFrame()
     assertEquals(
       Diff(
         propertyDiffs = listOf(
-          PropertyDiff(Id(1U), 1U /* text */, JsonPrimitive("state: 1")),
+          PropertyDiff(Id(1), 1U /* text */, JsonPrimitive("state: 1")),
         ),
       ),
       diffs.removeFirst(),
     )
 
     // Invoke the onClick lambda to move the state from 1 to 2.
-    bridge.sendEvent(Event(Id(1U), 2U))
+    bridge.sendEvent(Event(Id(1), 2U))
     yield() // Allow state change to be handled.
 
     clock.awaitFrame()
     assertEquals(
       Diff(
         propertyDiffs = listOf(
-          PropertyDiff(Id(1U), 1U /* text */, JsonPrimitive("state: 2")),
-          PropertyDiff(Id(1U), 2U /* text */, JsonPrimitive(false)),
+          PropertyDiff(Id(1), 1U /* text */, JsonPrimitive("state: 2")),
+          PropertyDiff(Id(1), 2U /* text */, JsonPrimitive(false)),
         ),
       ),
       diffs.removeFirst(),
@@ -185,7 +185,7 @@ class ProtocolTest {
     assertEquals(
       Diff(
         propertyDiffs = listOf(
-          PropertyDiff(Id(1U), 1U /* text */, JsonPrimitive("state: 3")),
+          PropertyDiff(Id(1), 1U /* text */, JsonPrimitive("state: 3")),
         ),
       ),
       diffs.removeFirst(),
diff --git a/redwood-protocol-widget/src/commonTest/kotlin/app/cash/redwood/protocol/widget/DiffConsumingWidgetFactoryTest.kt b/redwood-protocol-widget/src/commonTest/kotlin/app/cash/redwood/protocol/widget/DiffConsumingWidgetFactoryTest.kt
index 4af336df6..0d3ac43a0 100644
--- a/redwood-protocol-widget/src/commonTest/kotlin/app/cash/redwood/protocol/widget/DiffConsumingWidgetFactoryTest.kt
+++ b/redwood-protocol-widget/src/commonTest/kotlin/app/cash/redwood/protocol/widget/DiffConsumingWidgetFactoryTest.kt
@@ -229,7 +229,7 @@ class DiffConsumingWidgetFactoryTest {
     val textInput = factory.create(Id.Root, ThrowingWidgetChildren(), 5)!!
 
     val throwingEventSink = EventSink { error(it) }
-    textInput.apply(PropertyDiff(Id(1U), 2U, JsonPrimitive("PT10S")), throwingEventSink)
+    textInput.apply(PropertyDiff(Id(1), 2U, JsonPrimitive("PT10S")), throwingEventSink)
 
     assertEquals(10.seconds, recordingTextInput.customType)
   }
@@ -238,7 +238,7 @@ class DiffConsumingWidgetFactoryTest {
     val factory = DiffConsumingExampleSchemaWidgetFactory(EmptyExampleSchemaWidgetFactory())
     val button = factory.create(Id.Root, ThrowingWidgetChildren(), 4)!!
 
-    val diff = PropertyDiff(Id(1U), 345432U)
+    val diff = PropertyDiff(Id(1), 345432U)
     val eventSink = EventSink { throw UnsupportedOperationException() }
     val t = assertFailsWith<IllegalArgumentException> {
       button.apply(diff, eventSink)
@@ -254,7 +254,7 @@ class DiffConsumingWidgetFactoryTest {
     )
     val button = factory.create(Id.Root, ThrowingWidgetChildren(), 4)!!
 
-    button.apply(PropertyDiff(Id(1U), 345432U)) { throw UnsupportedOperationException() }
+    button.apply(PropertyDiff(Id(1), 345432U)) { throw UnsupportedOperationException() }
 
     assertEquals("Unknown property 345432 for 4", handler.events.single())
   }
@@ -275,11 +275,11 @@ class DiffConsumingWidgetFactoryTest {
     val textInput = factory.create(Id.Root, ThrowingWidgetChildren(), 5)!!
 
     val eventSink = RecordingEventSink()
-    textInput.apply(PropertyDiff(Id(1U), 4U, JsonPrimitive(true)), eventSink)
+    textInput.apply(PropertyDiff(Id(1), 4U, JsonPrimitive(true)), eventSink)
 
     recordingTextInput.onChangeCustomType!!.invoke(10.seconds)
 
-    assertEquals(Event(Id(1U), 4U, JsonPrimitive("PT10S")), eventSink.events.single())
+    assertEquals(Event(Id(1), 4U, JsonPrimitive("PT10S")), eventSink.events.single())
   }
 
   class RecordingTextInput : TextInput<Nothing> {
diff --git a/redwood-protocol-widget/src/commonTest/kotlin/app/cash/redwood/protocol/widget/ProtocolBridgeTest.kt b/redwood-protocol-widget/src/commonTest/kotlin/app/cash/redwood/protocol/widget/ProtocolBridgeTest.kt
index 504fe697d..852cd5f85 100644
--- a/redwood-protocol-widget/src/commonTest/kotlin/app/cash/redwood/protocol/widget/ProtocolBridgeTest.kt
+++ b/redwood-protocol-widget/src/commonTest/kotlin/app/cash/redwood/protocol/widget/ProtocolBridgeTest.kt
@@ -60,7 +60,7 @@ class ProtocolBridgeTest {
         ChildrenDiff.Insert(
           id = Id.Root,
           tag = RootChildrenTag,
-          childId = Id(1U),
+          childId = Id(1),
           kind = 4 /* button */,
           index = 0,
         ),
diff --git a/redwood-protocol/src/commonTest/kotlin/app/cash/redwood/protocol/ProtocolTest.kt b/redwood-protocol/src/commonTest/kotlin/app/cash/redwood/protocol/ProtocolTest.kt
index 84297272f..d21f8ff74 100644
--- a/redwood-protocol/src/commonTest/kotlin/app/cash/redwood/protocol/ProtocolTest.kt
+++ b/redwood-protocol/src/commonTest/kotlin/app/cash/redwood/protocol/ProtocolTest.kt
@@ -30,13 +30,13 @@ class ProtocolTest {
   }
 
   @Test fun eventNonNullValue() {
-    val model = Event(Id(1U), 2U, JsonPrimitive("Hello"))
+    val model = Event(Id(1), 2U, JsonPrimitive("Hello"))
     val json = """{"id":1,"tag":2,"value":"Hello"}"""
     assertJsonRoundtrip(Event.serializer(), model, json)
   }
 
   @Test fun eventNullValue() {
-    val model = Event(Id(1U), 2U)
+    val model = Event(Id(1), 2U)
     val json = """{"id":1,"tag":2}"""
     assertJsonRoundtrip(Event.serializer(), model, json)
   }
@@ -44,13 +44,13 @@ class ProtocolTest {
   @Test fun diff() {
     val model = Diff(
       childrenDiffs = listOf(
-        ChildrenDiff.Insert(Id(1U), 2U, Id(3U), 4, 5),
-        ChildrenDiff.Move(Id(1U), 2U, 3, 4, 5),
-        ChildrenDiff.Remove(Id(1U), 2U, 3, 4),
+        ChildrenDiff.Insert(Id(1), 2U, Id(3), 4, 5),
+        ChildrenDiff.Move(Id(1), 2U, 3, 4, 5),
+        ChildrenDiff.Remove(Id(1), 2U, 3, 4),
       ),
       layoutModifiers = listOf(
         LayoutModifiers(
-          Id(1U),
+          Id(1),
           buildJsonArray {
             add(
               buildJsonArray {
@@ -62,8 +62,8 @@ class ProtocolTest {
         ),
       ),
       propertyDiffs = listOf(
-        PropertyDiff(Id(1U), 2U, JsonPrimitive("Hello")),
-        PropertyDiff(Id(1U), 2U, JsonNull),
+        PropertyDiff(Id(1), 2U, JsonPrimitive("Hello")),
+        PropertyDiff(Id(1), 2U, JsonNull),
       ),
     )
     val json = "" +