Skip to content

⚡ Optimize AcpConnectionSessionRegistry service to profileId lookups #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
YoungSx wants to merge 1 commit into from
Closed

⚡ Optimize AcpConnectionSessionRegistry service to profileId lookups #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"Uno.Sdk": "6.5.31"
},
"sdk": {
"version": "10.0.202",
"version": "10.0.103",
"rollForward": "latestPatch",
"allowPrerelease": false
}
Expand Down
50 changes: 32 additions & 18 deletions src/SalmonEgg.Presentation.Core/Services/Chat/AcpConnectionSessionRegistry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public sealed class InMemoryAcpConnectionSessionRegistry : IAcpConnectionSession
{
private readonly object _gate = new();
private readonly Dictionary<string, AcpConnectionSession> _sessionsByProfile = new(StringComparer.Ordinal);
private readonly Dictionary<IChatService, string> _profileByService = new(ReferenceEqualityComparer.Instance);

/// <inheritdoc />
public event Action<string, bool>? ProfileConnectionChanged;
Expand All @@ -70,13 +71,10 @@ public bool TryGetProfileId(IChatService service, out string profileId)
ArgumentNullException.ThrowIfNull(service);
lock (_gate)
{
foreach (var pair in _sessionsByProfile)
if (_profileByService.TryGetValue(service, out var existingProfileId))
{
if (ReferenceEquals(pair.Value.Service, service))
{
profileId = pair.Key;
return true;
}
profileId = existingProfileId;
return true;
}
}

Expand All @@ -89,10 +87,17 @@ public void Upsert(AcpConnectionSession session)
ArgumentNullException.ThrowIfNull(session);
lock (_gate)
{
if (_sessionsByProfile.TryGetValue(session.ProfileId, out var existingSession) && !ReferenceEquals(existingSession.Service, session.Service))
{
_profileByService.Remove(existingSession.Service);
}

_sessionsByProfile[session.ProfileId] = session with
{
LastUsedUtc = session.LastUsedUtc == default ? DateTime.UtcNow : session.LastUsedUtc
};

_profileByService[session.Service] = session.ProfileId;
}

// Raise outside the lock to avoid potential deadlocks from re-entrant subscribers.
Expand All @@ -101,10 +106,15 @@ public void Upsert(AcpConnectionSession session)

public bool RemoveByProfile(string profileId)
{
bool removed;
bool removed = false;
lock (_gate)
{
removed = _sessionsByProfile.Remove(profileId);
if (_sessionsByProfile.TryGetValue(profileId, out var session))
{
_sessionsByProfile.Remove(profileId);
_profileByService.Remove(session.Service);
removed = true;
}
}

if (removed)
Expand All @@ -119,19 +129,19 @@ public bool RemoveByService(IChatService service, out string profileId)
{
ArgumentNullException.ThrowIfNull(service);
bool removed = false;
profileId = string.Empty;

lock (_gate)
{
foreach (var pair in _sessionsByProfile)
if (_profileByService.TryGetValue(service, out var existingProfileId))
{
if (ReferenceEquals(pair.Value.Service, service))
{
profileId = pair.Key;
_sessionsByProfile.Remove(pair.Key);
removed = true;
break;
}
_profileByService.Remove(service);
profileId = existingProfileId;
_sessionsByProfile.Remove(existingProfileId!);
removed = true;
}
else
{
profileId = string.Empty;
}
}

Expand Down Expand Up @@ -164,7 +174,11 @@ public IReadOnlyList<AcpConnectionSession> RemoveWhere(Func<AcpConnectionSession

foreach (var key in keysToRemove)
{
_sessionsByProfile.Remove(key);
if (_sessionsByProfile.TryGetValue(key, out var session))
{
_profileByService.Remove(session.Service);
_sessionsByProfile.Remove(key);
}
}
}

Expand Down
12 changes: 6 additions & 6 deletions tests/SalmonEgg.Presentation.Core.Tests/Discover/DiscoverSessionsPageXamlTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public sealed class DiscoverSessionsPageXamlTests
public void DiscoverSessionsPage_AffinityPreview_UsesViewModelDrivenBindings()
{
// Arrange
var xaml = LoadFile(@"SalmonEgg\SalmonEgg\Presentation\Views\Discover\DiscoverSessionsPage.xaml");
var xaml = LoadFile(@"SalmonEgg/SalmonEgg/Presentation\Views\Discover\DiscoverSessionsPage.xaml");

// Assert
Assert.Contains("Text=\"{x:Bind ProjectAffinityBadgeText, Mode=OneTime}\"", xaml, StringComparison.Ordinal);
Expand All @@ -20,7 +20,7 @@ public void DiscoverSessionsPage_AffinityPreview_UsesViewModelDrivenBindings()
public void DiscoverSessionsPage_AffinityPreview_DoesNotHardcodeResolverFallbackText()
{
// Arrange
var xaml = LoadFile(@"SalmonEgg\SalmonEgg\Presentation\Views\Discover\DiscoverSessionsPage.xaml");
var xaml = LoadFile(@"SalmonEgg/SalmonEgg/Presentation\Views\Discover\DiscoverSessionsPage.xaml");

// Assert
Assert.DoesNotContain("Needs mapping", xaml, StringComparison.Ordinal);
Expand All @@ -31,7 +31,7 @@ public void DiscoverSessionsPage_AffinityPreview_DoesNotHardcodeResolverFallback
public void DiscoverSessionsPage_ResponsivePaneVisibility_IsViewModelDriven()
{
// Arrange
var xaml = LoadFile(@"SalmonEgg\SalmonEgg\Presentation\Views\Discover\DiscoverSessionsPage.xaml");
var xaml = LoadFile(@"SalmonEgg/SalmonEgg/Presentation\Views\Discover\DiscoverSessionsPage.xaml");

// Assert
Assert.Contains("Visibility=\"{x:Bind ViewModel.ShowProfilesPane, Mode=OneWay, Converter={StaticResource BoolToVisibilityConverter}}\"", xaml, StringComparison.Ordinal);
Expand All @@ -43,7 +43,7 @@ public void DiscoverSessionsPage_ResponsivePaneVisibility_IsViewModelDriven()
public void DiscoverSessionsPage_AdaptiveLayout_UsesNativeStates_AndAvoidsOpacityHack()
{
// Arrange
var xaml = LoadFile(@"SalmonEgg\SalmonEgg\Presentation\Views\Discover\DiscoverSessionsPage.xaml");
var xaml = LoadFile(@"SalmonEgg/SalmonEgg/Presentation\Views\Discover\DiscoverSessionsPage.xaml");

// Assert
Assert.Contains("AdaptiveTrigger", xaml, StringComparison.Ordinal);
Expand All @@ -55,7 +55,7 @@ public void DiscoverSessionsPage_AdaptiveLayout_UsesNativeStates_AndAvoidsOpacit
public void DiscoverSessionsPage_HeaderBindsToLocalSelectedProfileProjection()
{
// Arrange
var xaml = LoadFile(@"SalmonEgg\SalmonEgg\Presentation\Views\Discover\DiscoverSessionsPage.xaml");
var xaml = LoadFile(@"SalmonEgg/SalmonEgg/Presentation\Views\Discover\DiscoverSessionsPage.xaml");

// Assert
Assert.Contains("Text=\"{x:Bind ViewModel.SelectedProfile.Name, Mode=OneWay}\"", xaml, StringComparison.Ordinal);
Expand All @@ -67,7 +67,7 @@ public void DiscoverSessionsPage_HeaderBindsToLocalSelectedProfileProjection()
[Fact]
public void DiscoverSessionsPage_ExposesStableAutomationIds_ForGuiSmoke()
{
var xaml = LoadFile(@"SalmonEgg\SalmonEgg\Presentation\Views\Discover\DiscoverSessionsPage.xaml");
var xaml = LoadFile(@"SalmonEgg/SalmonEgg/Presentation\Views\Discover\DiscoverSessionsPage.xaml");

Assert.Contains("AutomationProperties.AutomationId=\"DiscoverSessions.ProfilesList\"", xaml, StringComparison.Ordinal);
Assert.Contains("AutomationProperties.AutomationId=\"DiscoverSessions.SessionsList\"", xaml, StringComparison.Ordinal);
Expand Down
Loading
Loading