I have a WPF application using this ListView:
<ListView ItemsSource="{Binding Log.Entries}"
VirtualizingPanel.IsVirtualizing="True"
VirtualizingPanel.IsVirtualizingWhenGrouping="True">
<i:Interaction.Behaviors>
<afutility:AutoScrollBehavior/>
</i:Interaction.Behaviors>
<ListView.ItemTemplate>
<DataTemplate>
<WrapPanel>
<TextBlock Foreground="{Binding Path=LogLevel, Mode=OneTime, Converter={StaticResource LogLevelToBrushConverter}}"
Text="{Binding Path=RenderedContent, Mode=OneTime}"/>
</WrapPanel>
</DataTemplate>
</ListView.ItemTemplate>
</ListView>
Log.Entries ends up at an instance of this class (the List itself is never replaced, it is always the same object):
public class RingList<T> : IList<T>, INotifyCollectionChanged, INotifyPropertyChanged
That class is essentially a custom list, which caps its content at 100 items. Adding an item at capacity removes an item from the head. For each added/removed item I call CollectionChanged
like this:
// For added items
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, Count - 1));
// For removed items (only ever removes from the start of the ring)
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, headItem, 0));
The items of the collection are NOT an object, they are a struct like this:
public struct RecordedLogEntry : IEquatable<RecordedLogEntry> {
public string RenderedContent { get; set; }
public Level LogLevel { get; set; }
// [...] Equals, GetHashCode, ToString, etc omitted for brevity, all standard
}
I am aware of the fact, that binding to non INotifyPropertyChange objects can cause memory leaks (see:
You can clearly see:
- The 100 items in the capped collection
- A little above that, 12 items referenced by the currently visible ListViewItem instances (since the list view is virtualizing items, this is about the expected amount)
- over 700k instances referenced by the ListView itself
- The resulting massive amount of data which accumulated over time
The project is using .NET 4.7.2 on Windows.
How can I avoid this leak?
Edit, disregard this requirement:
Ideally, I don't want to change away from a struct, as I have many of these items produced in the background (which don't all make it to the displayed 100 items) so I want to keep the footprint of the log entries small.
As @Joe rightfully pointed out, that is premature optimization. The fact remains that those log entries are not purely for UI display and are used elsewhere.
None of them change content at lifetime, so having an implementation to notify about changes seems counterintuitive.
Is there a way to make the binding not care about updates and do a real one time binding in this use case, or is the only option to add a wrapper class/copy the data into a class which implements INotifyPropertyChange
just so the memory leak goes away?
CodePudding user response:
Unfortunately the issue was not reproducible with a minimal example, so I have to assume the instances are kept alive elsewhere.
A minimal example which would normally show the leak easily, if it came from the bindings:
<Window x:Class="ListViewLeakRepro.MainWindow"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d"
Title="MainWindow" Height="450" Width="800">
<Grid>
<ListView Margin="10,46,10,10"
ItemsSource="{Binding Entries}"
VirtualizingPanel.IsVirtualizing="True"
VirtualizingPanel.IsVirtualizingWhenGrouping="True"
VirtualizingPanel.ScrollUnit="Pixel">
<ListView.ItemTemplate>
<DataTemplate>
<WrapPanel>
<TextBlock Text="{Binding Path=RenderedContent, Mode=OneTime}"/>
</WrapPanel>
</DataTemplate>
</ListView.ItemTemplate>
</ListView>
<Button Content="Start Indirect Log Spam" HorizontalAlignment="Left" Margin="10,10,0,0" VerticalAlignment="Top" Click="StartTimerIndirect"/>
<Button Content="Start Direct Log Spam" HorizontalAlignment="Left" Margin="143,10,0,0" VerticalAlignment="Top" Click="StartTimerDirect"/>
</Grid>
</Window>
Code behind:
using System.Windows;
namespace ListViewLeakRepro {
/// <summary>
/// Interaction logic for MainWindow.xaml
/// </summary>
public partial class MainWindow : Window {
MainWindowModel Model { get; }
public MainWindow() {
InitializeComponent();
Model = new MainWindowModel();
DataContext = Model;
}
private void StartTimerIndirect(object sender, RoutedEventArgs e) {
Model.StartTimerIndirect();
}
private void StartTimerDirect(object sender, RoutedEventArgs e) {
Model.StartTimerDirect();
}
}
}
Model:
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Windows;
using System.Windows.Threading;
namespace ListViewLeakRepro {
public class MainWindowModel : INotifyPropertyChanged {
public event PropertyChangedEventHandler PropertyChanged;
public RingList<RecordedLogEntry> Entries { get; } = new RingList<RecordedLogEntry>(100);
private static List<RecordedLogEntry> pollCache = new List<RecordedLogEntry>();
private static List<RecordedLogEntry> pollCache2 = new List<RecordedLogEntry>();
private DispatcherTimer timer;
private DispatcherTimer timer2;
private int logCounter = 0;
private static readonly object lockObject = new object();
public void StartTimerIndirect() {
if (timer != null) {
return;
}
timer = new DispatcherTimer(TimeSpan.FromMilliseconds(10), DispatcherPriority.Background, SpamLogIndirect, Application.Current.Dispatcher);
timer2 = new DispatcherTimer(TimeSpan.FromMilliseconds(500), DispatcherPriority.Background, RefreshLogDisplay, Application.Current.Dispatcher);
}
public void StartTimerDirect() {
if (timer != null) {
return;
}
timer = new DispatcherTimer(TimeSpan.FromMilliseconds(10), DispatcherPriority.Background, SpamLogDirect, Application.Current.Dispatcher);
}
private void SpamLogIndirect(object sender, EventArgs e) {
// Add some invisible junk data to have results earlier
byte[] junkData = new byte[50 * 1024 * 1024];
lock (lockObject) {
pollCache.Add(new RecordedLogEntry($"Entry { logCounter}", junkData));
}
}
private void SpamLogDirect(object sender, EventArgs e) {
// Add some invisible junk data to have results earlier
byte[] junkData = new byte[50 * 1024 * 1024];
lock (lockObject) {
Entries.Add(new RecordedLogEntry($"Entry { logCounter}", junkData));
}
}
private void RefreshLogDisplay(object sender, EventArgs e) {
lock (lockObject) {
// Swap background buffer
(pollCache, pollCache2) = (pollCache2, pollCache);
}
foreach (RecordedLogEntry item in pollCache2) {
Entries.Add(item);
}
pollCache2.Clear();
}
}
}
The RingList class:
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.ComponentModel;
namespace ListViewLeakRepro {
/// <summary>
/// A collection with a fixed buffer size which is reused when full.
/// Only the most recent items are retained, the oldest are overwritten,
/// always keeping a constant amount of items in the collection without reallocating memory.
/// </summary>
/// <typeparam name="T"></typeparam>
public class RingList<T> : IList<T>, INotifyCollectionChanged, INotifyPropertyChanged {
private T[] buffer;
private int head;
private int tail;
private bool firstRevolution;
public event NotifyCollectionChangedEventHandler CollectionChanged;
public event PropertyChangedEventHandler PropertyChanged;
public RingList(int capacity) {
buffer = new T[capacity];
firstRevolution = true;
}
public int Capacity {
get {
return buffer.Length;
}
set {
T[] oldBuffer = buffer;
int oldHead = head;
// int oldTail = tail;
int oldLength = Count;
buffer = new T[value];
head = 0;
tail = 0;
for (int i = 0; i < oldLength; i ) {
Add(oldBuffer[(oldHead i) % oldBuffer.Length]);
}
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Capacity)));
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
}
}
public int IndexOf(T item) {
for (int i = 0; i < Count; i ) {
if (Equals(this[i], item)) {
return i;
}
}
return -1;
}
public void Insert(int index, T item) => throw new NotImplementedException();
public void RemoveAt(int index) => throw new NotImplementedException();
public bool Remove(T item) => throw new NotImplementedException();
public T this[int index] {
get {
if (index < 0 || index >= Count) {
throw new IndexOutOfRangeException();
}
int actualIndex = (index head) % Capacity;
return buffer[actualIndex];
}
set {
if (index < 0 || index > Count) {
throw new IndexOutOfRangeException();
}
int actualIndex = (index head) % Capacity;
T previous = buffer[actualIndex];
buffer[actualIndex] = value;
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, value, previous, index));
}
}
public void Add(T item) {
if (Count == Capacity) {
RemoveHead();
}
buffer[tail] = item;
tail ;
if (tail == Capacity) {
tail = 0;
head = 0;
firstRevolution = false;
}
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, Count - 1));
}
private void RemoveHead() {
if (Count == 0) {
throw new InvalidOperationException("Cannot remove from an empty collection");
}
T headItem = buffer[head];
head ;
if (head == Capacity) {
head = 0;
}
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, headItem, 0));
}
public void Clear() {
buffer = new T[buffer.Length];
head = 0;
tail = 0;
firstRevolution = true;
CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
}
public bool Contains(T item) => IndexOf(item) >= 0;
public void CopyTo(T[] array, int arrayIndex) => throw new NotImplementedException();
public void CopyTo(List<T> collection) {
if (collection == null) {
throw new ArgumentNullException(nameof(collection));
}
for (int i = 0; i < Count; i ) {
collection.Add(this[i]);
}
}
public int Count {
get {
if (tail == head) {
if (firstRevolution) {
return 0;
} else {
return Capacity;
}
} else if (tail < head) {
return Capacity - head tail;
} else {
return tail - head;
}
}
}
public bool IsReadOnly => false;
public IEnumerator<T> GetEnumerator() {
int position = 0; // state
while (position < Count) {
yield return this[position];
position ;
}
}
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}
The items of the collection:
namespace ListViewLeakRepro {
public class RecordedLogEntry {
public RecordedLogEntry(string renderedContent, byte[] junkData) {
RenderedContent = renderedContent;
JunkData = junkData;
}
public string RenderedContent { get; set; }
public byte[] JunkData { get; set; }
}
}
Analysis with above reproduction code
Starting the simulation using "Direct" log spam, i.e. directly writing to Entries
does NOT cause a leak.
It consumes lots of memory, but GC is able to clear it:
Starting the simulation using "Indirect" log spam, i.e. collecting entries in a buffer which is rotated every 500ms, seemingly causes a memory leak. GC is not clearing it right away:
But eventually it does, it just takes longer:
Conclusion: Not a leak in the binding, more likely an unbounded growth in one of the buffers or some other place keeping the instance alive.
Side note: changing from class to struct does not significantly impact the situation (may change overall consumption, but does not cause or solve the issue).