Browse Source

Stop rescanning references to paint the bookmark gutter every frame

Two costs on the gutter's hot path:

The margin rebuilt its document-line -> glyph map on every Render, resolving
each bookmark by scanning the document's reference segments. A scroll, a hover,
or the 600 ms post-navigation pulse repaints many times a second, so this
rescanned the references per bookmark per frame. Compute the map once and reuse
it, clearing the cache only when the document or the bookmark set changes (the
model's DebugInfo/References are set before the text, so the document-change
hook sees them current).

CanToggleBookmarkAtLine, called for every gutter line the pointer moves over,
built a full bookmark including a captured editor view state -- a foldings
snapshot, scroll offsets, and a tree-path walk -- only to test it for null. It
now does an anchor-only check; the view state is captured only when a bookmark
is actually created.

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3839/head
Siegfried Pammer 1 week ago committed by Siegfried Pammer
parent
commit
2846f00080
  1. 65
      ILSpy.Tests/Bookmarks/BookmarkGutterTests.cs
  2. 57
      ILSpy/Bookmarks/BookmarkMargin.cs
  3. 24
      ILSpy/TextView/DecompilerTextView.axaml.cs

65
ILSpy.Tests/Bookmarks/BookmarkGutterTests.cs

@ -30,8 +30,10 @@ using AvaloniaEdit.Rendering; @@ -30,8 +30,10 @@ using AvaloniaEdit.Rendering;
using AwesomeAssertions;
using ICSharpCode.ILSpy;
using ICSharpCode.ILSpy.AppEnv;
using ICSharpCode.ILSpy.Bookmarks;
using ICSharpCode.ILSpy.Properties;
using ICSharpCode.ILSpy.TextView;
using ICSharpCode.ILSpy.TreeNodes;
@ -210,4 +212,67 @@ public class BookmarkGutterTests @@ -210,4 +212,67 @@ public class BookmarkGutterTests
Dispatcher.UIThread.RunJobs();
margin.HoverPreviewLine.Should().Be(line0, "leaving and re-entering the line restores its preview");
}
// Regression: the editor reuses one TextDocument and only swaps its text on navigation, so the
// gutter's cached glyph map must rebuild off the document's content (DebugInfo/References), not off
// the document reference -- otherwise switching nodes leaves the gutter showing the previous (or no)
// document's bookmarks.
[AvaloniaTest]
public async Task Gutter_Glyph_Map_Rebuilds_When_The_Displayed_Document_Changes()
{
var (window, vm) = await TestHarness.BootAsync(3);
var manager = AppComposition.Current.GetExport<BookmarkManager>();
manager.Clear();
var coreLibName = typeof(object).Assembly.GetName().Name!;
async Task<DecompilerTextView> Show(string typeName)
{
var node = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(coreLibName, "System", typeName);
vm.AssemblyTreeModel.SelectNode(node);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
var shown = await window.WaitForComponent<DecompilerTextView>();
for (int i = 0; i < 8; i++)
{
Dispatcher.UIThread.RunJobs();
await Task.Delay(25);
}
return shown;
}
var view = await Show("System.Object");
var margin = view.Editor.TextArea.LeftMargins.OfType<BookmarkMargin>().Single();
var toggle = AppComposition.Current.GetExport<ContextMenuEntryRegistry>().GetEntry(nameof(Resources.BookmarkToggle));
// A body (IL-offset) anchor, so it only resolves inside System.Object -- a token/line anchor on
// the type would also resolve in System.String, which merely references System.Object.
Bookmark? bookmark = null;
foreach (int candidate in Enumerable.Range(1, view.Editor.Document.LineCount).Where(view.CanToggleBookmarkAtLine))
{
int candidateOffset = view.Editor.Document.GetLineByNumber(candidate).Offset;
view.Editor.TextArea.Caret.Offset = candidateOffset;
toggle.Execute(new TextViewContext { TextView = view, TextLocation = candidateOffset });
Dispatcher.UIThread.RunJobs();
var candidateBookmark = manager.Bookmarks.Single();
if (candidateBookmark.Kind == BookmarkKind.Body)
{
bookmark = candidateBookmark;
break;
}
manager.Clear();
}
bookmark.Should().NotBeNull("System.Object has a method-body line to anchor to");
int objectLine = view.GetLineForBookmark(bookmark!)!.Value;
margin.GlyphLinesForTest().Keys.Should().Contain(objectLine, "the gutter shows the bookmark on its own document");
// Navigate to another type. The preview tab reuses the same view (and gutter), only swapping the
// document text, so the gutter must drop the stale glyph: System.Object's bookmark is not here.
var stringView = await Show("System.String");
stringView.Should().BeSameAs(view, "the preview tab reuses the view, so this exercises the stale cache");
margin.GlyphLinesForTest().Should().BeEmpty("the gutter must rebuild for System.String, where the bookmark does not resolve");
// Returning to the bookmarked document must surface its glyph again.
await Show("System.Object");
margin.GlyphLinesForTest().Keys.Should().Contain(view.GetLineForBookmark(bookmark!)!.Value,
"returning to the bookmarked document must show its glyph again");
}
}

57
ILSpy/Bookmarks/BookmarkMargin.cs

@ -60,6 +60,9 @@ namespace ICSharpCode.ILSpy.Bookmarks @@ -60,6 +60,9 @@ namespace ICSharpCode.ILSpy.Bookmarks
int hoverLine = -1;
// A line whose hover preview stays hidden after a removal click, until the pointer leaves it.
int suppressedHoverLine = -1;
// Cached document-line -> glyph map and the content version it was built against.
Dictionary<int, IImage>? glyphByLine;
(object?, object?) glyphByLineVersion;
public BookmarkMargin(TextView.DecompilerTextView owner)
{
@ -105,9 +108,14 @@ namespace ICSharpCode.ILSpy.Bookmarks @@ -105,9 +108,14 @@ namespace ICSharpCode.ILSpy.Bookmarks
pulseTimer.Stop();
pulseLine = -1;
hoverLine = -1;
glyphByLine = null;
}
void OnBookmarksChanged(object? sender, EventArgs e) => InvalidateVisual();
void OnBookmarksChanged(object? sender, EventArgs e)
{
glyphByLine = null;
InvalidateVisual();
}
/// <summary>Plays the one-shot bounce on the bookmark glyph at <paramref name="line"/>.</summary>
public void PulseLine(int line)
@ -146,20 +154,7 @@ namespace ICSharpCode.ILSpy.Bookmarks @@ -146,20 +154,7 @@ namespace ICSharpCode.ILSpy.Bookmarks
if (manager == null || textView == null || !textView.VisualLinesValid)
return;
// Map the document lines that currently hold a bookmark to their glyph. Bookmarks not in
// this document resolve to no line and are skipped.
var glyphByLine = new Dictionary<int, IImage>();
foreach (var bookmark in manager.Bookmarks)
{
if (owner.GetLineForBookmark(bookmark, updateRenderedLine: false) is { } line)
{
if (bookmark.LineNumber != line)
{
Dispatcher.UIThread.Post(() => bookmark.UpdateRenderedLineNumber(line), DispatcherPriority.Background);
}
glyphByLine[line] = bookmark.Enabled ? Images.Bookmark : Images.BookmarkDisable;
}
}
var glyphByLine = GetGlyphByLine(manager);
foreach (var visualLine in textView.VisualLines)
{
int lineNumber = visualLine.FirstDocumentLine.LineNumber;
@ -194,6 +189,33 @@ namespace ICSharpCode.ILSpy.Bookmarks @@ -194,6 +189,33 @@ namespace ICSharpCode.ILSpy.Bookmarks
}
}
// The document line -> glyph map for the current document and bookmark set. Resolving a bookmark
// to its line scans the document's references, so it is computed once and reused across the many
// repaints a scroll, hover or pulse triggers. It is rebuilt when the bookmark set changes (which
// clears the cache) or when the displayed document changes -- detected by its content version,
// since the editor reuses one TextDocument and only swaps its text, so the document reference
// never changes.
Dictionary<int, IImage> GetGlyphByLine(BookmarkManager manager)
{
var version = owner.BookmarkContentVersion;
if (glyphByLine != null && version.Equals(glyphByLineVersion))
return glyphByLine;
glyphByLineVersion = version;
// Bookmarks not anchored in this document resolve to no line and are skipped.
var map = new Dictionary<int, IImage>();
foreach (var bookmark in manager.Bookmarks)
{
if (owner.GetLineForBookmark(bookmark, updateRenderedLine: false) is { } line)
{
if (bookmark.LineNumber != line)
Dispatcher.UIThread.Post(() => bookmark.UpdateRenderedLineNumber(line), DispatcherPriority.Background);
map[line] = bookmark.Enabled ? Images.Bookmark : Images.BookmarkDisable;
}
}
return glyphByLine = map;
}
// A bitmap copy of the bookmark glyph, shared by all gutters, used only for the hover preview.
// SvgImage paints through a custom Skia draw operation that ignores DrawingContext.PushOpacity,
// so the SVG can't be faded directly; a rasterized bitmap is composited at the pushed opacity.
@ -300,5 +322,10 @@ namespace ICSharpCode.ILSpy.Bookmarks @@ -300,5 +322,10 @@ namespace ICSharpCode.ILSpy.Bookmarks
// The line currently drawing a hover-preview glyph, or -1 when none. Exposed for tests.
internal int HoverPreviewLine => hoverLine;
// The document lines that currently carry a bookmark glyph, resolved against the live document.
// Exposed for tests because the gutter only paints when rendering is on (off in the headless CI).
internal IReadOnlyDictionary<int, IImage> GlyphLinesForTest()
=> Manager is { } m ? GetGlyphByLine(m) : new Dictionary<int, IImage>();
}
}

24
ILSpy/TextView/DecompilerTextView.axaml.cs

@ -686,14 +686,22 @@ namespace ICSharpCode.ILSpy.TextView @@ -686,14 +686,22 @@ namespace ICSharpCode.ILSpy.TextView
// Bookmarks live only on the decompiled C# view, not on IL / metadata / resource output.
bool ShowsBookmarkableCode => DataContext is DecompilerTabPageModel { SyntaxExtension: ".cs" };
Bookmarks.Bookmark? CreateBookmarkForLine(int line)
// Just the anchor for a line, or null when the line can't be bookmarked. Cheap enough to call
// per hovered line: it does not capture the editor view state (foldings, scroll, tree path),
// which only a bookmark that is actually being created needs.
Bookmarks.Bookmark? CreateAnchorForLine(int line)
{
if (!ShowsBookmarkableCode || DataContext is not DecompilerTabPageModel model)
return null;
var fallbackOwner = model.CurrentNode is IMemberTreeNode memberNode ? memberNode.Member : null;
var bookmark = Bookmarks.BookmarkAnchoring.CreateForLine(model.DebugInfo, model.References, Editor.Document, line,
return Bookmarks.BookmarkAnchoring.CreateForLine(model.DebugInfo, model.References, Editor.Document, line,
fallbackOwner, GetLocationNodeName(model.CurrentNode));
if (bookmark != null)
}
Bookmarks.Bookmark? CreateBookmarkForLine(int line)
{
var bookmark = CreateAnchorForLine(line);
if (bookmark != null && DataContext is DecompilerTabPageModel model)
{
var displaySettings = AppComposition.TryGetExport<SettingsService>()?.DisplaySettings;
var selectedTreeNodePath = AssemblyTreeModel.GetPathForNode(model.CurrentNode);
@ -706,7 +714,7 @@ namespace ICSharpCode.ILSpy.TextView @@ -706,7 +714,7 @@ namespace ICSharpCode.ILSpy.TextView
=> node is IMemberTreeNode { Member: { } member } ? member.FullName : node?.ToString();
/// <summary>Whether <paramref name="line"/> can hold a bookmark in the current document.</summary>
internal bool CanToggleBookmarkAtLine(int line) => CreateBookmarkForLine(line) != null;
internal bool CanToggleBookmarkAtLine(int line) => CreateAnchorForLine(line) != null;
internal bool CanToggleBookmarkAtOffset(int offset)
{
@ -855,6 +863,14 @@ namespace ICSharpCode.ILSpy.TextView @@ -855,6 +863,14 @@ namespace ICSharpCode.ILSpy.TextView
scrollViewer.Offset = new Vector(scrollViewer.Offset.X, Math.Max(0, target));
}
// The identity of the inputs that decide where bookmarks resolve in the current C# document.
// Both are replaced on every decompile, while the editor reuses one TextDocument (only its text
// changes), so this -- not the document reference -- is what tells the gutter its cache is stale.
internal (object? DebugInfo, object? References) BookmarkContentVersion
=> DataContext is DecompilerTabPageModel { SyntaxExtension: ".cs" } model
? (model.DebugInfo, model.References)
: default;
/// <summary>
/// The document line a bookmark sits on in the currently shown C# document, or null when the
/// bookmark belongs to other code. Body anchors resolve via the IL-offset map; token anchors

Loading…
Cancel
Save