Skip to content

Commit

Permalink
Reduce heap allocation by deferring formatting of log and error messa…
Browse files Browse the repository at this point in the history
…ges until actually accessed by consumer
  • Loading branch information
adams85 committed Jul 30, 2024
1 parent 0a3195a commit 75ea32b
Show file tree
Hide file tree
Showing 14 changed files with 102 additions and 36 deletions.
2 changes: 1 addition & 1 deletion src/ConfigCat.Client.Tests/ConfigCatClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2114,7 +2114,7 @@ public async Task Hooks_MockedClientRaisesEvents()
var onFetch = (ProjectConfig latestConfig, CancellationToken _) =>
{
var logMessage = loggerWrapper.FetchFailedDueToUnexpectedError(errorException);
return FetchResult.Failure(latestConfig, RefreshErrorCode.HttpRequestFailure, errorMessage: logMessage.InvariantFormattedMessage, errorException: errorException);
return FetchResult.Failure(latestConfig, RefreshErrorCode.HttpRequestFailure, errorMessage: logMessage.ToLazyString(), errorException: errorException);
};
this.fetcherMock.Setup(m => m.FetchAsync(It.IsAny<ProjectConfig>(), It.IsAny<CancellationToken>())).ReturnsAsync(onFetch);

Expand Down
4 changes: 2 additions & 2 deletions src/ConfigCatClient/ConfigService/ConfigServiceBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public virtual RefreshResult RefreshConfig()
else
{
var logMessage = this.Logger.ConfigServiceCannotInitiateHttpCalls();
return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.InvariantFormattedMessage);
return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.ToLazyString());
}
}

Expand Down Expand Up @@ -133,7 +133,7 @@ public virtual async ValueTask<RefreshResult> RefreshConfigAsync(CancellationTok
else
{
var logMessage = this.Logger.ConfigServiceCannotInitiateHttpCalls();
return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.InvariantFormattedMessage);
return RefreshResult.Failure(RefreshErrorCode.OfflineClient, logMessage.ToLazyString());
}
}

Expand Down
13 changes: 10 additions & 3 deletions src/ConfigCatClient/Evaluation/EvaluationDetails.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using ConfigCat.Client.Evaluation;
using ConfigCat.Client.Utils;

namespace ConfigCat.Client;

Expand Down Expand Up @@ -28,13 +29,13 @@ internal static EvaluationDetails<TValue> FromEvaluateResult<TValue>(string key,
}

internal static EvaluationDetails<TValue> FromDefaultValue<TValue>(string key, TValue defaultValue, DateTime? fetchTime, User? user,
string errorMessage, Exception? errorException = null, EvaluationErrorCode errorCode = EvaluationErrorCode.UnexpectedError)
LazyString errorMessage, Exception? errorException = null, EvaluationErrorCode errorCode = EvaluationErrorCode.UnexpectedError)
{
var instance = new EvaluationDetails<TValue>(key, defaultValue)
{
User = user,
IsDefaultValue = true,
ErrorMessage = errorMessage,
errorMessage = errorMessage,
ErrorException = errorException,
ErrorCode = errorCode,
};
Expand All @@ -47,6 +48,8 @@ internal static EvaluationDetails<TValue> FromDefaultValue<TValue>(string key, T
return instance;
}

private LazyString errorMessage;

private protected EvaluationDetails(string key)
{
Key = key;
Expand Down Expand Up @@ -93,7 +96,11 @@ private protected EvaluationDetails(string key)
/// <summary>
/// Error message in case evaluation failed.
/// </summary>
public string? ErrorMessage { get; set; }
public string? ErrorMessage
{
get => this.errorMessage.ToString();
set => this.errorMessage = value;
}

/// <summary>
/// The <see cref="Exception"/> object related to the error in case evaluation failed (if any).
Expand Down
6 changes: 3 additions & 3 deletions src/ConfigCatClient/Evaluation/EvaluationHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ public static EvaluationDetails<T> Evaluate<T>(this IRolloutEvaluator evaluator,
{
logMessage = logger.ConfigJsonIsNotPresent(key, nameof(defaultValue), defaultValue);
return EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: remoteConfig?.TimeStamp, user,
logMessage.InvariantFormattedMessage, errorCode: EvaluationErrorCode.ConfigJsonNotAvailable);
logMessage.ToLazyString(), errorCode: EvaluationErrorCode.ConfigJsonNotAvailable);
}

if (!settings.TryGetValue(key, out var setting))
{
var availableKeys = new StringListFormatter(settings.Keys).ToString();
var availableKeys = new StringListFormatter(settings.Keys);
logMessage = logger.SettingEvaluationFailedDueToMissingKey(key, nameof(defaultValue), defaultValue, availableKeys);
return EvaluationDetails.FromDefaultValue(key, defaultValue, fetchTime: remoteConfig?.TimeStamp, user,
logMessage.InvariantFormattedMessage, errorCode: EvaluationErrorCode.SettingKeyMissing);
logMessage.ToLazyString(), errorCode: EvaluationErrorCode.SettingKeyMissing);
}

var evaluateContext = new EvaluateContext(key, setting, user, settings);
Expand Down
11 changes: 6 additions & 5 deletions src/ConfigCatClient/FetchResult.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Diagnostics.CodeAnalysis;
using ConfigCat.Client.Utils;

namespace ConfigCat.Client;

Expand All @@ -17,12 +18,12 @@ public static FetchResult NotModified(ProjectConfig config)
return new FetchResult(config, RefreshErrorCode.None, NotModifiedToken);
}

public static FetchResult Failure(ProjectConfig config, RefreshErrorCode errorCode, string errorMessage, Exception? errorException = null)
public static FetchResult Failure(ProjectConfig config, RefreshErrorCode errorCode, LazyString errorMessage, Exception? errorException = null)
{
return new FetchResult(config, errorCode, errorMessage, errorException);
return new FetchResult(config, errorCode, errorMessage.Value ?? (object)errorMessage, errorException);
}

private readonly object? errorMessageOrToken;
private readonly object? errorMessageOrToken; // either null or a string or a boxed LazyString or NotModifiedToken

private FetchResult(ProjectConfig config, RefreshErrorCode errorCode, object? errorMessageOrToken, Exception? errorException = null)
{
Expand All @@ -35,10 +36,10 @@ private FetchResult(ProjectConfig config, RefreshErrorCode errorCode, object? er
public bool IsSuccess => this.errorMessageOrToken is null;
public bool IsNotModified => ReferenceEquals(this.errorMessageOrToken, NotModifiedToken);
[MemberNotNullWhen(true, nameof(ErrorMessage))]
public bool IsFailure => this.errorMessageOrToken is string;
public bool IsFailure => !IsSuccess && !IsNotModified;

public ProjectConfig Config { get; }
public RefreshErrorCode ErrorCode { get; }
public string? ErrorMessage => this.errorMessageOrToken as string;
public object? ErrorMessage => !IsNotModified ? this.errorMessageOrToken : null;
public Exception? ErrorException { get; }
}
8 changes: 4 additions & 4 deletions src/ConfigCatClient/FormattableStringFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.

// Based on: https://github.com/dotnet/runtime/blob/v6.0.13/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/FormattableStringFactory.cs
// This is a modified version of the built-in type that is only used only internally in the SDK. It allows
// creating value type instances instead of reference type instances to avoid unnecessary heap allocations.
// This is a modified version of the built-in type that is only used only internally in the SDK.
// It creates value type instances instead of reference type instances to avoid unnecessary heap allocations.

#pragma warning disable IDE0161 // Convert to file-scoped namespace
#pragma warning disable CS0436 // Type conflicts with imported type
Expand All @@ -19,7 +19,7 @@ internal static class FormattableStringFactory
/// Create a <see cref="FormattableString"/> from a composite format string and object
/// array containing zero or more objects to format.
/// </summary>
public static FormattableString Create(string format, params object?[] arguments)
public static ValueFormattableString Create(string format, params object?[] arguments)
{
if (format == null)
{
Expand All @@ -31,7 +31,7 @@ public static FormattableString Create(string format, params object?[] arguments
throw new ArgumentNullException(nameof(arguments));
}

return new FormattableString(format, arguments);
return new ValueFormattableString(format, arguments);
}
}
}
10 changes: 5 additions & 5 deletions src/ConfigCatClient/Hooks/Hooks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public void RaiseConfigFetched(RefreshResult result, bool isInitiatedByUser)
public void RaiseConfigChanged(IConfig newConfig)
=> this.events.RaiseConfigChanged(this.client, newConfig);

public void RaiseError(string message, Exception? exception)
=> this.events.RaiseError(this.client, message, exception);
public void RaiseError(ref FormattableLogMessage message, Exception? exception)
=> this.events.RaiseError(this.client, ref message, exception);

public event EventHandler<ClientReadyEventArgs>? ClientReady
{
Expand Down Expand Up @@ -84,7 +84,7 @@ public virtual void RaiseClientReady(IConfigCatClient? client, ClientCacheState
public virtual void RaiseFlagEvaluated(IConfigCatClient? client, EvaluationDetails evaluationDetails) { /* intentional no-op */ }
public virtual void RaiseConfigFetched(IConfigCatClient? client, RefreshResult result, bool isInitiatedByUser) { /* intentional no-op */ }
public virtual void RaiseConfigChanged(IConfigCatClient? client, IConfig newConfig) { /* intentional no-op */ }
public virtual void RaiseError(IConfigCatClient? client, string message, Exception? exception) { /* intentional no-op */ }
public virtual void RaiseError(IConfigCatClient? client, ref FormattableLogMessage message, Exception? exception) { /* intentional no-op */ }

public virtual event EventHandler<ClientReadyEventArgs>? ClientReady { add { /* intentional no-op */ } remove { /* intentional no-op */ } }
public virtual event EventHandler<FlagEvaluatedEventArgs>? FlagEvaluated { add { /* intentional no-op */ } remove { /* intentional no-op */ } }
Expand Down Expand Up @@ -115,9 +115,9 @@ public override void RaiseConfigChanged(IConfigCatClient? client, IConfig newCon
ConfigChanged?.Invoke(client, new ConfigChangedEventArgs(newConfig));
}

public override void RaiseError(IConfigCatClient? client, string message, Exception? exception)
public override void RaiseError(IConfigCatClient? client, ref FormattableLogMessage message, Exception? exception)
{
Error?.Invoke(client, new ConfigCatClientErrorEventArgs(message, exception));
Error?.Invoke(client, new ConfigCatClientErrorEventArgs(message.InvariantFormattedMessage, exception));
}

public override event EventHandler<ClientReadyEventArgs>? ClientReady;
Expand Down
4 changes: 2 additions & 2 deletions src/ConfigCatClient/Hooks/SafeHooksWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ public void RaiseConfigChanged(IConfig newConfig)
=> Hooks.RaiseConfigChanged(newConfig);

[MethodImpl(MethodImplOptions.NoInlining)]
public void RaiseError(string message, Exception? exception)
=> Hooks.RaiseError(message, exception);
public void RaiseError(ref FormattableLogMessage message, Exception? exception)
=> Hooks.RaiseError(ref message, exception);

public static implicit operator SafeHooksWrapper(Hooks? hooks) => hooks is not null ? new SafeHooksWrapper(hooks) : default;
}
10 changes: 5 additions & 5 deletions src/ConfigCatClient/HttpConfigFetcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private async ValueTask<FetchResult> FetchInternalAsync(ProjectConfig lastConfig
{
var exception = responseWithBody.Exception;
logMessage = this.logger.FetchReceived200WithInvalidBody(exception);
return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseContent, logMessage.InvariantFormattedMessage, exception);
return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseContent, logMessage.ToLazyString(), exception);
}

return FetchResult.Success(new ProjectConfig
Expand All @@ -105,7 +105,7 @@ private async ValueTask<FetchResult> FetchInternalAsync(ProjectConfig lastConfig
if (lastConfig.IsEmpty)
{
logMessage = this.logger.FetchReceived304WhenLocalCacheIsEmpty((int)response.StatusCode, response.ReasonPhrase);
return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseWhenLocalCacheIsEmpty, logMessage.InvariantFormattedMessage);
return FetchResult.Failure(lastConfig, RefreshErrorCode.InvalidHttpResponseWhenLocalCacheIsEmpty, logMessage.ToLazyString());
}

return FetchResult.NotModified(lastConfig.With(ProjectConfig.GenerateTimeStamp()));
Expand All @@ -115,13 +115,13 @@ private async ValueTask<FetchResult> FetchInternalAsync(ProjectConfig lastConfig
logMessage = this.logger.FetchFailedDueToInvalidSdkKey();

// We update the timestamp for extra protection against flooding.
return FetchResult.Failure(lastConfig.With(ProjectConfig.GenerateTimeStamp()), RefreshErrorCode.InvalidSdkKey, logMessage.InvariantFormattedMessage);
return FetchResult.Failure(lastConfig.With(ProjectConfig.GenerateTimeStamp()), RefreshErrorCode.InvalidSdkKey, logMessage.ToLazyString());

default:
logMessage = this.logger.FetchFailedDueToUnexpectedHttpResponse((int)response.StatusCode, response.ReasonPhrase);

ReInitializeHttpClient();
return FetchResult.Failure(lastConfig, RefreshErrorCode.UnexpectedHttpResponse, logMessage.InvariantFormattedMessage);
return FetchResult.Failure(lastConfig, RefreshErrorCode.UnexpectedHttpResponse, logMessage.ToLazyString());
}
}
catch (OperationCanceledException) when (this.cancellationTokenSource.IsCancellationRequested)
Expand Down Expand Up @@ -153,7 +153,7 @@ private async ValueTask<FetchResult> FetchInternalAsync(ProjectConfig lastConfig
}

ReInitializeHttpClient();
return FetchResult.Failure(lastConfig, errorCode, logMessage.InvariantFormattedMessage, errorException);
return FetchResult.Failure(lastConfig, errorCode, logMessage.ToLazyString(), errorException);
}

private async ValueTask<ResponseWithBody> FetchRequestAsync(string? httpETag, Uri requestUri, sbyte maxExecutionCount = 3)
Expand Down
7 changes: 7 additions & 0 deletions src/ConfigCatClient/Logging/FormattableLogMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ public FormattableLogMessage(string format, string[] argNames, object?[] argValu
/// </summary>
public string InvariantFormattedMessage => this.invariantFormattedMessage ??= ToString(CultureInfo.InvariantCulture);

internal LazyString ToLazyString()
{
return this.invariantFormattedMessage is { } invariantFormattedMessage
? invariantFormattedMessage
: new LazyString(this.format ?? string.Empty, this.argValues);
}

/// <summary>
/// Returns the log message formatted using <see cref="CultureInfo.CurrentCulture"/>.
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion src/ConfigCatClient/Logging/LogMessages.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using ConfigCat.Client.ConfigService;
using ConfigCat.Client.Utils;

namespace ConfigCat.Client;

Expand All @@ -17,7 +18,7 @@ public static FormattableLogMessage ConfigJsonIsNotPresent(this LoggerWrapper lo
$"Config JSON is not present when evaluating setting '{key}'. Returning the `{defaultParamName}` parameter that you specified in your application: '{defaultParamValue}'.",
"KEY", "DEFAULT_PARAM_NAME", "DEFAULT_PARAM_VALUE");

public static FormattableLogMessage SettingEvaluationFailedDueToMissingKey(this LoggerWrapper logger, string key, string defaultParamName, object? defaultParamValue, string availableKeys) => logger.LogInterpolated(
public static FormattableLogMessage SettingEvaluationFailedDueToMissingKey(this LoggerWrapper logger, string key, string defaultParamName, object? defaultParamValue, StringListFormatter availableKeys) => logger.LogInterpolated(
LogLevel.Error, 1001,
$"Failed to evaluate setting '{key}' (the key was not found in config JSON). Returning the `{defaultParamName}` parameter that you specified in your application: '{defaultParamValue}'. Available keys: [{availableKeys}].",
"KEY", "DEFAULT_PARAM_NAME", "DEFAULT_PARAM_VALUE", "AVAILABLE_KEYS");
Expand Down
2 changes: 1 addition & 1 deletion src/ConfigCatClient/Logging/LoggerWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void Log(LogLevel level, LogEventId eventId, ref FormattableLogMessage me

if (level == LogLevel.Error)
{
this.hooks.RaiseError(message.InvariantFormattedMessage, exception);
this.hooks.RaiseError(ref message, exception);
}
}
}
16 changes: 12 additions & 4 deletions src/ConfigCatClient/RefreshResult.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Diagnostics.CodeAnalysis;
using ConfigCat.Client.Utils;

namespace ConfigCat.Client;

Expand Down Expand Up @@ -29,17 +30,24 @@ public static RefreshResult Failure(RefreshErrorCode errorCode, string errorMess
errorException);
}

internal static RefreshResult Failure(RefreshErrorCode errorCode, LazyString errorMessage, Exception? errorException = null)
{
return new RefreshResult(errorCode, errorMessage, errorException);
}

internal static RefreshResult From(in FetchResult fetchResult)
{
return !fetchResult.IsFailure
? Success()
: Failure(fetchResult.ErrorCode, fetchResult.ErrorMessage, fetchResult.ErrorException);
: new RefreshResult(fetchResult.ErrorCode, fetchResult.ErrorMessage, fetchResult.ErrorException);
}

private RefreshResult(RefreshErrorCode errorCode, string? errorMessage, Exception? errorException)
private readonly object? errorMessage; // either null or a string or a boxed LazyString

private RefreshResult(RefreshErrorCode errorCode, object? errorMessage, Exception? errorException)
{
ErrorCode = errorCode;
ErrorMessage = errorMessage;
this.errorMessage = errorMessage;
ErrorException = errorException;
}

Expand All @@ -57,7 +65,7 @@ private RefreshResult(RefreshErrorCode errorCode, string? errorMessage, Exceptio
/// <summary>
/// Error message in case the operation failed, otherwise <see langword="null" />.
/// </summary>
public string? ErrorMessage { get; }
public string? ErrorMessage => this.errorMessage?.ToString();

/// <summary>
/// The <see cref="Exception"/> object related to the error in case the operation failed (if any).
Expand Down
42 changes: 42 additions & 0 deletions src/ConfigCatClient/Utils/LazyString.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System.Globalization;

namespace ConfigCat.Client.Utils;

internal struct LazyString
{
private readonly string? format;
private object? argsOrValue;

public LazyString(string? value)
{
this.format = null;
this.argsOrValue = value;
}

public LazyString(string format, params object?[]? args)
{
this.format = format;
this.argsOrValue = args ?? ArrayUtils.EmptyArray<string>();
}

public string? Value => this.argsOrValue as string;

public override string? ToString()
{
var argsOrValue = this.argsOrValue;
if (argsOrValue is null)
{
return null;
}

if (argsOrValue is not string value)
{
var args = (object?[])argsOrValue;
this.argsOrValue = value = string.Format(CultureInfo.InvariantCulture, this.format!, args);
}

return value;
}

public static implicit operator LazyString(string? value) => new LazyString(value);
}

0 comments on commit 75ea32b

Please sign in to comment.