Devolutions / Devolutions/IronRDP
Devilish resize bug
- Dominant language
- Rust
- Stars
- 3.2k
- Forks
- 275
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 189
Description
## Background
After adding protocol level support for dynamic resizes via the `DisplayControl` DVC in
- https://github.com/Devolutions/IronRDP/pull/418
- https://github.com/Devolutions/IronRDP/pull/419
- https://github.com/Devolutions/IronRDP/pull/425
this was all actually hooked up in the `ironrdp-client` in https://github.com/Devolutions/IronRDP/pull/430. The initial attempt succeeded in allowing for dynamic resize, however after resizes performance of the client noticeably degraded after a resize or two. **This issue does not reproduce with FreeRDP**, meaning that it's a bug in IronRDP.
## Pointer settings and FreeRDP alignment
We tried many things to resolve the issue including:
- all variations of pointer settings in IronRDP
- Meticulously aligning IronRDP's [Capability Sets](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/7cc40be0-ceb0-4983-85d0-e08af4bec2a6) with FreeRDP's
- Meticulously aligning IronRDP's [Client MCS Connect Initial PDU with GCC Conference Create Request](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/db6713ee-1c0e-4064-a3b3-0fac30b4037b) with FreeRDP's
(IronRDP rev `f7b4f546650231ce345e9ee67f6ad29b2b93f937` is aligned with FreeRDP `ba8cf8cf2158018fb7abbedb51ab245f369be813`)
all to no avail.
We know the problem is related to the RemoteFX codepath -- when we switched to using bitmaps, it went away.
## Profiling
Comparing [flamegraphs](https://crates.io/crates/flamegraph) between an IronRDP session with a resize vs without did not reveal anything of relevance to this issue: [Download](https://github.com/Devolutions/IronRDP/files/15100279/ironrdp-profiling.zip).
The absolute time of frame processing (basically how long [active_stage.process(&mut image, action, &payload)?](https://github.com/Devolutions/IronRDP/blob/8a5acc19b8b729baf3a38a60e801eef1fb4aaa25/crates/ironrdp-client/src/rdp.rs#L183) takes to run) was measured at <1ms, before and after resize. However, after a resize, the average time between frames (how often [frame = framed.read_pdu() => {](https://github.com/Devolutions/IronRDP/blob/8a5acc19b8b729baf3a38a60e801eef1fb4aaa25/crates/ironrdp-client/src/rdp.rs#L179) gets called) goes up 4-5x, 30ms vs 140-150ms).
## Temp Solution
What finally gave us a temporary solution was upping the [`maxUnacknowledgedFrameCount`](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdprfx/e4d498fd-822b-408d-b8b3-1c216f21265b) from `2` to `>= 10` (credit to @probakowski for discovering this). We know that this is not any sort of root cause, because FreeRDP uses `2` for this field

#### Implications, what to examine next
The `maxUnacknowledgedFrameCount` plays a role in when the server sends us frame updates: after sending us `maxUnacknowledgedFrameCount` frames, it waits for an ACK response before sending the next frame. The fact that bumping this value more or less solves the performance issue suggests that the holdup is related to that mechanism. Logically this seems to imply that the problem is either
1. IronRDP is taking too long to reply to frames with an ACK (after resize)
- Although processing and encoding the response (via [active_stage.process(&mut image, action, &payload)?](https://github.com/Devolutions/IronRDP/blob/8a5acc19b8b729baf3a38a60e801eef1fb4aaa25/crates/ironrdp-client/src/rdp.rs#L183)) doesn't _appear_ to change much per the note in the **Profiling** section above, there may yet be issues due to either
1. [The actual act of writing the response frame](https://github.com/Devolutions/IronRDP/blob/8a5acc19b8b729baf3a38a60e801eef1fb4aaa25/crates/ironrdp-client/src/rdp.rs#L257-L260), whose performance has not been measured.
2. Perhaps there's a substantial difference within the `<1ms` timeframe of [active_stage.process(&mut image, action, &payload)?](https://github.com/Devolutions/IronRDP/blob/8a5acc19b8b729baf3a38a60e801eef1fb4aaa25/crates/ironrdp-client/src/rdp.rs#L183) before and after resize, and that timescale is noticeable to the naked eye wrt performance.
2. The RDP server is taking too long to process the ACKs after resize.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.