Skip to content

Commit

Permalink
C#: Strict rules (valkey-io#1136)
Browse files Browse the repository at this point in the history
enable strict linting

Co-authored-by: Guriy Samarin <[email protected]>
  • Loading branch information
2 people authored and alex-arzola-imp committed Apr 12, 2024
1 parent 36a78ea commit fba1fdf
Show file tree
Hide file tree
Showing 14 changed files with 300 additions and 335 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/csharp.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ concurrency:

jobs:
run-tests:
timeout-minutes: 15
timeout-minutes: 25
strategy:
fail-fast: false
matrix:
Expand Down
244 changes: 106 additions & 138 deletions benchmarks/csharp/Program.cs

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions benchmarks/csharp/csharp_benchmark.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@
<TargetFrameworks>net6.0;net8.0</TargetFrameworks>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<LangVersion>preview</LangVersion>
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
</PropertyGroup>

<!-- Workaround for https://github.com/dotnet/roslyn/issues/41640 -->
<PropertyGroup>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<NoWarn>$(NoWarn);CS1591;CS1573;CS1587</NoWarn>
</PropertyGroup>

</Project>
2 changes: 2 additions & 0 deletions csharp/.editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -349,3 +349,5 @@ dotnet_naming_style.s_camelcase.required_prefix = s_
dotnet_naming_style.s_camelcase.required_suffix =
dotnet_naming_style.s_camelcase.word_separator =
dotnet_naming_style.s_camelcase.capitalization = camel_case

dotnet_analyzer_diagnostic.category-Style.severity = error
73 changes: 34 additions & 39 deletions csharp/lib/AsyncClient.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
/**
* Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
*/
// Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0

using System.Buffers;
using System.Runtime.InteropServices;
Expand All @@ -10,59 +8,59 @@ namespace Glide;
public class AsyncClient : IDisposable
{
#region public methods
public AsyncClient(string host, UInt32 port, bool useTLS)
public AsyncClient(string host, uint port, bool useTLS)
{
successCallbackDelegate = SuccessCallback;
var successCallbackPointer = Marshal.GetFunctionPointerForDelegate(successCallbackDelegate);
failureCallbackDelegate = FailureCallback;
var failureCallbackPointer = Marshal.GetFunctionPointerForDelegate(failureCallbackDelegate);
clientPointer = CreateClientFfi(host, port, useTLS, successCallbackPointer, failureCallbackPointer);
if (clientPointer == IntPtr.Zero)
_successCallbackDelegate = SuccessCallback;
nint successCallbackPointer = Marshal.GetFunctionPointerForDelegate(_successCallbackDelegate);
_failureCallbackDelegate = FailureCallback;
nint failureCallbackPointer = Marshal.GetFunctionPointerForDelegate(_failureCallbackDelegate);
_clientPointer = CreateClientFfi(host, port, useTLS, successCallbackPointer, failureCallbackPointer);
if (_clientPointer == IntPtr.Zero)
{
throw new Exception("Failed creating a client");
}
}

private async Task<string?> command(IntPtr[] args, int argsCount, RequestType requestType)
private async Task<string?> Command(IntPtr[] args, int argsCount, RequestType requestType)
{
// We need to pin the array in place, in order to ensure that the GC doesn't move it while the operation is running.
GCHandle pinnedArray = GCHandle.Alloc(args, GCHandleType.Pinned);
IntPtr pointer = pinnedArray.AddrOfPinnedObject();
var message = messageContainer.GetMessageForCall(args, argsCount);
CommandFfi(clientPointer, (ulong)message.Index, (int)requestType, pointer, (uint)argsCount);
var result = await message;
Message<string> message = _messageContainer.GetMessageForCall(args, argsCount);
CommandFfi(_clientPointer, (ulong)message.Index, (int)requestType, pointer, (uint)argsCount);
string? result = await message;
pinnedArray.Free();
return result;
}

public async Task<string?> SetAsync(string key, string value)
{
var args = this.arrayPool.Rent(2);
IntPtr[] args = _arrayPool.Rent(2);
args[0] = Marshal.StringToHGlobalAnsi(key);
args[1] = Marshal.StringToHGlobalAnsi(value);
var result = await command(args, 2, RequestType.SetString);
this.arrayPool.Return(args);
string? result = await Command(args, 2, RequestType.SetString);
_arrayPool.Return(args);
return result;
}

public async Task<string?> GetAsync(string key)
{
var args = this.arrayPool.Rent(1);
IntPtr[] args = _arrayPool.Rent(1);
args[0] = Marshal.StringToHGlobalAnsi(key);
var result = await command(args, 1, RequestType.GetString);
this.arrayPool.Return(args);
string? result = await Command(args, 1, RequestType.GetString);
_arrayPool.Return(args);
return result;
}

public void Dispose()
{
if (clientPointer == IntPtr.Zero)
if (_clientPointer == IntPtr.Zero)
{
return;
}
messageContainer.DisposeWithError(null);
CloseClientFfi(clientPointer);
clientPointer = IntPtr.Zero;
_messageContainer.DisposeWithError(null);
CloseClientFfi(_clientPointer);
_clientPointer = IntPtr.Zero;
}

#endregion public methods
Expand All @@ -71,24 +69,22 @@ public void Dispose()

private void SuccessCallback(ulong index, IntPtr str)
{
var result = str == IntPtr.Zero ? null : Marshal.PtrToStringAnsi(str);
string? result = str == IntPtr.Zero ? null : Marshal.PtrToStringAnsi(str);
// Work needs to be offloaded from the calling thread, because otherwise we might starve the client's thread pool.
Task.Run(() =>
_ = Task.Run(() =>
{
var message = messageContainer.GetMessage((int)index);
Message<string> message = _messageContainer.GetMessage((int)index);
message.SetResult(result);
});
}

private void FailureCallback(ulong index)
{
private void FailureCallback(ulong index) =>
// Work needs to be offloaded from the calling thread, because otherwise we might starve the client's thread pool.
Task.Run(() =>
{
var message = messageContainer.GetMessage((int)index);
Message<string> message = _messageContainer.GetMessage((int)index);
message.SetException(new Exception("Operation failed"));
});
}

~AsyncClient() => Dispose();
#endregion private methods
Expand All @@ -97,17 +93,16 @@ private void FailureCallback(ulong index)

/// Held as a measure to prevent the delegate being garbage collected. These are delegated once
/// and held in order to prevent the cost of marshalling on each function call.
private readonly FailureAction failureCallbackDelegate;
private readonly FailureAction _failureCallbackDelegate;

/// Held as a measure to prevent the delegate being garbage collected. These are delegated once
/// and held in order to prevent the cost of marshalling on each function call.
private readonly StringAction successCallbackDelegate;
private readonly StringAction _successCallbackDelegate;

/// Raw pointer to the underlying native client.
private IntPtr clientPointer;

private readonly MessageContainer<string> messageContainer = new();
private readonly ArrayPool<IntPtr> arrayPool = ArrayPool<IntPtr>.Shared;
private IntPtr _clientPointer;
private readonly MessageContainer<string> _messageContainer = new();
private readonly ArrayPool<IntPtr> _arrayPool = ArrayPool<IntPtr>.Shared;

#endregion private fields

Expand All @@ -116,11 +111,11 @@ private void FailureCallback(ulong index)
private delegate void StringAction(ulong index, IntPtr str);
private delegate void FailureAction(ulong index);
[DllImport("libglide_rs", CallingConvention = CallingConvention.Cdecl, EntryPoint = "command")]
private static extern void CommandFfi(IntPtr client, ulong index, Int32 requestType, IntPtr args, UInt32 argCount);
private static extern void CommandFfi(IntPtr client, ulong index, int requestType, IntPtr args, uint argCount);

private delegate void IntAction(IntPtr arg);
[DllImport("libglide_rs", CallingConvention = CallingConvention.Cdecl, EntryPoint = "create_client")]
private static extern IntPtr CreateClientFfi(String host, UInt32 port, bool useTLS, IntPtr successCallback, IntPtr failureCallback);
private static extern IntPtr CreateClientFfi(string host, uint port, bool useTLS, IntPtr successCallback, IntPtr failureCallback);

[DllImport("libglide_rs", CallingConvention = CallingConvention.Cdecl, EntryPoint = "close_client")]
private static extern void CloseClientFfi(IntPtr client);
Expand Down
25 changes: 14 additions & 11 deletions csharp/lib/Logger.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/**
* Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
*/
// Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0


using System.Runtime.InteropServices;
using System.Text;
Expand Down Expand Up @@ -29,7 +28,7 @@ public class Logger
{
#region private fields

private static Level? loggerLevel = null;
private static Level? s_loggerLevel = null;
#endregion private fields

#region internal methods
Expand All @@ -38,7 +37,7 @@ public class Logger
// If given a fileName argument, will write the logs to files postfixed with fileName. If fileName isn't provided, the logs will be written to the console.
internal static void Init(Level? level, string? filename = null)
{
if (Logger.loggerLevel is null)
if (s_loggerLevel is null)
{
SetLoggerConfig(level, filename);
}
Expand All @@ -51,11 +50,15 @@ internal static void Init(Level? level, string? filename = null)
// when the log is connect to certain task the identifier should be the task id, when the log is not part of specific task the identifier should give a context to the log - for example, "create client".
internal static void Log(Level logLevel, string logIdentifier, string message)
{
if (Logger.loggerLevel is null)
if (s_loggerLevel is null)
{
SetLoggerConfig(logLevel);
}
if (!(logLevel <= Logger.loggerLevel)) return;
if (!(logLevel <= s_loggerLevel))
{
return;
}

log(Convert.ToInt32(logLevel), Encoding.UTF8.GetBytes(logIdentifier), Encoding.UTF8.GetBytes(message));
}
#endregion internal methods
Expand All @@ -69,17 +72,17 @@ internal static void Log(Level logLevel, string logIdentifier, string message)
// the filename argument is optional - if provided the target of the logs will be the file mentioned, else will be the console
public static void SetLoggerConfig(Level? level, string? filename = null)
{
var buffer = filename is null ? null : Encoding.UTF8.GetBytes(filename);
Logger.loggerLevel = InitInternalLogger(Convert.ToInt32(level), buffer);
byte[]? buffer = filename is null ? null : Encoding.UTF8.GetBytes(filename);
s_loggerLevel = InitInternalLogger(Convert.ToInt32(level), buffer);
}
#endregion public methods

#region FFI function declaration
[DllImport("libglide_rs", CallingConvention = CallingConvention.Cdecl, EntryPoint = "log")]
private static extern void log(Int32 logLevel, byte[] logIdentifier, byte[] message);
private static extern void log(int logLevel, byte[] logIdentifier, byte[] message);

[DllImport("libglide_rs", CallingConvention = CallingConvention.Cdecl, EntryPoint = "init")]
private static extern Level InitInternalLogger(Int32 level, byte[]? filename);
private static extern Level InitInternalLogger(int level, byte[]? filename);

#endregion
}
Loading

0 comments on commit fba1fdf

Please sign in to comment.