Skip to content

Open the tray menu without a socket round trip - #860

Merged
SimonCropp merged 3 commits into
mainfrom
tray-menu-open-without-io
Aug 31, 2026
Merged

Open the tray menu without a socket round trip#860
SimonCropp merged 3 commits into
mainfrom
tray-menu-open-without-io

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

Right clicking the tray icon took half a second whenever a viewer owned the inline queue and had since exited.

The menu is built on the UI thread inside ContextMenuStrip.Opening, and BuildTrackingMenuItems read Tracker.Snapshots, which read the queue live — a loopback exchange bounded by ViewerClient.ShortTimeout. Measured from the shell's own notify icon message to the menu being open:

arrangement click 1 click 2 click 3 click 4
tray owns the queue 26 ms 9.5 ms 5.5 ms 3.1 ms
tray driving a remote owner 531 ms 515 ms 513 ms 513 ms

~511 ms of each slow row is the one inline.List() call, and it is every click rather than the first.

The menu reads the cache

Snapshots is the scan cache now. Nothing is lost where the queue is held here, since OwnedInlineHost.Changed already runs Refresh on every mutation and the tray's own accepts and discards refresh too, so the cache is the live queue. Where a viewer holds it the listing is at most one scan old — which is what TrackingAny and the icon have always shown, and what MenuBuilder already said it was showing.

The Tracker seeds the cache in its constructor rather than leaving it empty until the first scan two seconds later, so a tray that has just started does not show an empty menu over a queue that is not.

AcceptAllSnapshots keeps its live read, because that guard exists precisely so a stale empty cache cannot make it silently do nothing, but takes it inside the worker rather than in front of it: the caller is a click or a hot key and the read is a round trip.

Connection refused is not always fast

A connection to a port nothing is listening on is supposed to be refused at once, and every caller in RemoteInlineHost was written expecting it — "a refused connection means the viewer has gone". It is not refused at once everywhere: where the SYN is dropped rather than answered with a reset, the connect runs to its timeout instead. Measured on one machine:

  • closed loopback port, 500 ms cap: 503 ms median — ports 3492, 3493 and randomly chosen unused ones alike
  • same connect uncapped: ConnectionRefused after 2034 ms
  • identical for a dual-mode socket and an IPv4-only one, so not an address-family artifact

Ownership is decided at startup and this host is never replaced, so that was the price of every scan and every menu verb for the rest of the tray's life.

Exchange asks the OS whether anything holds the port before connecting. The listener table is a local query costing well under a millisecond. Cheaper than a backoff and with no staleness window: a viewer that starts is found on the next call, and racing the check costs no more than the connect always did.

Verified

Same instrumented right-click, unresponsive owner holding 3493 — 531/515/513/513 ms becomes 20.5/1.3/1.5/2.0 ms.

And the scan path, within a single run where the owner held the port and then exited:

scan: inline.List() took 512.6 ms     <- owner still holding the port
scan: inline.List() took 1.1 ms       <- owner gone, listener-table check short-circuits

Full solution: 1905 tests, 0 failed, 20 skipped.

Tests

TrayViewerSyncTest's two pair helpers gained a Listing that refreshes before reading, standing in for the scan timer. That is also what ViewerAcceptAllEmptiesTheTrayListing needed to be a test at all: it asserts an empty listing, and a cache satisfies that whether or not anything worked.

Right clicking the tray icon took half a second whenever a viewer owned the
inline queue and had since exited. The menu is built on the UI thread inside
ContextMenuStrip.Opening, and BuildTrackingMenuItems read Tracker.Snapshots,
which read the queue live - a loopback exchange bounded by
ViewerClient.ShortTimeout. Measured from the shell's own notify icon message to
the menu being open: 3-26ms with the queue held here, 513ms without, and on
every click rather than the first.

Snapshots is the scan cache now. Nothing is lost where the queue is held here,
since OwnedInlineHost.Changed already runs Refresh on every mutation and the
tray's own accepts and discards refresh too, so the cache is the live queue.
Where a viewer holds it the listing is at most one scan old, which is what
TrackingAny and the icon have always shown - and what MenuBuilder already said
it was showing. The Tracker seeds the cache in its constructor rather than
leaving it empty until the first scan two seconds later, so a tray that has just
started does not show an empty menu over a queue that is not.

AcceptAllSnapshots keeps its live read, because that guard exists precisely so a
stale empty cache cannot make it silently do nothing, but takes it inside the
worker rather than in front of it: the caller is a click or a hot key and the
read is a round trip.

The half second itself is the other half of this. A connection to a port nothing
is listening on is supposed to be refused at once, and every caller in
RemoteInlineHost was written expecting it - "a refused connection means the
viewer has gone". It is not refused at once everywhere: where the SYN is dropped
rather than answered with a reset, the connect runs to its timeout instead. On
one machine a closed loopback port costs 503ms against a 500ms cap, and 2034ms
uncapped, the same for a dual mode socket and an IPv4 one. Ownership is decided
at startup and this host is never replaced, so that was the price of every scan
and every menu verb for the rest of the tray's life.

Exchange asks the OS whether anything holds the port before connecting. The
listener table is a local query costing well under a millisecond, and it turns
the gone owner case from 513ms into 1.1ms. Cheaper than a backoff and with no
staleness window: a viewer that starts is found on the next call, and racing the
check costs no more than the connect always did.

TrayViewerSyncTest's two pair helpers gained a Listing that refreshes before
reading, standing in for the scan timer. That is also what
ViewerAcceptAllEmptiesTheTrayListing needed to be a test at all: it asserts an
empty listing, and a cache satisfies that whether or not anything worked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant