Skip to content

Commit ee312b2

Browse files
JeremyCaneyJeremyCaney
committed
Merge branch 'improvement/TopicMappingService-refactoring' into develop
The `TopicMappingService` is a potential performance bottleneck, with methods being called potentially hundreds of times for a single page. Given that, I've gone through it carefully to validate extraneous calls. Of significance, I optimized the order of `if`/`else if`/`else` statements so that the cheapest and most common calls were at the top, and to avoid unnecessary variables being defined upfront if they were only needed for, especially, later conditions. Further, I went through the function hierarchy to avoid redundant checks to e.g. `IsList()`. Some of the code is messier as a result, but it should prevent common unnecessary checks (e.g., for reference types or lists). While I was at it, I also removed all `Contract.Requires()` calls on `private` and `internal` methods, not just in the `TopicMappingService`, but across the application. With C# 10's flow analysis, it's now smart enough to recognize that these parameters will not be null at the entry point. We can't rely on this for `public` or `protected` methods, however, since calling applications may not support nullable reference type checking.
2 parents 30f5641 + cba12e0 commit ee312b2

10 files changed

Lines changed: 112 additions & 266 deletions

File tree

‎OnTopic.Data.Sql/SqlCommandExtensions.cs‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,6 @@ private static void AddParameter(
123123
ParameterDirection paramDirection = ParameterDirection.Input
124124
) {
125125

126-
/*------------------------------------------------------------------------------------------------------------------------
127-
| Validate input
128-
\-----------------------------------------------------------------------------------------------------------------------*/
129-
Contract.Requires(command, "The SQL command object must be specified.");
130-
Contract.Requires(command.Parameters, "The SQL command object's parameters collection must be available");
131-
132126
/*------------------------------------------------------------------------------------------------------------------------
133127
| Establish basic parameter
134128
\-----------------------------------------------------------------------------------------------------------------------*/

‎OnTopic/Collections/Specialized/TrackedRecordCollection{TItem,TValue,TAttribute}.cs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,6 @@ public abstract class TrackedRecordCollection<TItem, TValue, TAttribute> :
4545
/// <param name="parentTopic">A reference to the topic that the current collection is bound to.</param>
4646
internal TrackedRecordCollection(Topic parentTopic) : base(StringComparer.OrdinalIgnoreCase) {
4747

48-
/*------------------------------------------------------------------------------------------------------------------------
49-
| Validate parameters
50-
\-----------------------------------------------------------------------------------------------------------------------*/
51-
Contract.Requires(parentTopic, nameof(parentTopic));
52-
5348
/*------------------------------------------------------------------------------------------------------------------------
5449
| Set properties
5550
\-----------------------------------------------------------------------------------------------------------------------*/

‎OnTopic/Internal/Reflection/MemberAccessor.cs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ internal MemberAccessor(MemberInfo memberInfo) {
4040
/*------------------------------------------------------------------------------------------------------------------------
4141
| Validate parameters
4242
\-----------------------------------------------------------------------------------------------------------------------*/
43-
Contract.Requires(memberInfo, nameof(memberInfo));
4443
Contract.Requires(
4544
memberInfo is PropertyInfo or MethodInfo,
4645
$"The {nameof(memberInfo)} parameter must be of type {nameof(PropertyInfo)} or {nameof(MethodInfo)}."
@@ -163,8 +162,6 @@ internal bool IsSettable(Type? sourceType = null, bool allowConversion = false)
163162
/*------------------------------------------------------------------------------------------------------------------------
164163
| Validate parameters
165164
\-----------------------------------------------------------------------------------------------------------------------*/
166-
Contract.Requires(source, nameof(source));
167-
168165
if (MemberInfo.DeclaringType != source.GetType() && !MemberInfo.DeclaringType.IsAssignableFrom(source.GetType())) {
169166
throw new ArgumentException(
170167
$"The {nameof(MemberAccessor)} for {MemberInfo.DeclaringType} cannot be used to access a member of {source.GetType()}",
@@ -214,8 +211,6 @@ internal void SetValue(object target, object? value, bool allowConversion = fals
214211
/*------------------------------------------------------------------------------------------------------------------------
215212
| Validate parameters
216213
\-----------------------------------------------------------------------------------------------------------------------*/
217-
Contract.Requires(target, nameof(target));
218-
219214
if (MemberInfo.DeclaringType != target.GetType() && !MemberInfo.DeclaringType.IsAssignableFrom(target.GetType())) {
220215
throw new ArgumentException(
221216
$"The {nameof(MemberAccessor)} for {MemberInfo.DeclaringType} cannot be used to set a member of {target.GetType()}",

‎OnTopic/Internal/Reflection/TypeAccessor.cs‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,6 @@ internal class TypeAccessor {
6262
/// <param name="type"></param>
6363
internal TypeAccessor(Type type) {
6464

65-
/*------------------------------------------------------------------------------------------------------------------------
66-
| Validate parameters
67-
\-----------------------------------------------------------------------------------------------------------------------*/
68-
Contract.Requires(type, nameof(type));
69-
7065
/*------------------------------------------------------------------------------------------------------------------------
7166
| Initialize fields, properties
7267
\-----------------------------------------------------------------------------------------------------------------------*/
@@ -213,12 +208,6 @@ internal bool HasGettableMethod<T>(string name, Type? targetType = null) where T
213208
/// <returns>The value returned from the member.</returns>
214209
internal object? GetValue(object source, string memberName) {
215210

216-
/*------------------------------------------------------------------------------------------------------------------------
217-
| Validate parameters
218-
\-----------------------------------------------------------------------------------------------------------------------*/
219-
Contract.Requires(source, nameof(source));
220-
Contract.Requires(memberName, nameof(memberName));
221-
222211
/*------------------------------------------------------------------------------------------------------------------------
223212
| Retrieve member
224213
\-----------------------------------------------------------------------------------------------------------------------*/
@@ -335,12 +324,6 @@ internal bool HasSettableMethod<T>(string methodName, Type? targetType = null) w
335324
/// </param>
336325
internal void SetValue(object target, string memberName, object? value, bool allowConversion = false) {
337326

338-
/*------------------------------------------------------------------------------------------------------------------------
339-
| Validate parameters
340-
\-----------------------------------------------------------------------------------------------------------------------*/
341-
Contract.Requires(target, nameof(target));
342-
Contract.Requires(memberName, nameof(memberName));
343-
344327
/*------------------------------------------------------------------------------------------------------------------------
345328
| Validate dependencies
346329
\-----------------------------------------------------------------------------------------------------------------------*/

‎OnTopic/Internal/Reflection/TypeAccessorCache.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ internal static class TypeAccessorCache {
3434
/// <param name="type">The <see cref="Type"/> that needs to be dynamically accessed.</param>
3535
/// <returns>A <see cref="TypeAccessor"/> for dynamically accessing the supplied <paramref name="type"/>.</returns>
3636
internal static TypeAccessor GetTypeAccessor(Type type) {
37-
Contract.Requires(type, nameof(type));
3837
return _cache.GetOrAdd(type, t => new(t));
3938
}
4039

‎OnTopic/Mapping/Internal/ItemConfiguration.cs‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,6 @@ internal class ItemConfiguration {
5151
/// <param name="attributePrefix">The prefix to apply to the attributes.</param>
5252
internal ItemConfiguration(ICustomAttributeProvider source, string name, string? attributePrefix = "") {
5353

54-
/*------------------------------------------------------------------------------------------------------------------------
55-
| Validate parameters
56-
\-----------------------------------------------------------------------------------------------------------------------*/
57-
Contract.Requires(source, nameof(source));
58-
Contract.Requires(name, nameof(name));
59-
6054
/*------------------------------------------------------------------------------------------------------------------------
6155
| Set backing property
6256
\-----------------------------------------------------------------------------------------------------------------------*/

‎OnTopic/Mapping/Internal/MappedTopicCache.cs‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,6 @@ internal bool TryGetValue(int topicId, Type type, [NotNullWhen(true)] out Mapped
5454
/// <param name="viewModel">The mapped view model associated with the <paramref name="topicId"/>.</param>
5555
internal void Register(int topicId, AssociationTypes associations, object viewModel) {
5656

57-
/*------------------------------------------------------------------------------------------------------------------------
58-
| Validate input
59-
\-----------------------------------------------------------------------------------------------------------------------*/
60-
Contract.Requires(topicId, nameof(topicId));
61-
Contract.Requires(associations, nameof(associations));
62-
Contract.Requires(viewModel, nameof(viewModel));
63-
6457
/*------------------------------------------------------------------------------------------------------------------------
6558
| Construct cache entry
6659
\-----------------------------------------------------------------------------------------------------------------------*/
@@ -96,12 +89,6 @@ internal void Register(int topicId, AssociationTypes associations, object viewMo
9689
/// <param name="type">The <see cref="Type"/> that the <see cref="Topic"/> is being mapped to.</param>
9790
internal MappedTopicCacheEntry Preregister(int topicId, Type type) {
9891

99-
/*------------------------------------------------------------------------------------------------------------------------
100-
| Validate input
101-
\-----------------------------------------------------------------------------------------------------------------------*/
102-
Contract.Requires(topicId, nameof(topicId));
103-
Contract.Requires(type, nameof(type));
104-
10592
/*------------------------------------------------------------------------------------------------------------------------
10693
| Construct cache entry
10794
\-----------------------------------------------------------------------------------------------------------------------*/

‎OnTopic/Mapping/Reverse/BindingModelValidator.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,6 @@ [DisallowNull]Type listType
310310
Contract.Requires(sourceType, nameof(sourceType));
311311
Contract.Requires(configuration, nameof(configuration));
312312
Contract.Requires(attributeDescriptor, nameof(attributeDescriptor));
313-
//Contract.Requires(listType, nameof(listType));
314313

315314
/*------------------------------------------------------------------------------------------------------------------------
316315
| Define variables

‎OnTopic/Mapping/Reverse/ReverseTopicMappingService.cs‎

Lines changed: 2 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,6 @@ public ReverseTopicMappingService(ITopicRepository topicRepository) {
192192
\-----------------------------------------------------------------------------------------------------------------------*/
193193
if (source is null) return target;
194194

195-
/*------------------------------------------------------------------------------------------------------------------------
196-
| Validate input
197-
\-----------------------------------------------------------------------------------------------------------------------*/
198-
Contract.Requires(target, nameof(target));
199-
Contract.Assume(target.ContentType, nameof(target.ContentType));
200-
201195
/*------------------------------------------------------------------------------------------------------------------------
202196
| Validate model
203197
\-----------------------------------------------------------------------------------------------------------------------*/
@@ -244,13 +238,6 @@ private async Task SetPropertyAsync(
244238
string? attributePrefix = null
245239
) {
246240

247-
/*------------------------------------------------------------------------------------------------------------------------
248-
| Validate parameters
249-
\-----------------------------------------------------------------------------------------------------------------------*/
250-
Contract.Requires(source, nameof(source));
251-
Contract.Requires(target, nameof(target));
252-
Contract.Requires(property, nameof(property));
253-
254241
/*------------------------------------------------------------------------------------------------------------------------
255242
| Establish per-property variables
256243
\-----------------------------------------------------------------------------------------------------------------------*/
@@ -349,15 +336,7 @@ private static void SetScalarValue(
349336
object source,
350337
Topic target,
351338
PropertyConfiguration configuration
352-
)
353-
{
354-
355-
/*------------------------------------------------------------------------------------------------------------------------
356-
| Validate parameters
357-
\-----------------------------------------------------------------------------------------------------------------------*/
358-
Contract.Requires(source, nameof(source));
359-
Contract.Requires(target, nameof(target));
360-
Contract.Requires(configuration, nameof(configuration));
339+
) {
361340

362341
/*------------------------------------------------------------------------------------------------------------------------
363342
| Attempt to retrieve value from the binding model property
@@ -408,15 +387,7 @@ private void SetRelationships(
408387
object source,
409388
Topic target,
410389
PropertyConfiguration configuration
411-
)
412-
{
413-
414-
/*------------------------------------------------------------------------------------------------------------------------
415-
| Validate parameters
416-
\-----------------------------------------------------------------------------------------------------------------------*/
417-
Contract.Requires(source, nameof(source));
418-
Contract.Requires(target, nameof(target));
419-
Contract.Requires(configuration, nameof(configuration));
390+
) {
420391

421392
/*------------------------------------------------------------------------------------------------------------------------
422393
| Retrieve source list
@@ -468,13 +439,6 @@ private async Task SetNestedTopicsAsync(
468439
PropertyConfiguration configuration
469440
) {
470441

471-
/*------------------------------------------------------------------------------------------------------------------------
472-
| Validate parameters
473-
\-----------------------------------------------------------------------------------------------------------------------*/
474-
Contract.Requires(source, nameof(source));
475-
Contract.Requires(target, nameof(target));
476-
Contract.Requires(configuration, nameof(configuration));
477-
478442
/*------------------------------------------------------------------------------------------------------------------------
479443
| Retrieve source list
480444
\-----------------------------------------------------------------------------------------------------------------------*/
@@ -516,13 +480,6 @@ private void SetReference(
516480
PropertyConfiguration configuration
517481
) {
518482

519-
/*------------------------------------------------------------------------------------------------------------------------
520-
| Validate parameters
521-
\-----------------------------------------------------------------------------------------------------------------------*/
522-
Contract.Requires(source, nameof(source));
523-
Contract.Requires(target, nameof(target));
524-
Contract.Requires(configuration, nameof(configuration));
525-
526483
/*------------------------------------------------------------------------------------------------------------------------
527484
| Retrieve source value
528485
\-----------------------------------------------------------------------------------------------------------------------*/
@@ -578,12 +535,6 @@ private async Task PopulateTargetCollectionAsync(
578535
KeyedTopicCollection targetList
579536
) {
580537

581-
/*------------------------------------------------------------------------------------------------------------------------
582-
| Validate parameters
583-
\-----------------------------------------------------------------------------------------------------------------------*/
584-
Contract.Requires(sourceList, nameof(sourceList));
585-
Contract.Requires(targetList, nameof(targetList));
586-
587538
/*------------------------------------------------------------------------------------------------------------------------
588539
| Queue up mapping tasks
589540
\-----------------------------------------------------------------------------------------------------------------------*/

0 commit comments

Comments
 (0)