Skip to content

Commit 1e3227f

Browse files
pranavkmpranavkm
committed
Changes per PR comments
1 parent 894009e commit 1e3227f

File tree

6 files changed

+105
-28
lines changed

6 files changed

+105
-28
lines changed

‎src/Components/Components/src/Reflection/ComponentProperties.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ static void SetProperty(object target, PropertySetter writer, string parameterNa
159159
}
160160
}
161161

162-
internal static IEnumerable<PropertyInfo> GetCandidateBindableProperties([DynamicallyAccessedMembers(Component)] Type targetType)
162+
internal static MemberAssignment.PropertyEnumerable GetCandidateBindableProperties([DynamicallyAccessedMembers(Component)] Type targetType)
163163
=> MemberAssignment.GetPropertiesIncludingInherited(targetType, _bindablePropertyFlags);
164164

165165
[DoesNotReturn]

‎src/Components/Components/src/Reflection/MemberAssignment.cs

Lines changed: 90 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,21 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.Diagnostics.CodeAnalysis;
78
using System.Reflection;
9+
using System.Runtime.InteropServices;
810
using static Microsoft.AspNetCore.Internal.LinkerFlags;
911

1012
namespace Microsoft.AspNetCore.Components.Reflection
1113
{
1214
internal class MemberAssignment
1315
{
14-
public static IEnumerable<PropertyInfo> GetPropertiesIncludingInherited(
16+
public static PropertyEnumerable GetPropertiesIncludingInherited(
1517
[DynamicallyAccessedMembers(Component)] Type type,
1618
BindingFlags bindingFlags)
1719
{
18-
var dictionary = new Dictionary<string, List<PropertyInfo>>(StringComparer.Ordinal);
20+
var dictionary = new Dictionary<string, OneOrMoreProperties>(StringComparer.Ordinal);
1921

2022
Type? currentType = type;
2123

@@ -26,29 +28,104 @@ public static IEnumerable<PropertyInfo> GetPropertiesIncludingInherited(
2628
{
2729
if (!dictionary.TryGetValue(property.Name, out var others))
2830
{
29-
others = new List<PropertyInfo>();
31+
others = new OneOrMoreProperties { Single = property };
3032
dictionary.Add(property.Name, others);
3133
}
32-
33-
if (others.Exists(other => other.GetMethod?.GetBaseDefinition() == property.GetMethod?.GetBaseDefinition()))
34+
else if (!IsInheritedProperty(property, others))
3435
{
35-
// This is an inheritance case. We can safely ignore the value of property since
36-
// we have seen a more derived value.
37-
continue;
36+
others.Add(property);
3837
}
39-
40-
others.Add(property);
4138
}
4239

4340
currentType = currentType.BaseType;
4441
}
4542

46-
foreach (var list in dictionary.Values)
43+
return new PropertyEnumerable(dictionary);
44+
}
45+
46+
private static bool IsInheritedProperty(PropertyInfo property, OneOrMoreProperties others)
47+
{
48+
if (others.Single is not null)
49+
{
50+
return others.Single.GetMethod?.GetBaseDefinition() == property.GetMethod?.GetBaseDefinition();
51+
}
52+
53+
Debug.Assert(others.Many is not null);
54+
foreach (var other in CollectionsMarshal.AsSpan(others.Many))
55+
{
56+
if (other.GetMethod?.GetBaseDefinition() == property.GetMethod?.GetBaseDefinition())
57+
{
58+
return true;
59+
}
60+
}
61+
62+
return false;
63+
}
64+
65+
public struct OneOrMoreProperties
66+
{
67+
public PropertyInfo? Single;
68+
public List<PropertyInfo>? Many;
69+
70+
public void Add(PropertyInfo property)
71+
{
72+
if (Many is null)
73+
{
74+
Many ??= new() { Single! };
75+
Single = null;
76+
}
77+
78+
Many.Add(property);
79+
}
80+
}
81+
82+
public ref struct PropertyEnumerable
83+
{
84+
private readonly PropertyEnumerator _enumerator;
85+
86+
public PropertyEnumerable(Dictionary<string, OneOrMoreProperties> dictionary)
4787
{
48-
foreach (var property in list)
88+
_enumerator = new PropertyEnumerator(dictionary);
89+
}
90+
91+
public PropertyEnumerator GetEnumerator() => _enumerator;
92+
}
93+
94+
public ref struct PropertyEnumerator
95+
{
96+
// Do NOT make this readonly, or MoveNext will not work
97+
private Dictionary<string, OneOrMoreProperties>.Enumerator _dictionaryEnumerator;
98+
private Span<PropertyInfo>.Enumerator _spanEnumerator;
99+
100+
public PropertyEnumerator(Dictionary<string, OneOrMoreProperties> dictionary)
101+
{
102+
_dictionaryEnumerator = dictionary.GetEnumerator();
103+
_spanEnumerator = Span<PropertyInfo>.Empty.GetEnumerator();
104+
}
105+
106+
public PropertyInfo Current => _spanEnumerator.Current;
107+
108+
public bool MoveNext()
109+
{
110+
if (_spanEnumerator.MoveNext())
49111
{
50-
yield return property;
112+
return true;
51113
}
114+
115+
if (!_dictionaryEnumerator.MoveNext())
116+
{
117+
return false;
118+
}
119+
120+
var oneOrMoreProperties = _dictionaryEnumerator.Current.Value;
121+
var span = oneOrMoreProperties.Single is { } property ?
122+
MemoryMarshal.CreateSpan(ref property, 1) :
123+
CollectionsMarshal.AsSpan(oneOrMoreProperties.Many);
124+
125+
_spanEnumerator = span.GetEnumerator();
126+
var moveNext = _spanEnumerator.MoveNext();
127+
Debug.Assert(moveNext, "We expect this to at least have one item.");
128+
return moveNext;
52129
}
53130
}
54131
}

‎src/Components/Components/src/Routing/RouteTableFactory.cs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,33 +62,34 @@ static void GetRouteableComponents(List<Type> routeableComponents, Assembly asse
6262

6363
internal static RouteTable Create(List<Type> componentTypes)
6464
{
65-
var templatesByHandler = new Dictionary<Type, List<string>>();
65+
var templatesByHandler = new Dictionary<Type, string[]>();
6666
foreach (var componentType in componentTypes)
6767
{
6868
// We're deliberately using inherit = false here.
6969
//
7070
// RouteAttribute is defined as non-inherited, because inheriting a route attribute always causes an
7171
// ambiguity. You end up with two components (base class and derived class) with the same route.
72-
var routeAttributes = componentType.GetCustomAttributes<RouteAttribute>(inherit: false);
73-
var templates = new List<string>(routeAttributes.Length);
74-
foreach (var attribute in routeAttributes)
72+
var routeAttributes = componentType.GetCustomAttributes(typeof(RouteAttribute), inherit: false);
73+
var templates = new string[routeAttributes.Length];
74+
for (var i = 0; i < routeAttributes.Length; i++)
7575
{
76-
templates.Add(attribute.Template);
76+
var attribute = (RouteAttribute)routeAttributes[i];
77+
templates[i] = attribute.Template;
7778
}
7879

7980
templatesByHandler.Add(componentType, templates);
8081
}
8182
return Create(templatesByHandler);
8283
}
8384

84-
internal static RouteTable Create(Dictionary<Type, List<string>> templatesByHandler)
85+
internal static RouteTable Create(Dictionary<Type, string[]> templatesByHandler)
8586
{
8687
var routes = new List<RouteEntry>();
8788
foreach (var (type, templates) in templatesByHandler)
8889
{
8990
var allRouteParameterNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
90-
var parsedTemplates = new (RouteTemplate, HashSet<string>)[templates.Count];
91-
for (var i = 0; i < templates.Count; i++)
91+
var parsedTemplates = new (RouteTemplate, HashSet<string>)[templates.Length];
92+
for (var i = 0; i < templates.Length; i++)
9293
{
9394
var parsedTemplate = TemplateParser.ParseTemplate(templates[i]);
9495
var parameterNames = GetParameterNames(parsedTemplate);

‎src/Components/Components/src/Routing/Router.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ private void RefreshRouteTable()
170170

171171
private void RefreshRouteTableLegacy()
172172
{
173-
var assemblies = AdditionalAssemblies == null ? new[] { AppAssembly } : new[] { AppAssembly }.Concat(AdditionalAssemblies);
173+
var assemblies = AdditionalAssemblies == null ? new[] { AppAssembly } : AdditionalAssemblies.Prepend(AppAssembly);
174174
var assembliesSet = new HashSet<Assembly>(assemblies);
175175
Routes = LegacyRouteTableFactory.Create(assemblies);
176176

‎src/Components/Components/test/CascadingParameterStateTest.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using Microsoft.AspNetCore.Components;
5-
using Microsoft.AspNetCore.Components.Rendering;
6-
using Microsoft.AspNetCore.Components.Test.Helpers;
74
using System;
85
using System.Collections.Generic;
96
using System.Linq;
107
using System.Threading.Tasks;
8+
using Microsoft.AspNetCore.Components.Rendering;
9+
using Microsoft.AspNetCore.Components.Test.Helpers;
1110
using Xunit;
1211

13-
namespace Microsoft.AspNetCore.Components.Test
12+
namespace Microsoft.AspNetCore.Components
1413
{
1514
public class CascadingParameterStateTest
1615
{

‎src/Components/Components/test/Routing/RouteTableFactoryTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,7 @@ public RouteTable Build()
10281028
{
10291029
var templatesByHandler = _routeTemplates
10301030
.GroupBy(rt => rt.Handler)
1031-
.ToDictionary(group => group.Key, group => group.Select(g => g.Template).ToList());
1031+
.ToDictionary(group => group.Key, group => group.Select(g => g.Template).ToArray());
10321032
return RouteTableFactory.Create(templatesByHandler);
10331033
}
10341034
catch (InvalidOperationException ex) when (ex.InnerException is InvalidOperationException)

0 commit comments

Comments
 (0)