Coalesce the assembly tree's selection and removal fan-out

ICSharpCode.ILSpyX/AssemblyList.cs

@@ -57,7 +57,7 @@ public sealed class AssemblyList
 		/// Technically read accesses need locking when done on non-GUI threads... but whenever possible, use the
 		/// thread-safe <see cref="GetAssemblies()"/> method.
 		/// </remarks>
-		readonly ObservableCollection<LoadedAssembly> assemblies = new ObservableCollection<LoadedAssembly>();
+		readonly AssemblyCollection assemblies = new AssemblyCollection();
 
 		/// <summary>
 		/// Assembly lookup by filename.
@@ -428,6 +428,70 @@ public void Unload(LoadedAssembly assembly)
 			// the last reference is gone.
 		}
 
+		/// <summary>
+		/// Removes every assembly in <paramref name="assembliesToUnload"/> in one step.
+		/// </summary>
+		/// <remarks>
+		/// Removing them one at a time is not equivalent: each removal raises its own collection
+		/// change, and consumers react to one of those by pruning navigation history, restarting a
+		/// running search, re-querying every command and re-decompiling the surviving selection.
+		/// Repeating that per assembly is what made clearing an expanded list freeze the UI.
+		/// Contiguous runs are removed as ranges, so listeners keep the indices and the removed
+		/// items they need to splice their own state. Emptying the list this way still reports a
+		/// Remove rather than the Reset <see cref="Clear"/> raises: consumers treat Reset as
+		/// "re-read everything" and several of them short-circuit their per-removal cleanup on it,
+		/// which would leave tabs and navigation history pointing at unloaded assemblies.
+		/// </remarks>
+		public void UnloadRange(IEnumerable<LoadedAssembly> assembliesToUnload)
+		{
+			ArgumentNullException.ThrowIfNull(assembliesToUnload);
+			VerifyAccess();
+			var doomed = new HashSet<LoadedAssembly>(assembliesToUnload);
+			if (doomed.Count == 0)
+				return;
+			lock (lockObj)
+			{
+				// Only the named entries leave byFilename, so an assembly a background thread is
+				// still loading survives -- unlike Clear(), which drops the lookup wholesale.
+				foreach (var assembly in doomed)
+					byFilename.Remove(assembly.FileName);
+				// Backwards, so the indices of the runs not yet visited stay valid.
+				for (int end = assemblies.Count - 1; end >= 0;)
+				{
+					if (!doomed.Contains(assemblies[end]))
+					{
+						end--;
+						continue;
+					}
+					int start = end;
+					while (start > 0 && doomed.Contains(assemblies[start - 1]))
+						start--;
+					assemblies.RemoveAssemblies(start, end - start + 1);
+					end = start - 1;
+				}
+			}
+			// Removed from the list but NOT disposed -- see Unload.
+		}
+
+		sealed class AssemblyCollection : ObservableCollection<LoadedAssembly>
+		{
+			public void RemoveAssemblies(int index, int count)
+			{
+				if (count <= 0)
+					return;
+				CheckReentrancy();
+				var removed = new List<LoadedAssembly>(count);
+				for (int i = 0; i < count; i++)
+					removed.Add(Items[index + i]);
+				for (int i = 0; i < count; i++)
+					Items.RemoveAt(index);
+				OnPropertyChanged(new System.ComponentModel.PropertyChangedEventArgs(nameof(Count)));
+				OnPropertyChanged(new System.ComponentModel.PropertyChangedEventArgs("Item[]"));
+				OnCollectionChanged(new NotifyCollectionChangedEventArgs(
+					NotifyCollectionChangedAction.Remove, removed, index));
+			}
+		}
+
 		public void Clear()
 		{
 			VerifyAccess();

ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs

@@ -212,11 +212,18 @@ public virtual void OnChildrenChanged(NotifyCollectionChangedEventArgs e)
 			}
 			if (e.OldItems != null)
 			{
+				// Detach the whole set before announcing any of it. The per-node notifications
+				// below reach the selection, and a listener that reacts to the first of them would
+				// otherwise still find the rest of the same removal attached, and treat those as a
+				// selection the user made -- ending with the last node of the set on its own.
 				foreach (SharpTreeNode node in e.OldItems)
 				{
 					Debug.Assert(node.modelParent == this);
 					node.modelParent = null;
 					node.OnParentChanged();
+				}
+				foreach (SharpTreeNode node in e.OldItems)
+				{
 					Debug.WriteLine("Removing {0} from {1}", node, this);
 					SharpTreeNode removeEnd = node;
 					while (removeEnd.modelChildren != null && removeEnd.modelChildren.Count > 0)
@@ -704,6 +711,18 @@ public virtual void DeleteCore()
 			throw new NotSupportedException(GetType().Name + " does not support deletion");
 		}
 
+		/// <summary>
+		/// Deletes <paramref name="nodes"/> -- the whole set the user asked to remove, this node
+		/// included. Override when the underlying model can drop a set in one operation, so its
+		/// listeners see one change instead of one per node.
+		/// </summary>
+		public virtual void Delete(SharpTreeNode[] nodes)
+		{
+			ArgumentNullException.ThrowIfNull(nodes);
+			foreach (var node in nodes)
+				node.Delete();
+		}
+
 		public virtual IPlatformDataObject Copy(SharpTreeNode[] nodes)
 		{
 			throw new NotSupportedException(GetType().Name + " does not support copy/paste or drag'n'drop");

ILSpy.Tests/AssemblyList/SelectAllDeleteBatchingTests.cs

@@ -0,0 +1,132 @@
+// Copyright (c) 2026 Siegfried Pammer
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy of this
+// software and associated documentation files (the "Software"), to deal in the Software
+// without restriction, including without limitation the rights to use, copy, modify, merge,
+// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
+// to whom the Software is furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in all copies or
+// substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
+// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+// DEALINGS IN THE SOFTWARE.
+
+using System.Collections.Specialized;
+using System.ComponentModel;
+using System.Threading.Tasks;
+
+using Avalonia.Controls;
+using Avalonia.Headless.NUnit;
+using Avalonia.Input;
+
+using AwesomeAssertions;
+
+using ICSharpCode.ILSpy.AssemblyTree;
+using ICSharpCode.ILSpy.Controls.TreeView;
+
+using NUnit.Framework;
+
+namespace ICSharpCode.ILSpy.Tests;
+
+/// <summary>
+/// Ctrl+A followed by Delete must cost one batch, not one fan-out per row. Every selection
+/// change reaches a full application-wide command re-query, a history prune, and a decompile
+/// kick-off; repeating that per node (and again per removed assembly) is what made clearing an
+/// expanded list freeze the UI. These pin the batching so the cost stays proportional to the
+/// number of user actions rather than to the number of selected nodes.
+/// </summary>
+[TestFixture]
+public class SelectAllDeleteBatchingTests
+{
+	static void RaiseKey(Control target, Key key, KeyModifiers modifiers)
+	{
+		target.RaiseEvent(new KeyEventArgs {
+			Key = key,
+			KeyModifiers = modifiers,
+			RoutedEvent = InputElement.KeyDownEvent,
+			Source = target,
+		});
+	}
+
+	// Deliberately not WaitForIdleAsync: a select-all starts a decompile of everything selected,
+	// and its progress spinner keeps posting to the dispatcher until that finishes, so the app is
+	// never idle. Everything asserted here -- the selection, the fan-out count, the list change --
+	// lands synchronously while the key is handled, so pumping once is the right synchronization
+	// point. The tree is left collapsed for the same reason: expanding an assembly first would put
+	// every type in it into the selection, and decompiling those dominates the run without making
+	// the assertions any sharper. One fan-out versus one per row already separates the two
+	// behaviours at three rows.
+	static void SettleInput() => Waiters.PumpUI();
+
+	[AvaloniaTest]
+	public async Task Select_All_Fans_Out_Once_Not_Once_Per_Node()
+	{
+		var (window, vm) = await TestHarness.BootAsync(3);
+		var pane = await window.WaitForComponent<AssemblyListPane>();
+		var tree = pane.FindControl<SharpTreeView>("Tree")!;
+		var model = vm.AssemblyTreeModel;
+
+		int rows = ((System.Collections.IList)tree.ItemsSource!).Count;
+		rows.Should().BeGreaterThan(2, "the fan-out only shows up on a multi-row selection");
+
+		int fanOuts = 0;
+		void OnChanged(object? _, PropertyChangedEventArgs e)
+		{
+			if (e.PropertyName == nameof(AssemblyTreeModel.SelectedItem))
+				fanOuts++;
+		}
+		model.PropertyChanged += OnChanged;
+		try
+		{
+			RaiseKey(tree, Key.A, KeyModifiers.Control);
+			SettleInput();
+		}
+		finally
+		{
+			model.PropertyChanged -= OnChanged;
+		}
+
+		model.SelectedItems.Should().HaveCount(rows, "Ctrl+A selects every visible row");
+		fanOuts.Should().Be(1,
+			"selecting N rows fans out once, not once per row -- each fan-out runs a full command "
+			+ "re-query, a session-settings write and a decompile of the selection");
+	}
+
+	[AvaloniaTest]
+	public async Task Deleting_Every_Assembly_Raises_One_List_Change_Not_One_Per_Assembly()
+	{
+		var (window, vm) = await TestHarness.BootAsync(3);
+		var pane = await window.WaitForComponent<AssemblyListPane>();
+		var tree = pane.FindControl<SharpTreeView>("Tree")!;
+		var model = vm.AssemblyTreeModel;
+		var list = model.AssemblyList!;
+
+		int assemblies = list.Count;
+		assemblies.Should().BeGreaterThan(2);
+
+		int listChanges = 0;
+		void OnListChanged(object? _, NotifyCollectionChangedEventArgs e) => listChanges++;
+		list.CollectionChanged += OnListChanged;
+		try
+		{
+			RaiseKey(tree, Key.A, KeyModifiers.Control);
+			SettleInput();
+			RaiseKey(tree, Key.Delete, KeyModifiers.None);
+			SettleInput();
+		}
+		finally
+		{
+			list.CollectionChanged -= OnListChanged;
+		}
+
+		list.Count.Should().Be(0, "Delete on a select-all removes every assembly");
+		listChanges.Should().Be(1,
+			"removing N assemblies must raise one batched collection change, not one per assembly -- "
+			+ "every consumer (history prune, search restart, command re-query, tab prune) runs per event");
+	}
+}

ILSpy/AssemblyTree/AssemblyListPane.axaml.cs

@@ -272,7 +272,8 @@ protected override void OnDataContextChanged(EventArgs e)
 					Tree.Root = model.Root;
 					WireDropSelection(model);
 				}
-				selectionBinder = new ICSharpCode.ILSpy.Controls.TreeView.TreeSelectionBinder(Tree, model.SelectedItems);
+				selectionBinder = new ICSharpCode.ILSpy.Controls.TreeView.TreeSelectionBinder(
+					Tree, model.SelectedItems, model.BatchSelectionChange);
 			}
 		}
 

ILSpy/AssemblyTree/AssemblyTreeModel.cs

@@ -109,21 +109,16 @@ public void SelectNodes(IReadOnlyList<SharpTreeNode> nodes)
 			ArgumentNullException.ThrowIfNull(nodes);
 			if (SelectionMatches(nodes))
 				return;
-			batchingSelectionChange = true;
-			try
+			using (BatchSelectionChange())
 			{
 				SelectedItems.Clear();
+				var seen = new HashSet<SharpTreeNode>();
 				foreach (var node in nodes)
 				{
-					if (node != null && !SelectedItems.Contains(node))
+					if (node != null && seen.Add(node))
 						SelectedItems.Add(node);
 				}
 			}
-			finally
-			{
-				batchingSelectionChange = false;
-			}
-			RaiseSelectionChanged();
 		}
 
 		bool SelectionMatches(IReadOnlyList<SharpTreeNode> nodes)
@@ -204,10 +199,49 @@ void OnNavigateToReference(object? sender, Util.NavigateToReferenceEventArgs e)
 				decompTab.HighlightedReference = e.Source;
 		}
 
-		// True while the SelectedItem setter is replacing the collection via Clear()+Add();
-		// suppresses the selection-changed fan-out until the final state is in place so consumers
-		// never observe the transient empty/multi mid-replace.
-		bool batchingSelectionChange;
+		// Greater than zero while a bulk edit is rewriting SelectedItems; suppresses the
+		// selection-changed fan-out until the final state is in place so consumers never observe a
+		// transient empty/multi mid-replace, and so the fan-out costs one pass rather than one per
+		// node. RaiseSelectionChanged reaches a full command re-query, a session-settings write, a
+		// message-bus broadcast and a decompile of the new selection, so running it per node is
+		// what made selecting a large subtree freeze the UI.
+		int selectionBatchDepth;
+
+		// Set when a change actually arrived while a batch was open, so a batch that rewrote the
+		// selection to what it already was closes without notifying anyone.
+		bool selectionChangedDuringBatch;
+
+		/// <summary>
+		/// Suppresses the selection fan-out for the lifetime of the returned scope, then raises it
+		/// exactly once. Nests; only the outermost scope raises.
+		/// </summary>
+		public IDisposable BatchSelectionChange() => new SelectionBatchScope(this);
+
+		sealed class SelectionBatchScope : IDisposable
+		{
+			readonly AssemblyTreeModel model;
+			bool disposed;
+
+			public SelectionBatchScope(AssemblyTreeModel model)
+			{
+				this.model = model;
+				model.selectionBatchDepth++;
+			}
+
+			public void Dispose()
+			{
+				if (disposed)
+					return;
+				disposed = true;
+				if (--model.selectionBatchDepth > 0)
+					return;
+				if (model.selectionChangedDuringBatch)
+				{
+					model.selectionChangedDuringBatch = false;
+					model.RaiseSelectionChanged();
+				}
+			}
+		}
 
 		void OnSelectedItemsChanged(object? sender, NotifyCollectionChangedEventArgs e)
 		{
@@ -218,10 +252,13 @@ void OnSelectedItemsChanged(object? sender, NotifyCollectionChangedEventArgs e)
 				foreach (SharpTreeNode n in e.OldItems)
 					n.IsSelected = false;
 
-			// During a SelectedItem-setter batch the fan-out is deferred to the single
-			// RaiseSelectionChanged() the setter issues once the final selection is in place.
-			if (batchingSelectionChange)
+			// Inside a batch the fan-out is deferred to the single RaiseSelectionChanged() the
+			// outermost scope issues once the final selection is in place.
+			if (selectionBatchDepth > 0)
+			{
+				selectionChangedDuringBatch = true;
 				return;
+			}
 			RaiseSelectionChanged();
 		}
 

ILSpy/Controls/TreeView/SharpTreeView.cs

@@ -304,7 +304,7 @@ public void FocusNode(SharpTreeNode node, bool scroll = true)
 		public bool IsNodeFullyVisible(SharpTreeNode node)
 		{
 			ArgumentNullException.ThrowIfNull(node);
-			var scrollViewer = this.GetVisualDescendants().OfType<ScrollViewer>().FirstOrDefault();
+			var scrollViewer = ScrollHost;
 			if (scrollViewer is null)
 				return false;
 			if (ContainerFromItem(node) is Control row && row.IsVisible
@@ -313,6 +313,17 @@ public bool IsNodeFullyVisible(SharpTreeNode node)
 			return false;
 		}
 
+		ScrollViewer? scrollHost;
+
+		/// <summary>
+		/// The template's <see cref="ScrollViewer"/>, looked up once. Callers that ask per node --
+		/// a selection sync checks every selected row -- would otherwise walk the visual tree once
+		/// per call. Re-resolved while null so a query made before the template is applied does not
+		/// cache the miss.
+		/// </summary>
+		ScrollViewer? ScrollHost
+			=> scrollHost ??= this.GetVisualDescendants().OfType<ScrollViewer>().FirstOrDefault();
+
 		/// <summary>Moves the (single) selection to <paramref name="node"/> and focuses it.
 		/// Used by keyboard navigation where selection must follow the caret.</summary>
 		void SelectAndFocus(SharpTreeNode node)
@@ -535,8 +546,7 @@ bool DeleteSelection()
 			if (nodes.Length == 0 || !nodes.All(n => n.CanDelete()))
 				return false;
 			int index = nodes.Min(flattener.IndexOf);
-			foreach (var node in nodes)
-				node.Delete();
+			nodes[0].Delete(nodes);
 			// The deleted rows leave the selection with the source; pick the nearest survivor.
 			if (SelectedItems!.Count == 0 && flattener.Count > 0)
 				SelectAndFocus((SharpTreeNode)flattener[Math.Clamp(index, 0, flattener.Count - 1)]!);