Interaction latency rules #38

pyricau · 2022-09-29T00:06:11Z

Redesigning the API and implementation for tracking interaction latency, with a primary goal of decoupling the code where we send signals (events) from the place where we tie those events together an interaction.

Example usage code

interface PosInteractionEvent
interface PosInteraction

object OpenCheckoutApplet : PosInteraction

class Navigation(val destination: Any) : PosInteractionEvent

class CheckoutApplet : PosInteraction

object TapItem : PosInteractionEvent {
  val itemId = ""
}

object ItemVisibleInCart : PosInteractionEvent {
  val itemId = ""
}

class AddItemToCart(val itemId: String) : PosInteraction

object KeypadVisible : PosInteractionEvent
object UserPressedBack : PosInteractionEvent

fun setup() {

  val client = InteractionRuleClient<PosInteractionEvent, PosInteraction>()

  client.sendEvent(KeypadVisible)
  client.sendEvent(Navigation(Any()))

  val configuredRule = client.addInteractionRule<OpenCheckoutApplet> {
    onEvent<Navigation> { navigation ->
      if (navigation.destination is CheckoutApplet) {
        cancelRunningInteractions()
        startInteraction(OpenCheckoutApplet, onCancel = {
          Log.d("event canceled", it.cancelReason)
        })
      }
    }
    onEvent<UserPressedBack> {
      runningInteractions().singleOrNull()?.cancel("user pressed back")
    }
    onEvent<KeypadVisible> {
      runningInteractions().singleOrNull()?.finishOnFrameRendered { result ->
        logToAnalytics(result)
      }
    }
  }

  client.addInteractionRule<AddItemToCart> {
    onEvent<TapItem> { tapItem ->
      startInteraction(AddItemToCart(tapItem.itemId))
    }
    onEvent<ItemVisibleInCart> { itemVisibleInCart ->
      val addItemToCart = runningInteractions().firstOrNull {
        it.interaction.itemId == itemVisibleInCart.itemId
      }
      addItemToCart?.let { interaction ->
        interaction.finishOnFrameRendered { result ->
          logToAnalytics(result)
        }
      }
    }
  }

  // when we don't need this rule anymore
  configuredRule.remove()
}

pyricau · 2022-09-29T00:09:05Z

papa/src/main/java/papa/InputTracker.kt

@@ -36,5 +39,35 @@ interface InputTracker {

 class DeliveredInput<T : InputEvent>(
  val event: T,
-  val deliveryUptimeMillis: Long
-)
+  val deliveryUptime: Duration,


All these years I had no idea we had a Duration class in Kotlin (and also another one in Java!!)

papa/src/main/java/papa/internal/RealInputTracker.kt

rjrjr · 2022-09-29T16:04:10Z

papa/src/main/java/papa/InputTracker.kt

+import kotlin.time.Duration
+import kotlin.time.Duration.Companion.milliseconds
+import kotlin.time.DurationUnit.MILLISECONDS


@0legg Should PY be using 310 alternatives here?

I'm really taking a liking to Kotlin's duration class, looks nicer than the JDK ones.

jeffhashfield · 2022-09-29T22:00:00Z

sample/src/main/java/com/example/papa/MainActivity.kt

-    val stateHolder = PerformanceMetricsState.getForHierarchy(textView).state!!
-    stateHolder.addState("textview", "updated at $date")
+    interactionEventReceiver.sendEvent(OnMainActivityButtonClick(element, previousText, newText))
+    // val stateHolder = PerformanceMetricsState.getForHierarchy(textView).state!!


Nit: Remove commented code?

jeffhashfield · 2022-09-29T22:02:17Z

papa/src/main/java/papa/internal/RealInputTracker.kt


  override val currentKeyEvent: DeliveredInput<KeyEvent>?
    get() = currentKeyEventLocal.get()

-  private val motionEventTriggeringClickLocal = ThreadLocal<DeliveredInput<MotionEvent>>()
+  /**
+   * The thread locals here aren't in charge of actually holding the event holder for the duration


Nit: This would be easier to read if it were broken up into multiple sentences.

jeffhashfield · 2022-09-29T22:02:46Z

papa/src/main/java/papa/internal/RealInputTracker.kt

+   * The thread locals here aren't in charge of actually holding the event holder for the duration
+   * of its lifecycle, but instead of holding the event in specific time spans (wrapping onClick
+   * callback invocations) so that we can reach back here from an onClick and capture the event but
+   * also ensure that if we reach back outside of an onClick sandwich we actually find nothing


Nit: find nothing event if overall -> Seems like it's missing a word or two in there?

jeffhashfield · 2022-09-29T22:03:39Z

papa/src/main/java/papa/internal/RealInputTracker.kt

+      // if this event is consumed as part of the frame we did the increment in.
+      // There's a slight edge case: if the event consumption triggered in between doFrame and
+      // the post at front of queue, the count would be short by 1. We can live with this, it's
+      // unlikely to happen unless and even is triggered from a postAtFront.


Nit: and even -> an event?

jeffhashfield · 2022-09-29T22:04:15Z

papa/src/main/java/papa/internal/RealInputTracker.kt

+          //  holder. Why replace it? Because we want to increase the frame count over time, but we
+          //  want to do that by swapping an immutable event, so that if we capture such event at
+          //  time N and then the count gets updated at N + 1, the count update isn't reflected in
+          //  the code that captured the event a time N.


Nit: a time -> at time

jeffhashfield · 2022-09-29T22:07:15Z

papa/src/main/java/papa/InputTracker.kt

+  val event: InputEventType,
+  val deliveryUptime: Duration,
+  val framesSinceDelivery: Int,
+  private var endTrace: ((() -> Unit))?


Nit: Is there an unnecessary set of parentheses there?

jeffhashfield · 2022-09-29T22:14:55Z

papa/src/main/java/papa/InputTracker.kt

+) {
+
+  val eventUptime: Duration
+    get() = event.eventTime.milliseconds


Nit: Does this need to be a custom getter, or can it just be initialized? Can event.eventTime.milliseconds change?

It won't change, but as a general practice I avoid creating backing fields when delegation works just fine and is cheap, especially if the backing field might never be actually read.

jeffhashfield · 2022-09-29T22:22:06Z

papa/src/main/java/papa/InteractionRuleClient.kt

+    private val onCancel: (CanceledInteractionResult<InteractionType>) -> Unit
+  ) : RunningInteraction<InteractionType>, FinishingInteraction<InteractionType>, FrameCallback {
+
+    private var frameCount: Int = if (isChoreographerDoingFrame()) {


Nit: One line without braces?

jeffhashfield · 2022-09-29T22:26:04Z

papa/src/main/java/papa/InteractionRuleClient.kt

+  override fun sendEvent(event: EventType) {
+    val eventSentUptime = System.nanoTime().nanoseconds
+    if (isMainThread) {
+      for (engine in interactionEngines) {


Nit: interactionEngines.forEach?

I find traditional for loops a bit more readable but to be honest it's not like a strongly informed opinion.

jeffhashfield · 2022-09-29T22:26:51Z

papa/src/main/java/papa/InteractionRuleClient.kt

+    } else {
+      mainHandler.post {
+        for (engine in interactionEngines) {
+          engine.sendEvent(event, eventSentUptime, null)


Why is the interactionInput parameter null?

pyricau added 6 commits September 27, 2022 13:21

Add new way to track interaction latency

4fed0f8

Switch from long to Duration for choreographer and interaction client

cbb1d51

format

b0eb7a3

API updates

ac322bb

Remove old interaction latency & update example code

d03ab4b

Remove example code

74bdd98

pyricau commented Sep 29, 2022

View reviewed changes

Removing one more forgotten class

edb6ce2

pyricau commented Sep 29, 2022

View reviewed changes

papa/src/main/java/papa/internal/RealInputTracker.kt Show resolved Hide resolved

pyricau commented Sep 29, 2022

View reviewed changes

papa/src/main/java/papa/internal/RealInputTracker.kt Show resolved Hide resolved

Cleanup RealInputTracker

039a96f

pyricau mentioned this pull request Sep 29, 2022

Track number of frames for delayed interactions #29

Closed

pyricau merged commit b172be8 into main Sep 29, 2022

rjrjr reviewed Sep 29, 2022

View reviewed changes

jeffhashfield reviewed Sep 29, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interaction latency rules #38

Interaction latency rules #38

pyricau commented Sep 29, 2022

pyricau Sep 29, 2022

rjrjr Sep 29, 2022

pyricau Sep 30, 2022

jeffhashfield Sep 29, 2022

jeffhashfield Sep 29, 2022

jeffhashfield Sep 29, 2022

jeffhashfield Sep 29, 2022

jeffhashfield Sep 29, 2022

jeffhashfield Sep 29, 2022

jeffhashfield Sep 29, 2022

pyricau Sep 30, 2022

jeffhashfield Sep 29, 2022

jeffhashfield Sep 29, 2022

pyricau Sep 30, 2022

jeffhashfield Sep 29, 2022

Interaction latency rules #38

Interaction latency rules #38

Conversation

pyricau commented Sep 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment