Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allocation cleanup #508

Merged
merged 9 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## Why this PR?

-motivation for making this change-

## Visible Changes

-changes that are visible to developers using this library-
6 changes: 3 additions & 3 deletions examples/ConsoleApp/ConsoleApp.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.0" />
<PackageReference Include="System.Text.Json" Version="8.0.4" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.1" />
<PackageReference Include="System.Text.Json" Version="8.0.5" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.1" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion examples/ConsoleApp/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@
}
]
}
}
}
6 changes: 3 additions & 3 deletions examples/TargetingConsoleApp/TargetingConsoleApp.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.0" />
<PackageReference Include="System.Text.Json" Version="8.0.4" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.1" />
<PackageReference Include="System.Text.Json" Version="8.0.5" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.1" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public FeatureGateAttribute(RequirementType requirementType, params object[] fea
public RequirementType RequirementType { get; }

/// <summary>
/// Performs controller action pre-procesing to ensure that any or all of the specified features are enabled.
/// Performs controller action pre-processing to ensure that any or all of the specified features are enabled.
/// </summary>
/// <param name="context">The context of the MVC action.</param>
/// <param name="next">The action delegate.</param>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP
// provider to be marked for caching as well.

private readonly IConfiguration _configuration;
private readonly ConcurrentDictionary<string, FeatureDefinition> _definitions;
private readonly ConcurrentDictionary<string, Task<FeatureDefinition>> _definitions;
private IDisposable _changeSubscription;
private int _stale = 0;
public Func<string, Task<FeatureDefinition>> schemaReadFunc;
rossgrambo marked this conversation as resolved.
Show resolved Hide resolved
avanigupta marked this conversation as resolved.
Show resolved Hide resolved

const string ParseValueErrorString = "Invalid setting '{0}' with value '{1}' for feature '{2}'.";

Expand All @@ -37,11 +38,16 @@ public sealed class ConfigurationFeatureDefinitionProvider : IFeatureDefinitionP
public ConfigurationFeatureDefinitionProvider(IConfiguration configuration)
{
_configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));
_definitions = new ConcurrentDictionary<string, FeatureDefinition>();
_definitions = new ConcurrentDictionary<string, Task<FeatureDefinition>>();

_changeSubscription = ChangeToken.OnChange(
() => _configuration.GetReloadToken(),
() => _stale = 1);

schemaReadFunc = (featureName) =>
{
return Task.FromResult(GetMicrosoftSchemaFeatureDefinition(featureName, _configuration) ?? GetDotnetSchemaFeatureDefinition(featureName, _configuration, RootConfigurationFallbackEnabled));
};
}

/// <summary>
Expand Down Expand Up @@ -69,6 +75,7 @@ public void Dispose()
/// </summary>
/// <param name="featureName">The name of the feature to retrieve the definition for.</param>
/// <returns>The feature's definition.</returns>

avanigupta marked this conversation as resolved.
Show resolved Hide resolved
public Task<FeatureDefinition> GetFeatureDefinitionAsync(string featureName)
{
if (featureName == null)
Expand All @@ -86,10 +93,7 @@ public Task<FeatureDefinition> GetFeatureDefinitionAsync(string featureName)
_definitions.Clear();
}

return Task.FromResult(
_definitions.GetOrAdd(
featureName,
(_) => GetMicrosoftSchemaFeatureDefinition(featureName) ?? GetDotnetSchemaFeatureDefinition(featureName)));
return _definitions.GetOrAdd(featureName, schemaReadFunc);
}

/// <summary>
Expand All @@ -98,7 +102,7 @@ public Task<FeatureDefinition> GetFeatureDefinitionAsync(string featureName)
/// <returns>An enumerator which provides asynchronous iteration over feature definitions.</returns>
//
// The async key word is necessary for creating IAsyncEnumerable.
// The need to disable this warning occurs when implementaing async stream synchronously.
// The need to disable this warning occurs when implementing async stream synchronously.
#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
public async IAsyncEnumerable<FeatureDefinition> GetAllFeatureDefinitionsAsync()
#pragma warning restore CS1998
Expand All @@ -108,7 +112,7 @@ public async IAsyncEnumerable<FeatureDefinition> GetAllFeatureDefinitionsAsync()
_definitions.Clear();
}

IEnumerable<IConfigurationSection> microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections();
IEnumerable<IConfigurationSection> microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(_configuration);

foreach (IConfigurationSection featureSection in microsoftFeatureDefinitionSections)
{
Expand All @@ -121,15 +125,15 @@ public async IAsyncEnumerable<FeatureDefinition> GetAllFeatureDefinitionsAsync()

//
// Underlying IConfigurationSection data is dynamic so latest feature definitions are returned
FeatureDefinition definition = _definitions.GetOrAdd(featureName, (_) => ParseMicrosoftSchemaFeatureDefinition(featureSection));
FeatureDefinition definition = _definitions.GetOrAdd(featureName, schemaReadFunc).Result;

if (definition != null)
{
yield return definition;
}
}

IEnumerable<IConfigurationSection> dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections();
IEnumerable<IConfigurationSection> dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(_configuration, RootConfigurationFallbackEnabled);

foreach (IConfigurationSection featureSection in dotnetFeatureDefinitionSections)
{
Expand All @@ -142,7 +146,7 @@ public async IAsyncEnumerable<FeatureDefinition> GetAllFeatureDefinitionsAsync()

//
// Underlying IConfigurationSection data is dynamic so latest feature definitions are returned
FeatureDefinition definition = _definitions.GetOrAdd(featureName, (_) => ParseDotnetSchemaFeatureDefinition(featureSection));
FeatureDefinition definition = _definitions.GetOrAdd(featureName, schemaReadFunc).Result;

if (definition != null)
{
Expand All @@ -151,41 +155,41 @@ public async IAsyncEnumerable<FeatureDefinition> GetAllFeatureDefinitionsAsync()
}
}

private FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName)
private static FeatureDefinition GetDotnetSchemaFeatureDefinition(string featureName, IConfiguration configuration, bool rootConfigurationFallbackEnabled)
rossgrambo marked this conversation as resolved.
Show resolved Hide resolved
{
IEnumerable<IConfigurationSection> dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections();
IEnumerable<IConfigurationSection> dotnetFeatureDefinitionSections = GetDotnetFeatureDefinitionSections(configuration, rootConfigurationFallbackEnabled);

IConfigurationSection configuration = dotnetFeatureDefinitionSections
IConfigurationSection dotnetFeatureDefinitionConfiguration = dotnetFeatureDefinitionSections
.FirstOrDefault(section =>
string.Equals(section.Key, featureName, StringComparison.OrdinalIgnoreCase));

if (configuration == null)
if (dotnetFeatureDefinitionConfiguration == null)
{
return null;
}

return ParseDotnetSchemaFeatureDefinition(configuration);
return ParseDotnetSchemaFeatureDefinition(dotnetFeatureDefinitionConfiguration);
}

private FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string featureName)
private static FeatureDefinition GetMicrosoftSchemaFeatureDefinition(string featureName, IConfiguration configuration)
{
IEnumerable<IConfigurationSection> microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections();
IEnumerable<IConfigurationSection> microsoftFeatureDefinitionSections = GetMicrosoftFeatureDefinitionSections(configuration);

IConfigurationSection configuration = microsoftFeatureDefinitionSections
IConfigurationSection microsoftFeatureDefinitionConfiguration = microsoftFeatureDefinitionSections
.LastOrDefault(section =>
string.Equals(section[MicrosoftFeatureManagementFields.Id], featureName, StringComparison.OrdinalIgnoreCase));

if (configuration == null)
if (microsoftFeatureDefinitionConfiguration == null)
{
return null;
}

return ParseMicrosoftSchemaFeatureDefinition(configuration);
return ParseMicrosoftSchemaFeatureDefinition(microsoftFeatureDefinitionConfiguration);
}

private IEnumerable<IConfigurationSection> GetDotnetFeatureDefinitionSections()
private static IEnumerable<IConfigurationSection> GetDotnetFeatureDefinitionSections(IConfiguration configuration, bool rootConfigurationFallbackEnabled)
{
IConfigurationSection featureManagementConfigurationSection = _configuration.GetSection(DotnetFeatureManagementFields.FeatureManagementSectionName);
IConfigurationSection featureManagementConfigurationSection = configuration.GetSection(DotnetFeatureManagementFields.FeatureManagementSectionName);

if (featureManagementConfigurationSection.Exists())
{
Expand All @@ -195,25 +199,25 @@ private IEnumerable<IConfigurationSection> GetDotnetFeatureDefinitionSections()
//
// Root configuration fallback only applies to .NET schema.
// If Microsoft schema can be found, root configuration fallback will not be effective.
if (RootConfigurationFallbackEnabled &&
!_configuration.GetChildren()
if (rootConfigurationFallbackEnabled &&
!configuration.GetChildren()
.Any(section =>
string.Equals(section.Key, MicrosoftFeatureManagementFields.FeatureManagementSectionName, StringComparison.OrdinalIgnoreCase)))
{
return _configuration.GetChildren();
return configuration.GetChildren();
}

return Enumerable.Empty<IConfigurationSection>();
}

private IEnumerable<IConfigurationSection> GetMicrosoftFeatureDefinitionSections()
private static IEnumerable<IConfigurationSection> GetMicrosoftFeatureDefinitionSections(IConfiguration configuration)
{
return _configuration.GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName)
return configuration.GetSection(MicrosoftFeatureManagementFields.FeatureManagementSectionName)
.GetSection(MicrosoftFeatureManagementFields.FeatureFlagsSectionName)
.GetChildren();
}

private FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection)
private static FeatureDefinition ParseDotnetSchemaFeatureDefinition(IConfigurationSection configurationSection)
{
/*

Expand Down Expand Up @@ -304,7 +308,7 @@ We support
};
}

private FeatureDefinition ParseMicrosoftSchemaFeatureDefinition(IConfigurationSection configurationSection)
private static FeatureDefinition ParseMicrosoftSchemaFeatureDefinition(IConfigurationSection configurationSection)
{
/*

Expand Down Expand Up @@ -518,7 +522,7 @@ private FeatureDefinition ParseMicrosoftSchemaFeatureDefinition(IConfigurationSe
};
}

private T ParseEnum<T>(string feature, string rawValue, string fieldKeyword)
private static T ParseEnum<T>(string feature, string rawValue, string fieldKeyword)
where T : struct, Enum
{
Debug.Assert(!string.IsNullOrEmpty(rawValue));
Expand All @@ -533,7 +537,7 @@ private T ParseEnum<T>(string feature, string rawValue, string fieldKeyword)
return value;
}

private double ParseDouble(string feature, string rawValue, string fieldKeyword)
private static double ParseDouble(string feature, string rawValue, string fieldKeyword)
{
Debug.Assert(!string.IsNullOrEmpty(rawValue));

Expand All @@ -547,7 +551,7 @@ private double ParseDouble(string feature, string rawValue, string fieldKeyword)
return value;
}

private bool ParseBool(string feature, string rawValue, string fieldKeyword)
private static bool ParseBool(string feature, string rawValue, string fieldKeyword)
{
Debug.Assert(!string.IsNullOrEmpty(rawValue));

Expand Down
Loading
Loading