Skip to content

Commit

Permalink
Code quality updates (#2747)
Browse files Browse the repository at this point in the history
  • Loading branch information
BernieWhite authored Mar 10, 2024
1 parent 3aa9963 commit c03e3db
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 23 deletions.
18 changes: 18 additions & 0 deletions docs/benchmark/results-v1.34.2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
```
BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3155/23H2/2023Update/SunValley3)
Intel Core i7-1065G7 CPU 1.30GHz, 1 CPU, 8 logical and 4 physical cores
.NET SDK 8.0.200
[Host] : .NET 7.0.16 (7.0.1624.6629), X64 RyuJIT AVX2
DefaultJob : .NET 7.0.16 (7.0.1624.6629), X64 RyuJIT AVX2
```
| Method | Mean | Error | StdDev | Median | Gen0 | Gen1 | Allocated |
|------------------------------------- |----------------:|----------------:|-----------------:|----------------:|----------:|----------:|-----------:|
| Template | 91,883,381.6 ns | 3,632,849.07 ns | 10,597,191.25 ns | 89,313,550.0 ns | 8000.0000 | 2000.0000 | 35435008 B |
| PropertyCopyLoop | 49,633,655.3 ns | 1,505,203.29 ns | 4,318,710.40 ns | 47,957,783.3 ns | 5500.0000 | 2666.6667 | 23333345 B |
| UserDefinedFunctions | 29,551,473.2 ns | 677,400.84 ns | 1,910,621.08 ns | 29,457,092.2 ns | 2187.5000 | 62.5000 | 9336566 B |
| ResolvePolicyAliasPath | 2,408.4 ns | 129.91 ns | 381.01 ns | 2,252.3 ns | 0.2861 | - | 1200 B |
| GetResourceType | 297.3 ns | 9.93 ns | 28.18 ns | 287.5 ns | 0.0858 | - | 360 B |
| CustomTypeDependencyGraph_GetOrdered | 876.7 ns | 17.50 ns | 31.55 ns | 878.1 ns | 0.1602 | - | 672 B |
18 changes: 18 additions & 0 deletions docs/benchmark/results-v1.35.0.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
```
BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3155/23H2/2023Update/SunValley3)
Intel Core i7-1065G7 CPU 1.30GHz, 1 CPU, 8 logical and 4 physical cores
.NET SDK 8.0.200
[Host] : .NET 7.0.16 (7.0.1624.6629), X64 RyuJIT AVX2
DefaultJob : .NET 7.0.16 (7.0.1624.6629), X64 RyuJIT AVX2
```
| Method | Mean | Error | StdDev | Median | Gen0 | Gen1 | Gen2 | Allocated |
|------------------------------------- |-----------------:|-----------------:|-----------------:|-----------------:|----------:|----------:|---------:|-----------:|
| Template | 63,730,486.52 ns | 1,266,452.101 ns | 2,643,557.149 ns | 63,341,771.43 ns | 8285.7143 | 4142.8571 | 142.8571 | 35441751 B |
| PropertyCopyLoop | 39,934,076.76 ns | 773,166.984 ns | 1,852,458.712 ns | 39,569,050.00 ns | 5400.0000 | 100.0000 | - | 23337248 B |
| UserDefinedFunctions | 23,403,397.62 ns | 751,878.865 ns | 2,070,892.753 ns | 22,610,225.00 ns | 2156.2500 | 62.5000 | - | 9336567 B |
| ResolvePolicyAliasPath | 2,284.19 ns | 70.184 ns | 197.956 ns | 2,275.12 ns | 0.2861 | - | - | 1200 B |
| GetResourceType | 254.25 ns | 5.013 ns | 7.805 ns | 252.31 ns | 0.0858 | - | - | 360 B |
| CustomTypeDependencyGraph_GetOrdered | 58.35 ns | 1.192 ns | 2.352 ns | 58.09 ns | 0.0401 | - | - | 168 B |
38 changes: 38 additions & 0 deletions src/PSRule.Rules.Azure.Benchmark/PSRule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Management.Automation;
using System.Reflection;
using BenchmarkDotNet.Attributes;
using Newtonsoft.Json.Linq;
using PSRule.Rules.Azure.Configuration;
using PSRule.Rules.Azure.Data.Policy;
using PSRule.Rules.Azure.Data.Template;
Expand Down Expand Up @@ -34,6 +35,9 @@ public class PSRule
private PSObject _UserDefinedFunctionsTemplate;
private PolicyAliasProviderHelper _PolicyAliasProviderHelper;
private ResourceProviderHelper _ResourceProviderHelper;
private CustomTypeTopologyGraph _CustomTypeDependencyGraph;

#region Setup

[GlobalSetup]
public void Prepare()
Expand All @@ -43,6 +47,7 @@ public void Prepare()
PrepareUserDefinedFunctionsPipeline();
PrepareResolvePolicyAliasPath();
PrepareGetResourceType();
PrepareCustomTypeDependencyGraph();
}

private void PrepareTemplatePipeline()
Expand Down Expand Up @@ -85,6 +90,22 @@ private void PrepareGetResourceType()
_ResourceProviderHelper = new ResourceProviderHelper();
}

private void PrepareCustomTypeDependencyGraph()
{
_CustomTypeDependencyGraph = new CustomTypeTopologyGraph();
_CustomTypeDependencyGraph.Add("base", GetCustomTypeObject());
_CustomTypeDependencyGraph.Add("nestedComplexType", GetCustomTypeObject("complexType"));
_CustomTypeDependencyGraph.Add("basicType", GetCustomTypeObject("object"));
_CustomTypeDependencyGraph.Add("object", GetCustomTypeObject());
_CustomTypeDependencyGraph.Add("array", GetCustomTypeObject("object"));
_CustomTypeDependencyGraph.Add("complexType", GetCustomTypeObject("array"));
_CustomTypeDependencyGraph.Add("newBase", GetCustomTypeObject());
}

#endregion Setup

#region Benchmarks

[Benchmark]
public void Template()
{
Expand Down Expand Up @@ -118,6 +139,16 @@ public void GetResourceType()
_ResourceProviderHelper.GetResourceType("Microsoft.ContainerService", "managedClusters");
}

[Benchmark]
public void CustomTypeDependencyGraph_GetOrdered()
{
_CustomTypeDependencyGraph.GetOrdered();
}

#endregion Benchmarks

#region Helper methods

private static void RunPipelineTargets(IPipeline pipeline, PSObject templateSource)
{
pipeline.Begin();
Expand All @@ -141,5 +172,12 @@ private static ITemplatePipelineBuilder GetTemplatePipeline()
builder.PassThru(true);
return builder;
}

private static JObject GetCustomTypeObject(string ancestor = null)
{
return ancestor == null ? new JObject() : JObject.Parse(string.Concat("{ \"$ref\": \"#/definitions/", ancestor, "\"}"));
}

#endregion Helpers methods
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,33 @@
// Licensed under the MIT License.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Resources;
using System.Text;
using Newtonsoft.Json.Linq;

namespace PSRule.Rules.Azure.Data.Template
{
/// <summary>
/// A graph that tracks dependencies between type definitions.
/// </summary>
internal sealed class CustomTypeDependencyGraph
internal sealed class CustomTypeTopologyGraph
{
private const string PROPERTY_REF = "$ref";

private readonly Dictionary<string, Node> _ById = new(StringComparer.OrdinalIgnoreCase);

[DebuggerDisplay("{Resource.Id}")]
[DebuggerDisplay("{Id}")]
private sealed class Node
{
internal readonly string Id;
internal readonly JObject Value;

/// <summary>
/// The color of the node in the graph.
/// 0 = white, 1 = gray, 2 = black.
/// </summary>
internal int Color = 0;

public Node(string id, JObject value)
{
Id = id;
Expand All @@ -36,7 +41,7 @@ public Node(string id, JObject value)
/// </summary>
/// <param name="name">The custom type node to add to the graph.</param>
/// <param name="value">The definition of the custom type.</param>
internal void Track(string name, JObject value)
internal void Add(string name, JObject value)
{
if (value == null)
return;
Expand All @@ -49,16 +54,14 @@ internal void Track(string name, JObject value)
/// <summary>
/// Get a list of ordered custom types.
/// </summary>
internal IEnumerable<KeyValuePair<string, JObject>> GetOrdered()
internal KeyValuePair<string, JObject>[] GetOrdered()
{
var stack = new List<KeyValuePair<string, JObject>>(_ById.Values.Count);
var visited = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

var result = new List<KeyValuePair<string, JObject>>(_ById.Values.Count);
foreach (var item in _ById.Values)
{
Visit(item, visited, stack);
Visit(item, result);
}
return stack.ToArray();
return result.ToArray();
}

private bool TryGet(string id, out Node item)
Expand All @@ -69,17 +72,18 @@ private bool TryGet(string id, out Node item)
/// <summary>
/// Traverse a node and parent type.
/// </summary>
private void Visit(Node item, HashSet<string> visited, List<KeyValuePair<string, JObject>> stack)
private void Visit(Node item, List<KeyValuePair<string, JObject>> result)
{
if (visited.Contains(item.Id))
if (item.Color != 0)
return;

visited.Add(item.Id);
if (item.Value.TryStringProperty("$ref", out var id) && TryGet(id, out var parent))
item.Color = 1;
if (item.Value.TryStringProperty(PROPERTY_REF, out var id) && TryGet(id, out var parent))
{
Visit(parent, visited, stack);
Visit(parent, result);
}
stack.Add(new KeyValuePair<string, JObject>(item.Id, item.Value));
item.Color = 2;
result.Add(new KeyValuePair<string, JObject>(item.Id, item.Value));
}
}
}
9 changes: 4 additions & 5 deletions src/PSRule.Rules.Azure/Data/Template/TemplateVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -988,19 +988,18 @@ protected virtual void ContentVersion(TemplateContext context, string contentVer

#region Definitions

protected virtual void Definitions(TemplateContext context, JObject definitions)
protected void Definitions(TemplateContext context, JObject definitions)
{
if (definitions == null || definitions.Count == 0)
return;

var graph = new CustomTypeDependencyGraph();
var graph = new CustomTypeTopologyGraph();
foreach (var definition in definitions)
{
graph.Track(definition.Key, definition.Value as JObject);
graph.Add(definition.Key, definition.Value as JObject);
}

var ordered = graph.GetOrdered();
foreach (var definition in ordered)
foreach (var definition in graph.GetOrdered())
{
Definition(context, definition.Key, definition.Value);
}
Expand Down
48 changes: 48 additions & 0 deletions tests/PSRule.Rules.Azure.Tests/CustomTypeTopologyGraphTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Linq;
using Newtonsoft.Json.Linq;
using PSRule.Rules.Azure.Data.Template;
using Xunit;

namespace PSRule.Rules.Azure;

/// <summary>
/// Tests for <see cref="CustomTypeTopologyGraph"/>.
/// </summary>
public sealed class CustomTypeTopologyGraphTests
{
[Fact]
public void Sort_custom_type_graph()
{
var graph = new CustomTypeTopologyGraph();
graph.Add("base", GetObject());
graph.Add("nestedComplexType", GetObject("complexType"));
graph.Add("basicType", GetObject("object"));
graph.Add("object", GetObject());
graph.Add("array", GetObject("object"));
graph.Add("complexType", GetObject("array"));
graph.Add("newBase", GetObject());

var result = graph.GetOrdered().ToArray();
Assert.NotEmpty(result);

Assert.Equal("#/definitions/base", result[0].Key);
Assert.Equal("#/definitions/object", result[1].Key);
Assert.Equal("#/definitions/array", result[2].Key);
Assert.Equal("#/definitions/complexType", result[3].Key);
Assert.Equal("#/definitions/nestedComplexType", result[4].Key);
Assert.Equal("#/definitions/basicType", result[5].Key);
Assert.Equal("#/definitions/newBase", result[6].Key);
}

#region Helper methods

private static JObject GetObject(string ancestor = null)
{
return ancestor == null ? new JObject() : JObject.Parse(string.Concat("{ \"$ref\": \"#/definitions/", ancestor, "\"}"));
}

#endregion Helper methods
}

0 comments on commit c03e3db

Please sign in to comment.