Skip to content

client: drop-policy classification for RdpOutputEvent + drop-oldest/coalesce channel wrapper #1330

Description

Benoît Cortier (@CBenoit): picking up the invitation from #1309.

Background

#1309's reply on the bounded-channel discussion left the snapshot-only-keeps-latest improvement for a future PR, with the constraint that the latest snapshot must properly reflect correct state. This issue proposes a shape and requests maintainer signal before any code lands.

The current output_event_sender: mpsc::Sender<RdpOutputEvent> in crates/ironrdp-client/src/rdp.rs uses bounded capacity with .await backpressure on every send (post-#1309). That preserves correctness but pays full backpressure cost on every framebuffer update, even when intermediate updates would be subsumed by the next one.

Proposal

Tag each RdpOutputEvent variant with a DropPolicy and dispatch sends through a wrapper that consults the policy:

Policy Behavior Candidate variants
MustDeliver .await always ConnectionFailure, Terminated
DropOldest try_send; if full, discard oldest queued instance of the same variant, then send PointerPosition, PointerDefault, PointerHidden
Coalesce Receiver-side accumulator merges queued + incoming into a single dirty-region snapshot Image
Replace try_send; if full, replace the previous queued instance of the same variant outright PointerBitmap

The load-bearing case is Coalesce on Image. Naive drop-oldest on framebuffer regions corrupts the destination because region updates are stateful relative to the previous frame. Coalesce maintains a "current dirty region" accumulator (union of dirty rects, latest pixel values per region) that gets delivered as one snapshot when the consumer wakes. The accumulator design is what the correctness constraint on the original invitation requires.

Open questions

  1. Coalesce shape. Receiver-side accumulator vs sender-side merge. Sender-side is simpler (no separate receiver state) but requires the sender to know the previous queued payload. Receiver-side keeps the channel event-typed but adds an accumulator owned by the consumer. Maintainer preference?
  2. PointerBitmap policy. Arc<DecodedPointer> is cheap to clone. Replace versus a fixed-slot coalesce. Either works; preference?
  3. PointerDefault / PointerHidden. These are cursor-state transitions, conceptually stateful, but the latest one is what matters. DropOldest covers them. Acceptable, or should they be MustDeliver?
  4. Library boundary. New module crates/ironrdp-client/src/output_channel.rs (or similar), wrapper type around mpsc::Sender<RdpOutputEvent>, public API exposed via the library-side surface rather than the raw channel handle. Acceptable shape?
  5. DropPolicy placement. A method RdpOutputEvent::drop_policy(&self) -> DropPolicy keeps the classification next to the variant definition. A const table at the wrapper side keeps the enum pure. Either works; preference?

Measurement plan (prerequisite for the implementation PR)

The implementation PR will include data; this issue commits to that.

  • Workload: chatty server emitting fast framebuffer updates (rapid scrolling, animated content, or video playback). Either a real Windows host or a server-side IronRDP instance running on hardware sufficient to saturate the channel.
  • Client side: ironrdp-viewer instrumented to record per-event send timestamps, per-event consumer-receive timestamps, queue-depth at send time, and dropped/coalesced event counts.
  • Metrics: P50 / P99 / P99.9 end-to-end framebuffer latency (server frame produced to viewer rendered). Cursor position end-to-end latency for PointerPosition traffic. Total event count delivered vs total event count produced. CPU and memory deltas.
  • Comparator: current .await backpressure (master) vs proposed DropPolicy wrapper, same workload, same hardware.
  • Acceptance bar: implementation PR ships only if P99 framebuffer latency improves materially under burst load AND no rendering correctness regression appears in the coalesce path.

If the preferred sequence is data-before-code, the implementation PR holds until the baseline is recorded. If code-first-with-data-following is preferred, the wrapper can ship behind a feature flag or as an off-by-default opt-in path with data following.

Out of scope for this issue

  • Multi-monitor framebuffer state semantics under coalesce. The accumulator design here is single-buffer; multi-monitor needs a separate pass once the single-buffer shape settles.
  • Workloads beyond framebuffer + cursor. Typing latency, RDP audio sync, clipboard latency can become follow-up workloads if the initial measurement raises specific concerns.
  • RdpInputEvent direction. The current input channel is mpsc::UnboundedSender<RdpInputEvent>; whether it deserves the same treatment is a separate discussion.

Refs #1309.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions