Established initial threshold for AttributeDictionary mapping

The `AttributeDictionary` mapping introduces some overhead. If there aren't many attributes that will be mapped, this will actually slow down the mapping service, instead of speeding it up. If there are a lot of attributes that are mapped, however, it will dramatically improve the performance. With the help of the new load tests (3ecfa06, 7b529dc), I determined that this threshold is pretty quick; if 2-3 attributes are mapped, we reach a breakeven point; if more than 5-6 attributes are mapped, we start seeing a significant reduction.

Unfortunately, predetermining if the attributes map to properties is itself expensive, so we're instead just establishing a heuristic. In a typical OnTopic implementation, we typically expect each topic to have 2-3 attributes that won't be mapped with `AttributeDictionary` (such as `Title`, `LastModified`, `LastModifiedBy`). So the assumption here is that if there are five or more attributes, then 2-3 will be mapped, thus reaching that breakeven point.

In addition, if those attributes don't have properties to map to, there isn't any benefit to passing them in via the `AttributeDictionary`. This is even harder to evaluate because many of those will be compatible properties that aren't sourced from attributes (such as `Id`, `Key`, `ContentType`, `WebPath`, &c.)—but that will depend on the model itself, and may not hold for e.g. nested topics or references, or `[MapAs()]` types. In fact, we expect nearly all models to have 5-7 "base" properties as a result of that, in order to maintain compatibility with `ITopicViewModel`. Nevertheless, if there are less than five compatible properties, this likely isn't going to be worth the overhead.

In the future, we may revise this further based on real-world telemetry. Further, we may want to e.g. cache the number of convertible properties on the `TypeAccessor`, and even attempt to estimate the number of non-mapped types based on e.g., whether the target model implements `ITopicViewModel`. That's a bit expensive of a check, but again can be cached as part of the `TypeAccessor.

Author: JeremyCaney

OnTopic/Mapping/TopicMappingService.cs

@@ -170,9 +170,11 @@ public TopicMappingService(ITopicRepository topicRepository, ITypeLookupService
       | Identify parameters
       \-----------------------------------------------------------------------------------------------------------------------*/
       var typeAccessor          = TypeAccessorCache.GetTypeAccessor(type);
+      var properties            = typeAccessor.GetMembers(MemberTypes.Property);
       var parameters            = typeAccessor.ConstructorParameters;
       var arguments             = new object?[parameters.Count];
       var attributeArguments    = (IDictionary<string, string?>)new Dictionary<string, string?>();
+      var parameterQueue        = new Dictionary<int, Task<object?>>();
 
       /*------------------------------------------------------------------------------------------------------------------------
       | Pre-cache entry
@@ -185,15 +187,40 @@ public TopicMappingService(ITopicRepository topicRepository, ITypeLookupService
       cache.Preregister(topic.Id, type);
 
       /*------------------------------------------------------------------------------------------------------------------------
-      | Set parameters
+      | Handle AttributeDictionary constructor
+      >-------------------------------------------------------------------------------------------------------------------------
+      | A model may optionally expose a constructor with a single parameter accepting an AttributeDictionary. In this scenario,
+      | the TopicMappingService may optionally pass a lightweight AttributeDictionary, allowing the model's constructor to
+      | populate scalar values, instead of relying on reflection.
       \-----------------------------------------------------------------------------------------------------------------------*/
-      var parameterQueue        = new Dictionary<int, Task<object?>>();
-
       if (parameters.Count is 1 && parameters[0].Type == typeof(AttributeDictionary)) {
-        var attributes          = topic.Attributes.AsAttributeDictionary(true);
-        arguments[0]            = attributes;
-        attributeArguments      = attributes;
+
+        // This strategy is only performant if there are quite a several scalar properties and they are well-covered by the
+        // attributes. As a fast heuristic to evaluate this, we expect five or more attributes and properties. In practice, this
+        // should be benefitial with any more than mapped attributes, but we also expect that most topics will have 2-3 excluded
+        // or unmapped attributes (e.g., Title, LastModified) and that models will have five or properties that aren't mapped to
+        // attributes (e.g., Id, Key, WebPath). This doesn't guarantee that the attributes map to the properties, but a more
+        // accurate evaluation undermines the performance benefits of this optimization.
+        if (topic.Attributes.Count >= 5 && properties.Count(p => p.IsConvertible) >= 5) {
+          var attributes        = topic.Attributes.AsAttributeDictionary(true);
+          arguments[0]          = attributes;
+          attributeArguments    = attributes;
+        }
+        else {
+          parameters            = new();
+          arguments             = Array.Empty<object?>();
+        }
+
       }
+
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Handle other constructors
+      >-------------------------------------------------------------------------------------------------------------------------
+      | A model may optionally expose a constructor with multiple parameters, which can be defined via reflection in the same
+      | way as properties would be. This is especially useful for records using the positional syntax (i.e., where properties
+      | are defined using the constructor). This also, optionally, provides the model with more control, where needed, over how
+      | it's constructed.
+      \-----------------------------------------------------------------------------------------------------------------------*/
       else {
 
         foreach (var parameter in parameters) {