perf: cache sorted Radios list in FlexDiscoveryService

[g4dpz]

Summary

The Radios property previously sorted and allocated a new List<> on every access via LINQ OrderBy/ToList. Since the settings UI may poll this property on layout passes, this caused repeated allocations even when no radios had changed.

Changes

• Cache the sorted list as a volatile field
• Only rebuild when Upsert detects an actual data change or Clear() is called
• Lock-free reads on the happy path (cached list exists); lock only on rebuild

Tests

2 unit tests added:

• Radios_returns_cached_list_until_change — verifies Assert.Same on repeated access, cache survives duplicate datagrams, invalidates on actual change with correct re-sort
• Clear_invalidates_cached_radios — verifies Clear empties and rebuilds

All 5 FlexDiscoveryServiceTests pass.

[magicbug]

Thanks — direction looks good and the tests are welcome. Happy to merge once the cache invalidation race is tightened up.

Issue: rebuild of _cachedRadios happens under _gate (??=), but invalidation in Upsert / Clear sets _cachedRadios = null without that lock. A concurrent reader can rebuild a snapshot, then lose a later invalidation via ??=, leaving a stale sorted list until the next real change.

Please fix so invalidate and rebuild share the same locking (e.g. null the cache under _gate in Upsert/Clear, and keep the double-checked rebuild under _gate only). Happy path can stay lock-free for reads of a non-null cache if you like.

Once that is in, this looks good to merge.

[g4dpz]

Hi, Updated PR no in place. - Dave

…

On 2 Aug 2026, at 15:11, Peter Goodhall ***@***.***> wrote: @magicbug requested changes on this pull request. Thanks — direction looks good and the tests are welcome. Happy to merge once the cache invalidation race is tightened up. Issue: rebuild of _cachedRadios happens under _gate (??=), but invalidation in Upsert / Clear sets _cachedRadios = null without that lock. A concurrent reader can rebuild a snapshot, then lose a later invalidation via ??=, leaving a stale sorted list until the next real change. Please fix so invalidate and rebuild share the same locking (e.g. null the cache under _gate in Upsert/Clear, and keep the double-checked rebuild under _gate only). Happy path can stay lock-free for reads of a non-null cache if you like. Once that is in, this looks good to merge.

[magicbug]

Thanks for the update, Dave.

I had another look at 3bb3860 and the cache invalidation race from the earlier review still looks present. In Upsert / Clear, _cachedRadios is still set to null outside _gate, while rebuild uses _cachedRadios ??= ... under the lock. A concurrent reader can still rebuild a snapshot and then keep a stale sorted list via ??= until the next real change.

Could you please null the cache under _gate in both Upsert and Clear, and keep the double-checked rebuild under _gate only? Happy-path reads of a non-null cache can stay lock-free.

Happy to merge once that is in. CI is already green.

[g4dpz]

Hi, Take a look now - Dave

…

On 3 Aug 2026, at 11:56, Peter Goodhall ***@***.***> wrote: Thanks for the update, Dave. I had another look at 3bb3860 and the cache invalidation race from the earlier review still looks present. In Upsert / Clear, _cachedRadios is still set to null outside _gate, while rebuild uses _cachedRadios ??= ... under the lock. A concurrent reader can still rebuild a snapshot and then keep a stale sorted list via ??= until the next real change. Could you please null the cache under _gate in both Upsert and Clear, and keep the double-checked rebuild under _gate only? Happy-path reads of a non-null cache can stay lock-free. Happy to merge once that is in. CI is already green.