Skip to content

Commit

Permalink
Explore weird WPF performance issue
Browse files Browse the repository at this point in the history
(This change doesn't fix anything.  It just saves a few tweaks made
while fiddling with the problem.)

Steps to reproduce:

1. Open a large archive in full-list display mode.  I used
   a2gemII.iso.  Make sure the window is wide enough to show all
   columns in the file list.
2. Using the scroll bar, scroll down about halfway through the
   archive.
3. Close and reopen the archive.

When the file is reopened, the program appears to stall, and the
debugger shows a constant series of GCs.  Pausing the thread to
check the stack trace shows different things on each attempt, but
always somewhere deep in WPF GUI code.  It usually seems to have
something to do with dependency object property changed events,
often with DataGridRow referenced along the way.

The effect is to make opening the archive sluggish.  It does not
recover until a GUI element is clicked on, at which time it stops
immediately.  The file list is scrolled all the way to the bottom,
which is not the usual behavior when a file is opened.

The size of the archive does matter.  With smaller archives, or with
a large archive viewed in single-directory mode, the problem doesn't
happen.

The width of the main window also matters.  If the window is wide enough
to show past the end of the file list DataGrid, the problem happens.  If
the window cuts off a little bit of the last column, the problem doesn't
happen.

My best guess is this has something to do with DataGrid virtualization,
which is fraught with peril on a good day.  Changing fileListDataGrid
VirtualizingStackPanel.VirtualizationMode from "Recycling" to
"Standard" prevents the problem from occurring, at the expense of
making the archive display generally slower.

Note that using "Standard" doesn't entirely fix the problem.  When
the reproduction steps are followed, the load process will appear to
stall for a few seconds, with the list scrolled all the way to the end.
However, it will recover, and move the list position to the top.  Again,
partially obscuring the Access column fixes the issue.

(I tried setting the width of the Access column to "*" to possibly work
around the problem, but for some reason that doesn't work.)

Second bug:

1. Cause the problem as above.  Click on a GUI element to un-freeze.
2. Move the right edge of the window so the Access column is partly
   obscured.
3. Close and reopen.  When scrolling down, the leftmost (status icon)
   column will sometimes have a gray rectangle in it that offsets the
   line to the right, screwing up all the vertical lines.

The extra rect appears and disappears when lines are scrolled around,
again suggesting the involvement of the element recycling system.
  • Loading branch information
fadden committed Nov 13, 2023
1 parent fe12eb4 commit 3f97bd1
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 17 deletions.
2 changes: 1 addition & 1 deletion CommonUtil/Formatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public class FormatConfig {
}
access <<= 1;
}
return sb.ToString();
return string.Intern(sb.ToString()); // most entries will be similar
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion cp2_wpf/FileListItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public class FileListItem {
/// that isn't available in the Mac OS Roman character set, and that looks nicer than
/// four NUL control picture glyphs.
/// </summary>
private const string NO_TYPE = "\u23af\u23af\u23af\u23af";
private const string NO_TYPE = "\u23af\u23af\u23af\u23af"; // HORIZONTAL LINE EXTENSION

private static readonly ControlTemplate sInvalidIcon =
(ControlTemplate)Application.Current.FindResource("icon_StatusInvalid");
Expand Down
5 changes: 3 additions & 2 deletions cp2_wpf/FileViewer.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ private class ConverterComboItem {
} else if (mCurDataOutput is IBitmap) {
IBitmap bitmap = (IBitmap)mCurDataOutput;
previewImage.Source = WinUtil.ConvertToBitmapSource(bitmap);
Debug.Assert(previewImage.Source.IsFrozen);
ConfigureMagnification();
SetDisplayType(DisplayItemType.Bitmap);

Expand Down Expand Up @@ -626,8 +627,8 @@ private class ConverterComboItem {

previewImage.Width = Math.Floor(bitmap.Width * mult);
previewImage.Height = Math.Floor(bitmap.Height * mult);
Debug.WriteLine("Gfx zoom " + mult + " --> " +
previewImage.Width + "x" + previewImage.Height);
//Debug.WriteLine("Gfx zoom " + mult + " --> " +
// previewImage.Width + "x" + previewImage.Height);
}

#region Export and Clip
Expand Down
5 changes: 3 additions & 2 deletions cp2_wpf/MainController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ public enum AutoOpenDepth { Unknown = 0, Shallow, SubVol, Max }
} finally {
Mouse.OverrideCursor = null;
}
AppHook.LogI("Load of '" + pathName + "' completed");

mWorkPathName = pathName;
UpdateTitle();
Expand Down Expand Up @@ -762,8 +763,8 @@ public enum AutoOpenDepth { Unknown = 0, Shallow, SubVol, Max }
}
Debug.WriteLine("Closing " + mWorkPathName);

// Flush and close all disk images and file archives.
ClearArchiveTree();
// Flush and close all disk images and file archives. Purge GUI elements.
mMainWin.ClearTreesAndLists();
// Close the host file.
mWorkTree.Dispose();
mWorkTree = null;
Expand Down
10 changes: 2 additions & 8 deletions cp2_wpf/MainController_Panels.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,6 @@ public partial class MainController {
}
}

/// <summary>
/// Clears the contents of the archive tree.
/// </summary>
private void ClearArchiveTree() {
mMainWin.ArchiveTreeRoot.Clear();
}

/// <summary>
/// Prepares a work file for use.
/// </summary>
Expand Down Expand Up @@ -961,7 +954,8 @@ public partial class MainController {

// Evalute this file to see if it could be a disk image or file archive. If it
// is, add it to the work tree, and select it.
WorkTree.Node? newNode = mWorkTree!.TryCreateSub(arcTreeSel.WorkTreeNode, entry);
WorkTree.Node? newNode =
mWorkTree!.TryCreateSub(arcTreeSel.WorkTreeNode, entry);
if (newNode != null) {
// Successfully opened. Update the TreeView.
ArchiveTreeItem newItem =
Expand Down
25 changes: 22 additions & 3 deletions cp2_wpf/MainWindow.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,11 @@ public partial class MainWindow : Window, INotifyPropertyChanged {
/// <summary>
/// Data for archive tree TreeView control. This is a list of items, not a single item.
/// </summary>
public ObservableCollection<ArchiveTreeItem> ArchiveTreeRoot { get; private set; } =
public ObservableCollection<ArchiveTreeItem> ArchiveTreeRoot {
get { return mArchiveTreeRoot; }
private set { mArchiveTreeRoot = value; OnPropertyChanged(); }
}
private ObservableCollection<ArchiveTreeItem> mArchiveTreeRoot =
new ObservableCollection<ArchiveTreeItem>();

/// <summary>
Expand All @@ -680,14 +684,29 @@ public partial class MainWindow : Window, INotifyPropertyChanged {
mMainCtrl.ArchiveTree_SelectionChanged(item);
}

/// <summary>
/// Clears the contents of the trees and file list.
/// </summary>
internal void ClearTreesAndLists() {
ArchiveTreeRoot.Clear();
DirectoryTreeRoot.Clear();
FileList.Clear();

UpdateLayout();
}

#endregion Archive Tree

#region Directory Tree

/// <summary>
/// Data for directory tree TreeView control.
/// </summary>
public ObservableCollection<DirectoryTreeItem> DirectoryTreeRoot { get; private set; } =
public ObservableCollection<DirectoryTreeItem> DirectoryTreeRoot {
get { return mDirectoryTreeRoot; }
private set { mDirectoryTreeRoot = value; OnPropertyChanged(); }
}
private ObservableCollection<DirectoryTreeItem> mDirectoryTreeRoot =
new ObservableCollection<DirectoryTreeItem>();

/// <summary>
Expand Down Expand Up @@ -883,7 +902,7 @@ public enum CenterPanelChange { Unknown = 0, Files, Info, Toggle }

public ObservableCollection<FileListItem> FileList {
get { return mFileList; }
internal set { mFileList = value; OnPropertyChanged(); }
private set { mFileList = value; OnPropertyChanged(); }
}
private ObservableCollection<FileListItem> mFileList =
new ObservableCollection<FileListItem>();
Expand Down

0 comments on commit 3f97bd1

Please sign in to comment.