[Repo Assist] perf(shared): eliminate LINQ sort and repeated Contains calls in ChannelsAggregator#532
Conversation
…nelsAggregator ChannelsAggregator.Aggregate() is called on every gateway channel status update pushed to the UI. Two small allocation sources were unnecessary: 1. QrLinkChannels.Contains(id) was called twice per channel (once for CanShowQr/CanRelink and once for CanStop). Hoist to a local bool. 2. The final sort used LINQ OrderByDescending/ThenBy/ToList, which creates an IOrderedEnumerable wrapper and then materialises a second List<T>. Replace with List<T>.Sort() (in-place, no extra allocation) using a static Comparison<T> delegate. No behaviour change — the sort order (configured-first, then by SortOrder) is identical. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Codex review: needs maintainer review before merge. Latest ClawSweeper review: 2026-05-24 13:05 UTC / May 24, 2026, 9:05 AM ET. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: not applicable. this is a pull-request cleanup review rather than a bug report. The relevant source check is that current main still uses LINQ ordering and repeated PR rating Rank-up moves:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land the small allocation cleanup if maintainers are comfortable after the repository’s required build and test validation. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a pull-request cleanup review rather than a bug report. The relevant source check is that current main still uses LINQ ordering and repeated Is this the best way to solve the issue? Yes: caching the QR lookup and sorting the already-built list in place is a narrow maintainable performance improvement because Label justifications:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against ef6ac8acbab2. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
ClawSweeper PR egg ✨ Hatched: 🌱 uncommon Frosted Proofling Hatch commandComment Hatchability rules:
Rarity: 🌱 uncommon. What is this egg doing here?
|
🤖 This is an automated PR from Repo Assist.
What
Two small allocation reductions in
ChannelsAggregator.Aggregate(), which is called on every channel-status update pushed from the gateway to the UI.Changes
1. Hoist repeated
QrLinkChannels.Contains(id)to a local boolPreviously the loop called
QrLinkChannels.Contains(id)twice per channel (once forCanShowQr/CanRelink, once forCanStop). Hoisting tovar isQrChannel = QrLinkChannels.Contains(id)avoids the second hash lookup.2. Replace LINQ sort with
List<T>.Sort()The
OrderByDescending(r => r.IsConfigured).ThenBy(r => r.SortOrder).ToList()chain creates:IOrderedEnumerable<T>wrapper, andList<ChannelRecord>materialisationReplacing with
records.Sort(static (a, b) => ...)sorts in-place with a staticComparison<T>delegate — no extra allocations. The sort order is identical (configured channels first, then bySortOrder).Behaviour
No behaviour change. The existing
ChannelsPipelineTestsandChannelsStatusTestspass unchanged.Test Status
dotnet buildonOpenClaw.Shared.Testssucceeds. Test run shows 8 pre-existing failures (identical to master baseline) — no new failures introduced.