From 02334f27cb4cad7a336b9aa28121fc17674a2887 Mon Sep 17 00:00:00 2001 From: Adam Simon Date: Mon, 5 Feb 2024 20:03:10 +0100 Subject: [PATCH] Simplify EvaluateContext + improve perf. of user attribute retrieval during evaluation + reduce allocations --- .../Evaluation/EvaluateContext.cs | 18 +++-------- .../Evaluation/RolloutEvaluator.cs | 31 +++++++++++++------ src/ConfigCatClient/User.cs | 12 +++++++ 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/ConfigCatClient/Evaluation/EvaluateContext.cs b/src/ConfigCatClient/Evaluation/EvaluateContext.cs index 8815489c..eb88836f 100644 --- a/src/ConfigCatClient/Evaluation/EvaluateContext.cs +++ b/src/ConfigCatClient/Evaluation/EvaluateContext.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using ConfigCat.Client.Utils; namespace ConfigCat.Client.Evaluation; @@ -8,16 +7,9 @@ internal struct EvaluateContext { public readonly string Key; public readonly Setting Setting; + public readonly User? User; public readonly IReadOnlyDictionary Settings; - private readonly User? user; - - [MemberNotNullWhen(true, nameof(UserAttributes))] - public readonly bool IsUserAvailable => this.user is not null; - - private IReadOnlyDictionary? userAttributes; - public IReadOnlyDictionary? UserAttributes => this.userAttributes ??= this.user?.GetAllAttributes(); - private List? visitedFlags; public List VisitedFlags => this.visitedFlags ??= new List(); @@ -30,19 +22,17 @@ public EvaluateContext(string key, Setting setting, User? user, IReadOnlyDiction { this.Key = key; this.Setting = setting; - this.user = user; + this.User = user; this.Settings = settings; - this.userAttributes = null; this.visitedFlags = null; this.IsMissingUserObjectLogged = this.IsMissingUserObjectAttributeLogged = false; this.LogBuilder = null; // initialized by RolloutEvaluator.Evaluate } - public EvaluateContext(string key, Setting setting, ref EvaluateContext dependentFlagContext) - : this(key, setting, dependentFlagContext.user, dependentFlagContext.Settings) + public EvaluateContext(string key, Setting setting, in EvaluateContext dependentFlagContext) + : this(key, setting, dependentFlagContext.User, dependentFlagContext.Settings) { - this.userAttributes = dependentFlagContext.UserAttributes; this.visitedFlags = dependentFlagContext.VisitedFlags; // crucial to use the property here to make sure the list is created! this.LogBuilder = dependentFlagContext.LogBuilder; } diff --git a/src/ConfigCatClient/Evaluation/RolloutEvaluator.cs b/src/ConfigCatClient/Evaluation/RolloutEvaluator.cs index 0e9a3a94..09ca9bba 100644 --- a/src/ConfigCatClient/Evaluation/RolloutEvaluator.cs +++ b/src/ConfigCatClient/Evaluation/RolloutEvaluator.cs @@ -2,7 +2,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; -using System.Linq; using System.Text; using ConfigCat.Client.Utils; using ConfigCat.Client.Versioning; @@ -35,9 +34,9 @@ public EvaluateResult Evaluate(T defaultValue, ref EvaluateContext context, [ logBuilder.Append($"Evaluating '{context.Key}'"); - if (context.IsUserAvailable) + if (context.User is not null) { - logBuilder.Append($" for User '{context.UserAttributes.Serialize()}'"); + logBuilder.Append($" for User '{context.User.GetAllAttributes().Serialize()}'"); } logBuilder.IncreaseIndent(); @@ -178,7 +177,7 @@ private bool TryEvaluatePercentageOptions(PercentageOption[] percentageOptions, { var logBuilder = context.LogBuilder; - if (!context.IsUserAvailable) + if (context.User is null) { logBuilder?.NewLine("Skipping % options because the User Object is missing."); @@ -192,9 +191,20 @@ private bool TryEvaluatePercentageOptions(PercentageOption[] percentageOptions, return false; } - var percentageOptionsAttributeName = context.Setting.PercentageOptionsAttribute ?? nameof(User.Identifier); + var percentageOptionsAttributeName = context.Setting.PercentageOptionsAttribute; + object? percentageOptionsAttributeValue; - if (!context.UserAttributes.TryGetValue(percentageOptionsAttributeName, out var percentageOptionsAttributeValue)) + if (percentageOptionsAttributeName is null) + { + percentageOptionsAttributeName = nameof(User.Identifier); + percentageOptionsAttributeValue = context.User.Identifier; + } + else + { + percentageOptionsAttributeValue = context.User.GetAttribute(percentageOptionsAttributeName); + } + + if (percentageOptionsAttributeValue is null) { logBuilder?.NewLine().Append($"Skipping % options because the User.{percentageOptionsAttributeName} attribute is missing."); @@ -334,7 +344,7 @@ private bool EvaluateUserCondition(UserCondition condition, string contextSalt, var logBuilder = context.LogBuilder; logBuilder?.AppendUserCondition(condition); - if (!context.IsUserAvailable) + if (context.User is null) { if (!context.IsMissingUserObjectLogged) { @@ -347,8 +357,9 @@ private bool EvaluateUserCondition(UserCondition condition, string contextSalt, } var userAttributeName = condition.ComparisonAttribute ?? throw new InvalidOperationException("Comparison attribute name is missing."); + var userAttributeValue = context.User.GetAttribute(userAttributeName); - if (!context.UserAttributes.TryGetValue(userAttributeName, out var userAttributeValue) || userAttributeValue is string { Length: 0 }) + if (userAttributeValue is null || userAttributeValue is string { Length: 0 }) { this.logger.UserObjectAttributeIsMissing(condition.ToString(), context.Key, userAttributeName); error = string.Format(CultureInfo.InvariantCulture, MissingUserAttributeError, userAttributeName); @@ -723,7 +734,7 @@ private bool EvaluatePrerequisiteFlagCondition(PrerequisiteFlagCondition conditi throw new InvalidOperationException($"Circular dependency detected between the following depending flags: {dependencyCycle}."); } - var prerequisiteFlagContext = new EvaluateContext(prerequisiteFlagKey!, prerequisiteFlag!, ref context); + var prerequisiteFlagContext = new EvaluateContext(prerequisiteFlagKey!, prerequisiteFlag!, context); logBuilder? .NewLine("(") @@ -762,7 +773,7 @@ private bool EvaluateSegmentCondition(SegmentCondition condition, ref EvaluateCo var logBuilder = context.LogBuilder; logBuilder?.AppendSegmentCondition(condition); - if (!context.IsUserAvailable) + if (context.User is null) { if (!context.IsMissingUserObjectLogged) { diff --git a/src/ConfigCatClient/User.cs b/src/ConfigCatClient/User.cs index 1db63f8e..3b6b0afc 100644 --- a/src/ConfigCatClient/User.cs +++ b/src/ConfigCatClient/User.cs @@ -78,6 +78,18 @@ public IDictionary Custom get => this.custom ??= new Dictionary(); set => this.custom = value; } + + internal object? GetAttribute(string name) + { + return name switch + { + nameof(Identifier) => Identifier, + nameof(Email) => Email, + nameof(Country) => Country, + _ => this.custom is not null && this.custom.TryGetValue(name, out var value) ? value : null + }; + } + /// /// Returns all attributes of the user. ///