Skip to content
This repository was archived by the owner on Jun 5, 2025. It is now read-only.

fix how we update the properties in environments page after an operation #2950

Merged
merged 3 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
24 changes: 19 additions & 5 deletions common/Environments/Helpers/ComputeSystemHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading.Tasks;
using DevHome.Common.Environments.Models;
using Microsoft.UI.Xaml.Media.Imaging;
using Microsoft.VisualBasic;
using Microsoft.Windows.DevHome.SDK;
using Serilog;

Expand Down Expand Up @@ -131,7 +132,7 @@ public static EnvironmentsCallToActionData UpdateCallToActionText(int providerCo
}

/// <summary>
/// Safely remove all items from an observable collection.
/// Safely removes all items from an observable collection and replaces them with new items.
/// </summary>
/// <remarks>
/// There can be random COM exceptions due to using the "Clear()" method in an observable collection. This method
Expand All @@ -140,19 +141,32 @@ public static EnvironmentsCallToActionData UpdateCallToActionText(int providerCo
/// this method is used to remove all items individually from the end of the collection to the beginning of the collection.
/// </remarks>
/// <typeparam name="T">Type of objects that the collection contains</typeparam>
/// <param name="collection">An observable collection that contains zero to N elements</param>
public static void RemoveAllItems<T>(ObservableCollection<T> collection)
/// <param name="collectionToUpdate">An observable collection that contains zero to N elements that will have its contents replaced</param>
/// <param name="listWithUpdates">A list that contains zero to N elements whose elements will be added to collectionToUpdate</param>
/// <returns>
/// True only if we successfully replaced all items in the collection. False otherwise.
/// </returns>
public static bool RemoveAllItemsAnReplace<T>(ObservableCollection<T> collectionToUpdate, List<T> listWithUpdates)
{
try
{
for (var i = collection.Count - 1; i >= 0; i--)
for (var i = collectionToUpdate.Count - 1; i >= 0; i--)
{
collection.RemoveAt(i);
collectionToUpdate.RemoveAt(i);
}

for (var i = 0; i < listWithUpdates.Count; i++)
{
collectionToUpdate.Add(listWithUpdates[i]);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These loops fire OnCollectionChanged events for every operation which is inefficient and can make UX less responsive.
There is BulkObservableCollection class that has AddRange method and
BeginBulkOperation and EndBulkOperation methods that can be used to suspend notification while doing multiple changes to the collection.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using that requires adding the Microsoft.VisualStudio.Language.Intellisense nuget package so I don't think we want to add it this close to cut off. Luckily the amount of properties sent back by both the Hyper-V extension and Dev Box are small (4 for dev box and 5 for hyper-V). So I can investigate using the above nuget package after build

return true;
}
catch (Exception ex)
{
_log.Error(ex, "Unable to remove items from the collection");
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,9 @@ private async void SetPropertiesAsync()
}

var properties = await ComputeSystemHelpers.GetComputeSystemCardPropertiesAsync(ComputeSystem!, PackageFullName);

ComputeSystemHelpers.RemoveAllItems(Properties);
foreach (var property in properties)
if (!ComputeSystemHelpers.RemoveAllItemsAnReplace(Properties, properties))
{
Properties.Add(property);
Properties = new(properties);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,9 @@ private async Task UpdatePropertiesAsync()
var properties = await ComputeSystemHelpers.GetComputeSystemCardPropertiesAsync(ComputeSystem, _packageFullName);
lock (_lock)
{
ComputeSystemHelpers.RemoveAllItems(ComputeSystemProperties);
foreach (var property in properties)
if (!ComputeSystemHelpers.RemoveAllItemsAnReplace(ComputeSystemProperties, properties))
{
ComputeSystemProperties.Add(property);
ComputeSystemProperties = new(properties);
}
}
}
Expand Down