From 2b76b11c8a8d7d1224827a16da996d39a556ebf5 Mon Sep 17 00:00:00 2001 From: Marcus Markiewicz Date: Thu, 11 Jan 2024 10:48:38 -0500 Subject: [PATCH 1/2] Reduce caching in Resolver Instead of caching full graph objects (user and group), let's just cache the data we use. This brings down memory consumption by roughly ~60% on a test machine where there are a lot of objects to cache. --- Source/TeamMate/Services/ResolverService.cs | 33 ++++++++++++--------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/Source/TeamMate/Services/ResolverService.cs b/Source/TeamMate/Services/ResolverService.cs index 7de0138..bcec132 100644 --- a/Source/TeamMate/Services/ResolverService.cs +++ b/Source/TeamMate/Services/ResolverService.cs @@ -1,4 +1,5 @@ using Microsoft.VisualStudio.Services.Graph.Client; +using Microsoft.VisualStudio.Services.Users; using System; using System.Collections.Generic; using System.Collections.ObjectModel; @@ -10,13 +11,13 @@ namespace Microsoft.Tools.TeamMate.Services { public class ResolverService { - private List GraphUserCache { get; set; } + private Dictionary GraphUserCache { get; set; } - private List GraphGroupCache { get; set; } + private Dictionary GraphGroupCache { get; set; } private List Tasks = new List(); - private async Task> FetchUsersAsync( + private async Task> FetchUsersAsync( GraphHttpClient graphClient) { if (GraphUserCache != null) @@ -24,7 +25,7 @@ private async Task> FetchUsersAsync( return GraphUserCache; } - List users = new List(); + var users = new Dictionary(); string continuationToken = null; do @@ -33,7 +34,10 @@ private async Task> FetchUsersAsync( continuationToken = data.ContinuationToken != null ? data.ContinuationToken.First() : null; foreach (var user in data.GraphUsers) { - users.Add(user); + if (user.MailAddress != null) + { + users[user.MailAddress] = user.Descriptor; + } } } while (continuationToken != null); @@ -43,7 +47,7 @@ private async Task> FetchUsersAsync( return users; } - private async Task> FetchGroupsAsync( + private async Task> FetchGroupsAsync( GraphHttpClient graphClient) { if (GraphGroupCache != null) @@ -51,7 +55,7 @@ private async Task> FetchGroupsAsync( return GraphGroupCache; } - List groups = new List(); + var groups = new Dictionary(); string continuationToken = null; do @@ -61,7 +65,10 @@ private async Task> FetchGroupsAsync( continuationToken = data.ContinuationToken != null ? data.ContinuationToken.First() : null; foreach (var group in data.GraphGroups) { - groups.Add(group); + if (group.MailAddress != null) + { + groups[group.MailAddress] = group.Descriptor; + } } } while (continuationToken != null); @@ -91,10 +98,9 @@ public void FetchDataSync( foreach (var user in GraphUserCache) { - if (user.MailAddress != null && - (user.MailAddress.Contains(value))) + if (user.Key.Contains(value)) { - var storageKey = client.GetStorageKeyAsync(user.Descriptor).Result; + var storageKey = client.GetStorageKeyAsync(user.Value).Result; return storageKey.Value; } @@ -102,10 +108,9 @@ public void FetchDataSync( foreach (var group in GraphGroupCache) { - if (group.MailAddress != null && - (group.MailAddress.Contains(value))) + if (group.Key.Contains(value)) { - var storageKey = client.GetStorageKeyAsync(group.Descriptor).Result; + var storageKey = client.GetStorageKeyAsync(group.Value).Result; return storageKey.Value; } From 4e48384f3e6c5b397e7c7529531e6212411e6d3c Mon Sep 17 00:00:00 2001 From: Marcus Markiewicz Date: Thu, 11 Jan 2024 10:58:23 -0500 Subject: [PATCH 2/2] Resolver Cache should load on-demand Let's change it so we will only load the resolver cache when it is actually used. If a customer doesn't use PR functionality at all, it would never light up, so we save the bandwidth, CPU and memory. Compared to 0.1.8, this change yields ~80% memory savings on a machine with lots of graph objects. --- Source/TeamMate/Services/ResolverService.cs | 19 +++++++++++++++---- .../Services/VstsConnectionService.cs | 3 --- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/Source/TeamMate/Services/ResolverService.cs b/Source/TeamMate/Services/ResolverService.cs index bcec132..29ab166 100644 --- a/Source/TeamMate/Services/ResolverService.cs +++ b/Source/TeamMate/Services/ResolverService.cs @@ -17,6 +17,8 @@ public class ResolverService private List Tasks = new List(); + private bool Cached = false; + private async Task> FetchUsersAsync( GraphHttpClient graphClient) { @@ -78,11 +80,19 @@ public class ResolverService return groups; } - public void FetchDataSync( + private void FetchDataSyncIfNeeded( GraphHttpClient client) { - Tasks.Add(FetchUsersAsync(client)); - Tasks.Add(FetchGroupsAsync(client)); + lock (Tasks) + { + if (!Cached) + { + Tasks.Add(FetchUsersAsync(client)); + Tasks.Add(FetchGroupsAsync(client)); + + Cached = true; + } + } } public async Task Resolve( @@ -94,6 +104,8 @@ public void FetchDataSync( return null; } + this.FetchDataSyncIfNeeded(client); + await Task.Run(() => { foreach (var task in this.Tasks) { task.Wait(); } }); foreach (var user in GraphUserCache) @@ -117,7 +129,6 @@ public void FetchDataSync( } throw new ArgumentException("Could not resolve '" + value + "'. Try the full email for the person and/or group."); - } } } diff --git a/Source/TeamMate/Services/VstsConnectionService.cs b/Source/TeamMate/Services/VstsConnectionService.cs index cd90b7c..1bf7a97 100644 --- a/Source/TeamMate/Services/VstsConnectionService.cs +++ b/Source/TeamMate/Services/VstsConnectionService.cs @@ -190,9 +190,6 @@ private async Task DoConnectAsync(ProjectInfo projectInfo, Cance projectContext.WorkItemFieldsByName = fields.ToDictionary(f => f.ReferenceName, StringComparer.OrdinalIgnoreCase); projectContext.RequiredWorkItemFieldNames = GetWorkItemFieldsToPrefetch(projectContext.WorkItemFieldsByName); - this.ResolverService.FetchDataSync( - graphClient); - return projectContext; } catch (Exception e)