Skip to content

Commit

Permalink
Merge pull request #31 from frenzibyte/metal-remove-cb-locks
Browse files Browse the repository at this point in the history 
Remove Metal staging buffers cache to avoid lock contention overhead in command buffer completion handler
  • Loading branch information
@peppy
peppy authored Jul 10, 2023
2 parents b84af63 + d8d382c commit 9be64ed
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 107 deletions.
66 changes: 2 additions & 64 deletions src/Veldrid/MTL/MTLCommandList.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ internal unsafe class MTLCommandList : CommandList
private bool[] _vertexBuffersActive;
private bool _disposed;

private readonly List<MTLBuffer> _availableStagingBuffers = new List<MTLBuffer>();
private readonly Dictionary<MTLCommandBuffer, List<MTLBuffer>> _submittedStagingBuffers = new Dictionary<MTLCommandBuffer, List<MTLBuffer>>();
private readonly object _submittedCommandsLock = new object();

public MTLCommandBuffer CommandBuffer => _cb;

public MTLCommandList(ref CommandListDescription description, MTLGraphicsDevice gd)
Expand Down Expand Up @@ -357,7 +353,7 @@ private protected override void UpdateBufferCore(DeviceBuffer buffer, uint buffe
|| (sizeInBytes % 4 != 0 && bufferOffsetInBytes != 0 && sizeInBytes != buffer.SizeInBytes);

MTLBuffer dstMTLBuffer = Util.AssertSubtype<DeviceBuffer, MTLBuffer>(buffer);
MTLBuffer staging = GetFreeStagingBuffer(sizeInBytes);
MTLBuffer staging = (MTLBuffer)_gd.ResourceFactory.CreateBuffer(new BufferDescription(sizeInBytes, BufferUsage.Staging));

_gd.UpdateBuffer(staging, 0, source, sizeInBytes);

Expand All @@ -376,47 +372,7 @@ private protected override void UpdateBufferCore(DeviceBuffer buffer, uint buffe
(UIntPtr)(sizeInBytes + sizeRoundFactor));
}

lock (_submittedCommandsLock)
{
if (!_submittedStagingBuffers.TryGetValue(_cb, out List<MTLBuffer> bufferList))
{
_submittedStagingBuffers[_cb] = bufferList = new List<MTLBuffer>();
}

bufferList.Add(staging);
}
}

private MTLBuffer GetFreeStagingBuffer(uint sizeInBytes)
{
lock (_submittedCommandsLock)
{
foreach (MTLBuffer buffer in _availableStagingBuffers)
{
if (buffer.SizeInBytes >= sizeInBytes)
{
_availableStagingBuffers.Remove(buffer);
return buffer;
}
}
}

DeviceBuffer staging = _gd.ResourceFactory.CreateBuffer(
new BufferDescription(sizeInBytes, BufferUsage.Staging));

return Util.AssertSubtype<DeviceBuffer, MTLBuffer>(staging);
}

public void OnCompleted(MTLCommandBuffer cb)
{
lock (_submittedCommandsLock)
{
if (_submittedStagingBuffers.TryGetValue(cb, out List<MTLBuffer> bufferList))
{
_availableStagingBuffers.AddRange(bufferList);
_submittedStagingBuffers.Remove(cb);
}
}
staging.Dispose();
}

protected override void CopyBufferCore(
Expand Down Expand Up @@ -1278,24 +1234,6 @@ public override void Dispose()
_disposed = true;
EnsureNoRenderPass();

lock (_submittedStagingBuffers)
{
foreach (MTLBuffer buffer in _availableStagingBuffers)
{
buffer.Dispose();
}

foreach (KeyValuePair<MTLCommandBuffer, List<MTLBuffer>> kvp in _submittedStagingBuffers)
{
foreach (MTLBuffer buffer in kvp.Value)
{
buffer.Dispose();
}
}

_submittedStagingBuffers.Clear();
}

if (_cb.NativePtr != IntPtr.Zero)
{
ObjectiveCRuntime.release(_cb.NativePtr);
Expand Down
47 changes: 4 additions & 43 deletions src/Veldrid/MTL/MTLGraphicsDevice.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,6 @@ private static readonly Dictionary<IntPtr, MTLGraphicsDevice> s_aotRegisteredBlo
private readonly bool[] _supportedSampleCounts;
private BackendInfoMetal _metalInfo;

private readonly object _submittedCommandsLock = new object();
private readonly Dictionary<MTLCommandBuffer, MTLCommandList> _submittedCLs = new Dictionary<MTLCommandBuffer, MTLCommandList>();
private MTLCommandBuffer _latestSubmittedCB;

private readonly object _resetEventsLock = new object();
private readonly List<ManualResetEvent[]> _resetEvents = new List<ManualResetEvent[]>();

private const string UnalignedBufferCopyPipelineMacOSName = "MTL_UnalignedBufferCopy_macOS";
private const string UnalignedBufferCopyPipelineiOSName = "MTL_UnalignedBufferCopy_iOS";
private readonly object _unalignedBufferCopyPipelineLock = new object();
Expand Down Expand Up @@ -201,22 +194,7 @@ public MTLGraphicsDevice(

public override GraphicsDeviceFeatures Features { get; }

private void OnCommandBufferCompleted(IntPtr block, MTLCommandBuffer cb)
{
lock (_submittedCommandsLock)
{
MTLCommandList cl = _submittedCLs[cb];
_submittedCLs.Remove(cb);
cl.OnCompleted(cb);

if (_latestSubmittedCB.NativePtr == cb.NativePtr)
{
_latestSubmittedCB = default(MTLCommandBuffer);
}
}

ObjectiveCRuntime.release(cb.NativePtr);
}
private void OnCommandBufferCompleted(IntPtr block, MTLCommandBuffer cb) => ObjectiveCRuntime.release(cb.NativePtr);

// Xamarin AOT requires native callbacks be static.
[MonoPInvokeCallback(typeof(MTLCommandBufferHandler))]
Expand All @@ -242,12 +220,7 @@ private protected override void SubmitCommandsCore(CommandList commandList, Fenc
}

mtlCL.CommandBuffer.addCompletedHandler(_completionBlockLiteral);

lock (_submittedCommandsLock)
{
_submittedCLs.Add(mtlCL.CommandBuffer, mtlCL);
_latestSubmittedCB = mtlCL.Commit();
}
mtlCL.Commit();
}

private protected override void WaitForNextFrameReadyCore()
Expand Down Expand Up @@ -440,20 +413,8 @@ private protected override void UpdateTextureCore(

private protected override void WaitForIdleCore()
{
MTLCommandBuffer lastCB = default(MTLCommandBuffer);

lock (_submittedCommandsLock)
{
lastCB = _latestSubmittedCB;
ObjectiveCRuntime.retain(lastCB.NativePtr);
}

if (lastCB.NativePtr != IntPtr.Zero && lastCB.status != MTLCommandBufferStatus.Completed)
{
lastCB.waitUntilCompleted();
}

ObjectiveCRuntime.release(lastCB.NativePtr);
// todo: this may not work as it's not guaranteed that _latestSubmittedCB hasn't already been released yet.
throw new NotSupportedException("WaitForIdle is not supported on Metal currently. Use a fence and spin on the signal (because WaitForFence is also unsupported).");
}

protected override MappedResource MapCore(MappableResource resource, MapMode mode, uint subresource)
Expand Down

0 comments on commit 9be64ed

Please sign in to comment.