Skip to content

Commit

Permalink
Rework LockGuard to avoid allocations when the lock isn't contested
Browse files Browse the repository at this point in the history
  • Loading branch information
js6pak committed Dec 17, 2023
1 parent d91ab5c commit 43acae1
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 69 deletions.
7 changes: 4 additions & 3 deletions src/Impostor.Api/Games/IGame.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Impostor.Api.Innersloth.GameOptions;
using Impostor.Api.Net;
using Impostor.Api.Net.Inner;
using Impostor.Api.Utils;

namespace Impostor.Api.Games
{
Expand Down Expand Up @@ -53,10 +54,10 @@ public interface IGame
T? FindObjectByNetId<T>(uint netId)
where T : IInnerNetObject;

/// <summary>Lock an IGame to prevent concurrent modification.</summary>
/// <returns>A <see cref="LockGuard" /> that should be awaited before and disposed after modifying the game.</returns>
/// <summary>Lock an <see cref="IGame"/> to prevent concurrent modification.</summary>
/// <returns>A <see cref="AsyncLock.Releaser" /> that should be disposed after modifying the game.</returns>
/// <remarks>This method should be called when modifying a game outside of an <see cref="IGameEvent" /> handler.</remarks>
Task<LockGuard> LockAsync();
ValueTask<AsyncLock.Releaser> LockAsync();

/// <summary>
/// Adds an <see cref="IPAddress" /> to the ban list of this game.
Expand Down
61 changes: 0 additions & 61 deletions src/Impostor.Api/Net/LockGuard.cs

This file was deleted.

71 changes: 71 additions & 0 deletions src/Impostor.Api/Utils/AsyncLock.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
using System;
using System.Threading;
using System.Threading.Tasks;

namespace Impostor.Api.Utils;

/// <summary>
/// Represents an asynchronous lock mechanism.
/// </summary>
public sealed class AsyncLock
{
private readonly SemaphoreSlim _semaphore = new(1, 1);

/// <summary>
/// Asynchronously acquires a lock.
/// </summary>
/// <returns>
/// A <see cref="ValueTask{Releaser}"/> that represents the asynchronous lock operation.
/// The result of the task is a <see cref="Releaser"/> that should be disposed to release the lock.
/// </returns>
public ValueTask<Releaser> LockAsync()
{
return LockAsync(CancellationToken.None);
}

/// <inheritdoc cref="LockAsync()"/>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to observe while waiting for the lock.</param>
/// <exception cref="T:System.OperationCanceledException"><paramref name="cancellationToken" /> was canceled.</exception>
public ValueTask<Releaser> LockAsync(CancellationToken cancellationToken)
{
var wait = _semaphore.WaitAsync(cancellationToken);

if (wait.IsCompleted)
{
return ValueTask.FromResult(new Releaser(this));
}

return new ValueTask<Releaser>(wait.ContinueWith(
static (_, state) => new Releaser((AsyncLock)state!),
this,
cancellationToken,
TaskContinuationOptions.ExecuteSynchronously,
TaskScheduler.Default
));
}

/// <summary>
/// Represents a disposable object that, when disposed, releases the associated lock.
/// </summary>
public struct Releaser : IDisposable
{
private AsyncLock? _lock;

internal Releaser(AsyncLock @lock)
{
_lock = @lock;
}

/// <summary>
/// Releases the lock held by this <see cref="Releaser"/> instance.
/// </summary>
public void Dispose()
{
if (_lock != null)
{
_lock._semaphore.Release();
_lock = null;
}
}
}
}
9 changes: 4 additions & 5 deletions src/Impostor.Server/Net/State/Game.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Net;
using System.Threading;
using System.Threading.Tasks;
using Impostor.Api.Config;
using Impostor.Api.Events.Managers;
Expand All @@ -14,6 +13,7 @@
using Impostor.Api.Net;
using Impostor.Api.Net.Manager;
using Impostor.Api.Net.Messages.S2C;
using Impostor.Api.Utils;
using Impostor.Server.Events;
using Impostor.Server.Net.Manager;
using Microsoft.Extensions.Logging;
Expand All @@ -33,7 +33,7 @@ internal partial class Game
private readonly ICompatibilityManager _compatibilityManager;
private readonly CompatibilityConfig _compatibilityConfig;
private readonly TimeoutConfig _timeoutConfig;
private readonly SemaphoreSlim _lock;
private readonly AsyncLock _lock = new();

public Game(
ILogger<Game> logger,
Expand Down Expand Up @@ -68,7 +68,6 @@ internal partial class Game
_compatibilityConfig = compatibilityConfig.Value;
_timeoutConfig = timeoutConfig.Value;
Items = new ConcurrentDictionary<object, object>();
_lock = new SemaphoreSlim(1, 1);
}

public IPEndPoint PublicIp { get; }
Expand Down Expand Up @@ -116,9 +115,9 @@ public bool TryGetPlayer(int id, [MaybeNullWhen(false)] out ClientPlayer player)
return _players.TryGetValue(clientId, out var clientPlayer) ? clientPlayer : null;
}

public async Task<LockGuard> LockAsync()
public ValueTask<AsyncLock.Releaser> LockAsync()
{
return await LockGuard.LockAsync(_lock);
return _lock.LockAsync();
}

internal async ValueTask StartedAsync()
Expand Down

0 comments on commit 43acae1

Please sign in to comment.