Performance and Memory Improvements 1/x

- Reduce the number of allocations in the app by making the DefaultSubmitHandler instance-based and completing service lookups in the constructor rather than on every request.
An initial pass at tackling the architectural issues with #628.
Also starts to address the DI architectural concerns in #435 via #629, although those are a long way off from solving properly.

src/Microsoft.Restier.Core/ApiBase.cs

@@ -3,7 +3,10 @@
 
 using System;
 using System.Collections.Concurrent;
+using System.Threading;
+using System.Threading.Tasks;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Restier.Core.Submit;
 
 namespace Microsoft.Restier.Core
 {
@@ -12,35 +15,38 @@ namespace Microsoft.Restier.Core
     /// </summary>
     /// <remarks>
     /// <para>
-    /// An API configuration is intended to be long-lived, and can be
-    /// statically cached according to an API type specified when the
-    /// configuration is created. Additionally, the API model produced
-    /// as a result of a particular configuration is cached under the same
+    /// An API configuration is intended to be long-lived, and can be statically cached according to an API type specified when the
+    /// configuration is created. Additionally, the API model produced as a result of a particular configuration is cached under the same
     /// API type to avoid re-computing it on each invocation.
     /// </para>
     /// </remarks>
     public abstract class ApiBase : IDisposable
     {
+
+        #region Private Members
+
         private static ConcurrentDictionary<Type, Action<IServiceCollection>> publisherServicesCallback =
             new ConcurrentDictionary<Type, Action<IServiceCollection>>();
 
         private static readonly Action<IServiceCollection> emptyConfig = _ => { };
 
         private ApiConfiguration apiConfiguration;
 
-        /// <summary>
-        /// Initializes a new instance of the <see cref="ApiBase" /> class.
-        /// </summary>
-        /// <param name="serviceProvider">
-        /// An <see cref="IServiceProvider"/> containing all services of this <see cref="ApiConfiguration"/>.
-        /// </param>
-        protected ApiBase(IServiceProvider serviceProvider) => ServiceProvider = serviceProvider;
+        private readonly DefaultSubmitHandler submitHandler;
+
+        #endregion
+
+        #region Public Properties
 
         /// <summary>
         /// Gets the <see cref="IServiceProvider"/> which contains all services of this <see cref="ApiConfiguration"/>.
         /// </summary>
         public IServiceProvider ServiceProvider { get; private set; }
 
+        #endregion
+
+        #region Internal Properties
+
         /// <summary>
         /// Gets the API configuration for this API.
         /// </summary>
@@ -57,6 +63,26 @@ internal ApiConfiguration Configuration
             }
         }
 
+        #endregion
+
+        #region Constructors
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ApiBase" /> class.
+        /// </summary>
+        /// <param name="serviceProvider">
+        /// An <see cref="IServiceProvider"/> containing all services of this <see cref="ApiConfiguration"/>.
+        /// </param>
+        protected ApiBase(IServiceProvider serviceProvider)
+        {
+            ServiceProvider = serviceProvider;
+            submitHandler = new DefaultSubmitHandler(serviceProvider);
+        }
+
+        #endregion
+
+        #region Static Methods
+
         /// <summary>
         /// Configure services for this API.
         /// </summary>
@@ -99,9 +125,7 @@ public static void AddPublisherServices(Type apiType, Action<IServiceCollection>
         /// <summary>
         /// Get publisher registering service callback for specified Api.
         /// </summary>
-        /// <param name="apiType">
-        /// The Api type of which to get the publisher registering service callback.
-        /// </param>
+        /// <param name="apiType">The Api type of which to get the publisher registering service callback.</param>
         /// <returns>The service registering callback.</returns>
         //[CLSCompliant(false)]
         public static Action<IServiceCollection> GetPublisherServiceCallback(Type apiType)
@@ -114,6 +138,26 @@ public static Action<IServiceCollection> GetPublisherServiceCallback(Type apiTyp
             return emptyConfig;
         }
 
+        #endregion
+
+        #region Public Methods
+
+        /// <summary>
+        /// Asynchronously submits changes made using an API context.
+        /// </summary>
+        /// <param name="changeSet">A change set, or <c>null</c> to submit existing pending changes.</param>
+        /// <param name="cancellationToken">An optional cancellation token.</param>
+        /// <returns>A task that represents the asynchronous operation whose result is a submit result.</returns>
+        public async Task<SubmitResult> SubmitAsync(ChangeSet changeSet = null, CancellationToken cancellationToken = default)
+        {
+            var submitContext = new SubmitContext(ServiceProvider, changeSet);
+            return await submitHandler.SubmitAsync(submitContext, cancellationToken).ConfigureAwait(false);
+        }
+
+        #endregion
+
+        #region IDisposable Pattern
+
         /// <summary>
         /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
         /// </summary>
@@ -140,6 +184,8 @@ protected virtual void Dispose(bool disposing)
             }
         }
 
+        #endregion
+
     }
 
 }

src/Microsoft.Restier.Core/ApiConfiguration.cs

@@ -26,7 +26,7 @@ internal class ApiConfiguration
 
         internal IEdmModel Model { get; private set; }
 
-        internal TaskCompletionSource<IEdmModel> CompeteModelGeneration(out Task<IEdmModel> running)
+        internal TaskCompletionSource<IEdmModel> CompleteModelGeneration(out Task<IEdmModel> running)
         {
             var source = new TaskCompletionSource<IEdmModel>(TaskCreationOptions.AttachedToParent);
             var runningTask = Interlocked.CompareExchange(ref modelTask, source.Task, null);

src/Microsoft.Restier.Core/Exceptions/ChangeSetValidationException.cs

@@ -14,6 +14,9 @@ public class ChangeSetValidationException : Exception
     {
         private IEnumerable<ChangeSetItemValidationResult> errorValidationResults;
 
+        /// <summary>
+        /// 
+        /// </summary>
         public ChangeSetValidationException()
         {
         }

src/Microsoft.Restier.Core/Extensions/ApiBaseExtensions.cs

@@ -177,7 +177,7 @@ public static IEnumerable<T> GetApiServices<T>(this ApiBase api) where T : class
                 throw new InvalidOperationException(Resources.ModelBuilderNotRegistered);
             }
 
-            var source = config.CompeteModelGeneration(out var running);
+            var source = config.CompleteModelGeneration(out var running);
             if (source == null)
             {
                 return await running.ConfigureAwait(false);
@@ -403,34 +403,6 @@ public static IQueryable<TElement> GetQueryableSource<TElement>(this ApiBase api
 
         #endregion
 
-        #region Submit
-
-        /// <summary>
-        /// Asynchronously submits changes made using an API context.
-        /// </summary>
-        /// <param name="api">
-        /// An API.
-        /// </param>
-        /// <param name="changeSet">
-        /// A change set, or <c>null</c> to submit existing pending changes.
-        /// </param>
-        /// <param name="cancellationToken">
-        /// An optional cancellation token.
-        /// </param>
-        /// <returns>
-        /// A task that represents the asynchronous
-        /// operation whose result is a submit result.
-        /// </returns>
-        public static async Task<SubmitResult> SubmitAsync(this ApiBase api, ChangeSet changeSet = null, CancellationToken cancellationToken = default(CancellationToken))
-        {
-            Ensure.NotNull(api, nameof(api));
-
-            var submitContext = new SubmitContext(api.ServiceProvider, changeSet);
-            return await DefaultSubmitHandler.SubmitAsync(submitContext, cancellationToken).ConfigureAwait(false);
-        }
-
-        #endregion
-
         #region GetQueryableSource Private
 
         /// <summary>

src/Microsoft.Restier.Core/Extensions/ServiceCollectionExtensions.cs

@@ -1,14 +1,14 @@
 // Copyright (c) Microsoft Corporation.  All rights reserved.
 // Licensed under the MIT License.  See License.txt in the project root for license information.
 
-using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.DependencyInjection.Extensions;
-using Microsoft.Restier.Core.Query;
-using Microsoft.Restier.Core.Submit;
 using System;
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Restier.Core.Query;
+using Microsoft.Restier.Core.Submit;
 
 namespace Microsoft.Restier.Core
 {

src/Microsoft.Restier.Core/Submit/DefaultSubmitHandler.cs

@@ -10,14 +10,47 @@
 using System.Security;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.Restier.Core.Submit
 {
     /// <summary>
-    /// Represents the default submit handler.
+    /// The default handler for submitting changes through the <see cref="ApiBase"/>.
     /// </summary>
-    internal static class DefaultSubmitHandler
+    internal class DefaultSubmitHandler
     {
+
+        #region Private Members
+
+        private readonly IChangeSetInitializer initializer;
+        private readonly IChangeSetItemAuthorizer authorizer;
+        private readonly IChangeSetItemValidator validator;
+        private readonly IChangeSetItemFilter filter;
+        private readonly ISubmitExecutor executor;
+
+        #endregion
+
+        #region Constructors
+
+        /// <summary>
+        /// 
+        /// </summary>
+        /// <param name="serviceProvider"></param>
+        public DefaultSubmitHandler(IServiceProvider serviceProvider)
+        {
+            //RWM: This stuff SHOULD be getting passed into a constructor. But the DI implementation is less than awesome.
+            //     So we'll work around it for now and still save some allocations.
+            initializer = serviceProvider.GetService<IChangeSetInitializer>();
+            executor = serviceProvider.GetService<ISubmitExecutor>();
+            authorizer = serviceProvider.GetService<IChangeSetItemAuthorizer>();
+            validator = serviceProvider.GetService<IChangeSetItemValidator>();
+            filter = serviceProvider.GetService<IChangeSetItemFilter>();
+        }
+
+        #endregion
+
+        #region Public Methods
+
         /// <summary>
         /// Asynchronously executes the submit flow.
         /// </summary>
@@ -31,17 +64,16 @@ internal static class DefaultSubmitHandler
         /// A task that represents the asynchronous
         /// operation whose result is a submit result.
         /// </returns>
-        public static async Task<SubmitResult> SubmitAsync(SubmitContext context, CancellationToken cancellationToken)
+        public async Task<SubmitResult> SubmitAsync(SubmitContext context, CancellationToken cancellationToken)
         {
             Ensure.NotNull(context, nameof(context));
 
-            var preparer = context.GetApiService<IChangeSetInitializer>();
-            if (preparer == null)
+            if (initializer == null)
             {
                 throw new NotSupportedException(Resources.ChangeSetPreparerMissing);
             }
 
-            await preparer.InitializeAsync(context, cancellationToken).ConfigureAwait(false);
+            await initializer.InitializeAsync(context, cancellationToken).ConfigureAwait(false);
 
             if (context.Result != null)
             {
@@ -65,6 +97,10 @@ public static async Task<SubmitResult> SubmitAsync(SubmitContext context, Cancel
             return context.Result;
         }
 
+        #endregion
+
+        #region Private Methods
+
         private static string GetAuthorizeFailedMessage(ChangeSetItem item)
         {
             switch (item.Type)
@@ -96,7 +132,7 @@ private static string GetAuthorizeFailedMessage(ChangeSetItem item)
             }
         }
 
-        private static async Task PerformValidate(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
+        private async Task PerformValidate(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
         {
             await InvokeAuthorizers(context, changeSetItems, cancellationToken).ConfigureAwait(false);
 
@@ -115,9 +151,8 @@ private static async Task PerformValidate(SubmitContext context, IEnumerable<Cha
             }
         }
 
-        private static async Task InvokeAuthorizers(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
+        private async Task InvokeAuthorizers(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
         {
-            var authorizer = context.GetApiService<IChangeSetItemAuthorizer>();
             if (authorizer == null)
             {
                 return;
@@ -132,9 +167,8 @@ private static async Task InvokeAuthorizers(SubmitContext context, IEnumerable<C
             }
         }
 
-        private static async Task InvokeValidators(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
+        private async Task InvokeValidators(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
         {
-            var validator = context.GetApiService<IChangeSetItemValidator>();
             if (validator == null)
             {
                 return;
@@ -159,11 +193,12 @@ private static async Task InvokeValidators(SubmitContext context, IEnumerable<Ch
             }
         }
 
-        private static async Task PerformPreEvent(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
+        private async Task PerformPreEvent(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
         {
-
-            var processor = context.GetApiService<IChangeSetItemFilter>();
-            //TODO: Should we error out if there is no ChangeSetFilter? No consistent pattern to follow.
+            if (filter == null)
+            {
+                return;
+            }
 
             foreach (var item in changeSetItems)
             {
@@ -172,9 +207,9 @@ private static async Task PerformPreEvent(SubmitContext context, IEnumerable<Cha
                     item.ChangeSetItemProcessingStage = ChangeSetItemProcessingStage.PreEventing;
 
 
-                    if (processor != null)
+                    if (filter != null)
                     {
-                        await processor.OnChangeSetItemProcessingAsync(context, item, cancellationToken).ConfigureAwait(false);
+                        await filter.OnChangeSetItemProcessingAsync(context, item, cancellationToken).ConfigureAwait(false);
                     }
 
                     if (item.ChangeSetItemProcessingStage == ChangeSetItemProcessingStage.PreEventing)
@@ -193,9 +228,8 @@ private static async Task PerformPreEvent(SubmitContext context, IEnumerable<Cha
             }
         }
 
-        private static async Task PerformPersist(SubmitContext context, CancellationToken cancellationToken)
+        private async Task PerformPersist(SubmitContext context, CancellationToken cancellationToken)
         {
-            var executor = context.GetApiService<ISubmitExecutor>();
             if (executor == null)
             {
                 throw new NotSupportedException(Resources.SubmitExecutorMissing);
@@ -204,19 +238,21 @@ private static async Task PerformPersist(SubmitContext context, CancellationToke
             context.Result = await executor.ExecuteSubmitAsync(context, cancellationToken).ConfigureAwait(false);
         }
 
-        private static async Task PerformPostEvent(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
+        private async Task PerformPostEvent(SubmitContext context, IEnumerable<ChangeSetItem> changeSetItems, CancellationToken cancellationToken)
         {
-            var processor = context.GetApiService<IChangeSetItemFilter>();
-            if (processor == null)
+            if (filter == null)
             {
-                //TODO: It feels like we should be following the same pattern as teh method above.
                 return;
             }
 
             foreach (var item in changeSetItems)
             {
-                await processor.OnChangeSetItemProcessedAsync(context, item, cancellationToken).ConfigureAwait(false);
+                await filter.OnChangeSetItemProcessedAsync(context, item, cancellationToken).ConfigureAwait(false);
             }
         }
+
+        #endregion
+
     }
-}
+
+}