OData / ODataConnectedService

A Visual Studio extension for generating client code for OData Services
Other
81 stars 44 forks source link

Remove sync calls and replace them with async calls. #411

Closed marabooy closed 1 month ago

marabooy commented 2 months ago

Replaces Synchronous I/O with async I/O. ~Note that some calls had to be re-ordered as it seems having async calls then updating UI's afterwards for some components will always result in exceptions. The cause may be due to the threading model being quite different from the windows SDK in the tests.~

Seems this was not needed as ConfigureAwait(true) resolves these as it resumes the thread in the main thread. However the tests need to be updated to run using [STA mode]. (https://www.nuget.org/packages/Xunit.StaFact)

Note though this improve some of the UI updates to be more responsive. There is a performance penalty that lies within these lines here in ConnectedServiceFileHandler.

       /// <summary>
        /// Adds a file to a target path.
        /// </summary>
        /// <param name="fileName">The name of the file</param>
        /// <param name="targetPath">The path target where you want to copy a file to </param>
        /// <param name="oDataFileOptions">The options to use when adding a file to a target path.</param>
        /// <returns>Returns the path to the file that was added</returns>
        public async Task<string> AddFileAsync(string fileName, string targetPath, ODataFileOptions oDataFileOptions)
        {
            if (oDataFileOptions != null)
            {
                return await this.Context.HandlerHelper.AddFileAsync(fileName, targetPath, new AddFileOptions { SuppressOverwritePrompt = oDataFileOptions.SuppressOverwritePrompt, OpenOnComplete = oDataFileOptions.OpenOnComplete }).ConfigureAwait(true);
            }
            else
            {
                return await this.Context.HandlerHelper.AddFileAsync(fileName, targetPath).ConfigureAwait(true);
            }
        }

Benchmarking the code shows that this could end-up causing some freezes for when files are many some considerations should be past a certain threshold (e.g. 200 files) we switch ask the user to switch to a Namespace grouping for better performance or the CLI tool.

~This this.Context.HandlerHelper.AddFileAsync API on multiple runs averaged 40 file additions per minute.~ I need to re-run this benchmark again as this seems to not be the case anymore Seems there are some random variations in the computer. I was running these tests that caused the slowdown earlier

Another alternative would be to switch to the MSBuild same as with the CLI project or Project items (https://learn.microsoft.com/en-us/dotnet/api/envdte.projectitems?view=visualstudiosdk-2022) which in theory could provide better and faster API's.

Some quick benefits from this are

  1. Filtering schema items is now faster and causes less re-draws since we only paint one page at a time.
  2. Rendering schema items renders a paginated list which has less items hence the draw operation is fast.
  3. Model is cached between pages from Config OData endpoint page to the final page reducing IO during page transitions.
  4. The Async handlers removes duplicated code which overrode the current implementation with a similar method.
  5. During generation of multiple files, we no-longer read existing files to determine if they are different, we truncate and overwrite the generated file. instead.) This reduces the IO operations and CPU operations. Note since these are done in the temp directory and not in the VS directory the benefit of checking if the file is similar is marginal.

Some future improvements will be:-

  1. Compare generated files with existing project files before adding (note the read could be expensive).
  2. Add a namespace grouping e.g. If the number of files exceed a certain number prompt the user to prefer namespace grouping instead based on $n= count(SchemaTypes)\ m = count(Namespaces)\ :then\ n >= m$ in almost all cases. So if $C=n/m$ we can always get a $C$ speedup while using namespace grouping.
marabooy commented 2 months ago

@microsoft-github-policy-service agree company="Microsoft"

ElizabethOkerio commented 2 months ago

Could you please provide more information on the calls that you've had to reorder and the UI updates that have been more responsive because of this. Also, the benefits this change has added to what we had before.

marabooy commented 1 month ago

Closing this as it has been broken down to smaller prs