Skip to content

Commit 8a996e3

Browse files
authored
Use HashSet to track GraphicsDevice resources (MonoGame#9027)
<!-- Are you targeting develop? All PRs should target the develop branch unless otherwise noted. --> Improves performance of `GraphicsDevice.RemoveResourceReference` (and by extension `GraphicsResource.Dispose`) when there are a lot of resources. <!-- Please try to make sure that there is a bug logged for the issue being fixed if one is not present. Especially for issues which are complex. The bug should describe the problem and how to reproduce it. --> ### Description of Change `List<T>.Remove(T item)` needs to iterate every item of the list to find the one to remove, then shift items when removing from the middle. HashSet<T> has much faster deletion, at the cost of a slightly slower insert. My specific example had over 30000 resources, and triggered large stutters when clearing a cache with around 4000 recently allocated items. It was iterating approximately 120000000 resources removing these entries, since they would be at the end of the list. My test case was a little extreme since I was scrolling a map with a huge number of unique models that stream in+out, but it's an easy improvement to make. <!-- Enter description of the fix in this section. Please be as descriptive as possible, future contributors will need to know *why* these changes are being made. For inspiration review the commit/PR history in the MonoGame repository. -->
1 parent bcc73c6 commit 8a996e3

2 files changed

Lines changed: 31 additions & 3 deletions

File tree

MonoGame.Framework/Graphics/GraphicsDevice.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
using System.Diagnostics;
88
using System.Globalization;
99
using MonoGame.Framework.Utilities;
10-
using System.Runtime.InteropServices;
10+
using System.Runtime.InteropServices;
11+
using System.Linq;
1112

1213

1314
namespace Microsoft.Xna.Framework.Graphics
@@ -154,7 +155,7 @@ private bool PixelShaderDirty
154155
// Use WeakReference for the global resources list as we do not know when a resource
155156
// may be disposed and collected. We do not want to prevent a resource from being
156157
// collected by holding a strong reference to it in this list.
157-
private readonly List<WeakReference> _resources = new List<WeakReference>();
158+
private readonly HashSet<WeakReference> _resources = new HashSet<WeakReference>();
158159

159160
// TODO Graphics Device events need implementing
160161
/// <summary>
@@ -804,7 +805,7 @@ internal void OnDeviceResetting()
804805
}
805806

806807
// Remove references to resources that have been garbage collected.
807-
_resources.RemoveAll(wr => !wr.IsAlive);
808+
_resources.RemoveWhere(wr => !wr.IsAlive);
808809
}
809810
}
810811

Tests/Framework/Graphics/GraphicsDeviceTest.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,5 +852,32 @@ public void GetBackBufferData(Rectangle? rectangle)
852852

853853
CheckFrames();
854854
}
855+
856+
[Test]
857+
[RunOnUI]
858+
public void DisposeReferencedResources()
859+
{
860+
var rt = new RenderTarget2D(gdm.GraphicsDevice, 5, 5);
861+
var vb = new DynamicVertexBuffer(gd, VertexPositionColor.VertexDeclaration, 1, BufferUsage.None);
862+
863+
var middleIb = new DynamicIndexBuffer(gd, IndexElementSize.SixteenBits, 1, BufferUsage.None);
864+
865+
var ib = new DynamicIndexBuffer(gd, IndexElementSize.SixteenBits, 1, BufferUsage.None);
866+
var rtc = new RenderTargetCube(gd, 1, false, SurfaceFormat.Color, DepthFormat.Depth16);
867+
868+
middleIb.Dispose();
869+
Assert.IsTrue(middleIb.IsDisposed);
870+
871+
// Remaining referenced resources should be disposed.
872+
gd.Dispose();
873+
874+
Assert.IsTrue(rt.IsDisposed);
875+
876+
Assert.IsTrue(vb.IsDisposed);
877+
878+
Assert.IsTrue(ib.IsDisposed);
879+
880+
Assert.IsTrue(rtc.IsDisposed);
881+
}
855882
}
856883
}

0 commit comments

Comments
 (0)