From b2b639434c53fd122661f4551d7eb01aa7e11e52 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sun, 11 Oct 2020 09:46:41 +0100 Subject: [PATCH] ThreadedGraphicsContext improvements. - VertexBuffer interface redefined to remove an IntPtr overload for SetData. This removes some unsafe code in TerrainSpriteLayer. This also allows the ThreadedVertexBuffer to use a buffer and post these calls, meaning the SetData call can now be non-blocking. - ThreadedTexture SetData now checks the incoming array size. As the arrays sent here are usually large (megabytes) this allows us to avoid creating temp arrays in the LOH and skip Array.Copy calls on large arrays. This means the call is now blocking more often, but significantly reduces memory churn and GC Gen2 collections. --- OpenRA.Game/Graphics/PlatformInterfaces.cs | 3 +- OpenRA.Game/Graphics/TerrainSpriteLayer.cs | 10 +-- .../ThreadedGraphicsContext.cs | 61 ++++++++++++------- OpenRA.Platforms.Default/VertexBuffer.cs | 16 +---- 4 files changed, 44 insertions(+), 46 deletions(-) diff --git a/OpenRA.Game/Graphics/PlatformInterfaces.cs b/OpenRA.Game/Graphics/PlatformInterfaces.cs index d879825591..f4fc561b4f 100644 --- a/OpenRA.Game/Graphics/PlatformInterfaces.cs +++ b/OpenRA.Game/Graphics/PlatformInterfaces.cs @@ -103,8 +103,7 @@ namespace OpenRA { void Bind(); void SetData(T[] vertices, int length); - void SetData(T[] vertices, int start, int length); - void SetData(IntPtr data, int start, int length); + void SetData(T[] vertices, int offset, int start, int length); } public interface IShader diff --git a/OpenRA.Game/Graphics/TerrainSpriteLayer.cs b/OpenRA.Game/Graphics/TerrainSpriteLayer.cs index 47344a36f4..5794f63834 100644 --- a/OpenRA.Game/Graphics/TerrainSpriteLayer.cs +++ b/OpenRA.Game/Graphics/TerrainSpriteLayer.cs @@ -183,15 +183,7 @@ namespace OpenRA.Graphics continue; var rowOffset = rowStride * row; - - unsafe - { - // The compiler / language spec won't let us calculate a pointer to - // an offset inside a generic array T[], and so we are forced to - // calculate the start-of-row pointer here to pass in to SetData. - fixed (Vertex* vPtr = &vertices[0]) - vertexBuffer.SetData((IntPtr)(vPtr + rowOffset), rowOffset, rowStride); - } + vertexBuffer.SetData(vertices, rowOffset, rowOffset, rowStride); } Game.Renderer.WorldSpriteRenderer.DrawVertexBuffer( diff --git a/OpenRA.Platforms.Default/ThreadedGraphicsContext.cs b/OpenRA.Platforms.Default/ThreadedGraphicsContext.cs index c55e2d9c8f..593c98810e 100644 --- a/OpenRA.Platforms.Default/ThreadedGraphicsContext.cs +++ b/OpenRA.Platforms.Default/ThreadedGraphicsContext.cs @@ -29,9 +29,9 @@ namespace OpenRA.Platforms.Default readonly Stack messagePool = new Stack(); readonly Queue messages = new Queue(); + public readonly int BatchSize; readonly object syncObject = new object(); readonly Thread renderThread; - readonly int batchSize; volatile ExceptionDispatchInfo messageException; // Delegates that perform actions on the real device. @@ -53,7 +53,7 @@ namespace OpenRA.Platforms.Default public ThreadedGraphicsContext(Sdl2GraphicsContext context, int batchSize) { - this.batchSize = batchSize; + BatchSize = batchSize; renderThread = new Thread(RenderThread) { Name = "ThreadedGraphicsContext RenderThread", @@ -143,15 +143,15 @@ namespace OpenRA.Platforms.Default internal Vertex[] GetVertices(int size) { lock (verticesPool) - if (size <= batchSize && verticesPool.Count > 0) + if (size <= BatchSize && verticesPool.Count > 0) return verticesPool.Pop(); - return new Vertex[size < batchSize ? batchSize : size]; + return new Vertex[size < BatchSize ? BatchSize : size]; } internal void ReturnVertices(Vertex[] vertices) { - if (vertices.Length == batchSize) + if (vertices.Length == BatchSize) lock (verticesPool) verticesPool.Push(vertices); } @@ -513,7 +513,8 @@ namespace OpenRA.Platforms.Default readonly ThreadedGraphicsContext device; readonly Action bind; readonly Action setData1; - readonly Func setData2; + readonly Action setData2; + readonly Func setData3; readonly Action dispose; public ThreadedVertexBuffer(ThreadedGraphicsContext device, IVertexBuffer vertexBuffer) @@ -521,7 +522,8 @@ namespace OpenRA.Platforms.Default this.device = device; bind = vertexBuffer.Bind; setData1 = tuple => { var t = (ValueTuple)tuple; vertexBuffer.SetData(t.Item1, t.Item2); device.ReturnVertices(t.Item1); }; - setData2 = tuple => { var t = (ValueTuple)tuple; vertexBuffer.SetData(t.Item1, t.Item2, t.Item3); return null; }; + setData2 = tuple => { var t = (ValueTuple)tuple; vertexBuffer.SetData(t.Item1, t.Item2, t.Item3, t.Item4); device.ReturnVertices(t.Item1); }; + setData3 = tuple => { setData2(tuple); return null; }; dispose = vertexBuffer.Dispose; } @@ -537,17 +539,20 @@ namespace OpenRA.Platforms.Default device.Post(setData1, (buffer, length)); } - public void SetData(IntPtr data, int start, int length) + public void SetData(Vertex[] vertices, int offset, int start, int length) { - // We can't return until we are finished with the data, so we must Send here. - device.Send(setData2, (data, start, length)); - } - - public void SetData(Vertex[] vertices, int start, int length) - { - var buffer = device.GetVertices(length); - Array.Copy(vertices, start, buffer, 0, length); - device.Post(setData1, (buffer, length)); + if (length <= device.BatchSize) + { + // If we are able to use a buffer without allocation, post a message to avoid blocking. + var buffer = device.GetVertices(length); + Array.Copy(vertices, offset, buffer, 0, length); + device.Post(setData2, (buffer, 0, start, length)); + } + else + { + // If the length is too large for a buffer, send a message and block to avoid allocations. + device.Send(setData3, (vertices, offset, start, length)); + } } public void Dispose() @@ -567,6 +572,7 @@ namespace OpenRA.Platforms.Default readonly Func getData; readonly Func setData1; readonly Action setData2; + readonly Func setData3; readonly Action dispose; public ThreadedTexture(ThreadedGraphicsContext device, ITextureInternal texture) @@ -580,6 +586,7 @@ namespace OpenRA.Platforms.Default getData = () => texture.GetData(); setData1 = colors => { texture.SetData((uint[,])colors); return null; }; setData2 = tuple => { var t = (ValueTuple)tuple; texture.SetData(t.Item1, t.Item2, t.Item3); }; + setData3 = tuple => { setData2(tuple); return null; }; dispose = texture.Dispose; } @@ -630,11 +637,21 @@ namespace OpenRA.Platforms.Default public void SetData(byte[] colors, int width, int height) { - // This creates some garbage for the GC to clean up, - // but allows us post a message instead of blocking the message queue by sending it. - var temp = new byte[colors.Length]; - Array.Copy(colors, temp, temp.Length); - device.Post(setData2, (temp, width, height)); + // Objects 85000 bytes or more will be directly allocated in the Large Object Heap (LOH). + // https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/large-object-heap + if (colors.Length < 85000) + { + // If we are able to create a small array the GC can collect easily, post a message to avoid blocking. + var temp = new byte[colors.Length]; + Array.Copy(colors, temp, temp.Length); + device.Post(setData2, (temp, width, height)); + } + else + { + // If the length is large and would result in an array on the Large Object Heap (LOH), + // send a message and block to avoid LOH allocation as this requires a Gen2 collection. + device.Send(setData3, (colors, width, height)); + } } public void Dispose() diff --git a/OpenRA.Platforms.Default/VertexBuffer.cs b/OpenRA.Platforms.Default/VertexBuffer.cs index 2b54f2ffb3..e6759add36 100644 --- a/OpenRA.Platforms.Default/VertexBuffer.cs +++ b/OpenRA.Platforms.Default/VertexBuffer.cs @@ -57,10 +57,10 @@ namespace OpenRA.Platforms.Default public void SetData(T[] data, int length) { - SetData(data, 0, length); + SetData(data, 0, 0, length); } - public void SetData(T[] data, int start, int length) + public void SetData(T[] data, int offset, int start, int length) { Bind(); @@ -70,7 +70,7 @@ namespace OpenRA.Platforms.Default OpenGL.glBufferSubData(OpenGL.GL_ARRAY_BUFFER, new IntPtr(VertexSize * start), new IntPtr(VertexSize * length), - ptr.AddrOfPinnedObject()); + ptr.AddrOfPinnedObject() + VertexSize * offset); } finally { @@ -80,16 +80,6 @@ namespace OpenRA.Platforms.Default OpenGL.CheckGLError(); } - public void SetData(IntPtr data, int start, int length) - { - Bind(); - OpenGL.glBufferSubData(OpenGL.GL_ARRAY_BUFFER, - new IntPtr(VertexSize * start), - new IntPtr(VertexSize * length), - data); - OpenGL.CheckGLError(); - } - public void Bind() { VerifyThreadAffinity();