From 3b236f3296b6c1f2c6de8a4ead6c34d31c64574d Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 28 Jul 2021 15:59:16 -0500 Subject: [PATCH 1/2] [BaseTasks] improve Task settings in AsyncTaskExtensions Context: https://devblogs.microsoft.com/premier-developer/limiting-concurrency-for-faster-and-more-responsive-apps/ A work item came in from VS where AOT compilation is causing responsiveness issues. The culprit came down to some of the `TaskScheduler` and `TaskCreationOptions` settings in `AsyncTaskExtensions`. @davkean's recommendations are: > This code needs to do two things: > 1) It needs to pass `TaskCreationOptions.LongRunning`, so that we > don't burn queues meant for short lived tasks. > 2) It needs to limit how many tasks it starts at once, to avoid CPU > contention and reduce the number of threads that we end up > burning. Looking at `AsyncTaskExtensions` usage, it would happen during regular builds as well, because aapt2-related tasks use these. To improve these, I'm changing: * New overloads to pass in `int maxConcurrencyLevel` and `TaskCreationOptions`. * These default to `Environment.ProcessorCount * 2` and `LongRunning`. When this change lands, we should potentially pass in `$(Aapt2DaemonMaxInstanceCount)` where appropriate for `maxConcurrencyLevel`. I wrote a few tests to just check general sanity of `AsyncTaskExtensions`. We didn't have any. --- .../AsyncTaskExtensions.cs | 85 ++++++++++++++----- .../AsyncTaskExtensionsTests.cs | 68 +++++++++++++++ 2 files changed, 134 insertions(+), 19 deletions(-) create mode 100644 tests/Microsoft.Android.Build.BaseTasks-Tests/AsyncTaskExtensionsTests.cs diff --git a/src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs b/src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs index 3911cc3..d73667a 100644 --- a/src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs +++ b/src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs @@ -1,4 +1,4 @@ -// https://github.com/xamarin/xamarin-android/blob/83854738b8e01747f9536f426fe17ad784cc2081/src/Xamarin.Android.Build.Tasks/Utilities/AsyncTaskExtensions.cs +// https://github.com/xamarin/xamarin-android/blob/83854738b8e01747f9536f426fe17ad784cc2081/src/Xamarin.Android.Build.Tasks/Utilities/AsyncTaskExtensions.cs using System; using System.Collections.Generic; @@ -12,17 +12,24 @@ public static class AsyncTaskExtensions /// /// Creates a collection of Task with proper CancellationToken and error handling and waits via Task.WhenAll /// - public static Task WhenAll(this AsyncTask asyncTask, IEnumerable source, Action body) + public static Task WhenAll (this AsyncTask asyncTask, IEnumerable source, Action body) => + asyncTask.WhenAll (source, body, maxConcurrencyLevel: DefaultMaxConcurrencyLevel); + + /// + /// Creates a collection of Task with proper CancellationToken and error handling and waits via Task.WhenAll + /// + public static Task WhenAll(this AsyncTask asyncTask, IEnumerable source, Action body, int maxConcurrencyLevel, TaskCreationOptions creationOptions = TaskCreationOptions.LongRunning) { + var scheduler = GetTaskScheduler (maxConcurrencyLevel); var tasks = new List (); foreach (var s in source) { - tasks.Add (Task.Run (() => { + tasks.Add (Task.Factory.StartNew (() => { try { body (s); } catch (Exception exc) { LogErrorAndCancel (asyncTask, exc); } - }, asyncTask.CancellationToken)); + }, asyncTask.CancellationToken, creationOptions, scheduler)); } return Task.WhenAll (tasks); } @@ -31,18 +38,26 @@ public static Task WhenAll(this AsyncTask asyncTask, IEnumerable - public static Task WhenAllWithLock (this AsyncTask asyncTask, IEnumerable source, Action body) + public static Task WhenAllWithLock (this AsyncTask asyncTask, IEnumerable source, Action body) => + asyncTask.WhenAllWithLock (source, body, maxConcurrencyLevel: DefaultMaxConcurrencyLevel); + + /// + /// Creates a collection of Task with proper CancellationToken and error handling and waits via Task.WhenAll + /// Passes an object the inner method can use for locking. The callback is of the form: (T item, object lockObject) + /// + public static Task WhenAllWithLock (this AsyncTask asyncTask, IEnumerable source, Action body, int maxConcurrencyLevel, TaskCreationOptions creationOptions = TaskCreationOptions.LongRunning) { + var scheduler = GetTaskScheduler (maxConcurrencyLevel); var lockObject = new object (); var tasks = new List (); foreach (var s in source) { - tasks.Add (Task.Run (() => { + tasks.Add (Task.Factory.StartNew (() => { try { body (s, lockObject); } catch (Exception exc) { LogErrorAndCancel (asyncTask, exc); } - }, asyncTask.CancellationToken)); + }, asyncTask.CancellationToken, creationOptions, scheduler)); } return Task.WhenAll (tasks); } @@ -50,9 +65,15 @@ public static Task WhenAllWithLock (this AsyncTask asyncTask, IEnumerab /// /// Calls Parallel.ForEach() with appropriate ParallelOptions and exception handling. /// - public static ParallelLoopResult ParallelForEach (this AsyncTask asyncTask, IEnumerable source, Action body) + public static ParallelLoopResult ParallelForEach (this AsyncTask asyncTask, IEnumerable source, Action body) => + asyncTask.ParallelForEach (source, body, maxConcurrencyLevel: DefaultMaxConcurrencyLevel); + + /// + /// Calls Parallel.ForEach() with appropriate ParallelOptions and exception handling. + /// + public static ParallelLoopResult ParallelForEach (this AsyncTask asyncTask, IEnumerable source, Action body, int maxConcurrencyLevel) { - var options = ParallelOptions (asyncTask); + var options = ParallelOptions (asyncTask, maxConcurrencyLevel); return Parallel.ForEach (source, options, s => { try { body (s); @@ -66,9 +87,16 @@ public static ParallelLoopResult ParallelForEach (this AsyncTask asyncT /// Calls Parallel.ForEach() with appropriate ParallelOptions and exception handling. /// Passes an object the inner method can use for locking. The callback is of the form: (T item, object lockObject) /// - public static ParallelLoopResult ParallelForEachWithLock (this AsyncTask asyncTask, IEnumerable source, Action body) + public static ParallelLoopResult ParallelForEachWithLock (this AsyncTask asyncTask, IEnumerable source, Action body) => + asyncTask.ParallelForEachWithLock (source, body, maxConcurrencyLevel: DefaultMaxConcurrencyLevel); + + /// + /// Calls Parallel.ForEach() with appropriate ParallelOptions and exception handling. + /// Passes an object the inner method can use for locking. The callback is of the form: (T item, object lockObject) + /// + public static ParallelLoopResult ParallelForEachWithLock (this AsyncTask asyncTask, IEnumerable source, Action body, int maxConcurrencyLevel) { - var options = ParallelOptions (asyncTask); + var options = ParallelOptions (asyncTask, maxConcurrencyLevel); var lockObject = new object (); return Parallel.ForEach (source, options, s => { try { @@ -79,11 +107,19 @@ public static ParallelLoopResult ParallelForEachWithLock (this AsyncTas }); } - static ParallelOptions ParallelOptions (AsyncTask asyncTask) => new ParallelOptions { + static ParallelOptions ParallelOptions (AsyncTask asyncTask, int maxConcurrencyLevel) => new ParallelOptions { CancellationToken = asyncTask.CancellationToken, - TaskScheduler = TaskScheduler.Default, + TaskScheduler = GetTaskScheduler (maxConcurrencyLevel), }; + static TaskScheduler GetTaskScheduler (int maxConcurrencyLevel) + { + var pair = new ConcurrentExclusiveSchedulerPair (TaskScheduler.Default, maxConcurrencyLevel); + return pair.ConcurrentScheduler; + } + + static int DefaultMaxConcurrencyLevel => Environment.ProcessorCount; + static void LogErrorAndCancel (AsyncTask asyncTask, Exception exc) { asyncTask.LogCodedError ("XA0000", Properties.Resources.XA0000_Exception, exc); @@ -91,16 +127,27 @@ static void LogErrorAndCancel (AsyncTask asyncTask, Exception exc) } /// - /// Calls Task.Run() with a proper CancellationToken. + /// Calls Task.Factory.StartNew() with a proper CancellationToken, TaskScheduler, and TaskCreationOptions.LongRunning. + /// + public static Task RunTask (this AsyncTask asyncTask, Action body) => + asyncTask.RunTask (body, maxConcurrencyLevel: DefaultMaxConcurrencyLevel); + + /// + /// Calls Task.Factory.StartNew() with a proper CancellationToken /// - public static Task RunTask (this AsyncTask asyncTask, Action body) => - Task.Run (body, asyncTask.CancellationToken); + public static Task RunTask (this AsyncTask asyncTask, Action body, int maxConcurrencyLevel, TaskCreationOptions creationOptions = TaskCreationOptions.LongRunning) => + Task.Factory.StartNew (body, asyncTask.CancellationToken, creationOptions, GetTaskScheduler (maxConcurrencyLevel)); + /// + /// Calls Task.Factory.StartNew() with a proper CancellationToken, TaskScheduler, and TaskCreationOptions.LongRunning. + /// + public static Task RunTask (this AsyncTask asyncTask, Func body) => + asyncTask.RunTask (body, maxConcurrencyLevel: DefaultMaxConcurrencyLevel); /// - /// Calls Task.Run() with a proper CancellationToken. + /// Calls Task.Factory.StartNew() with a proper CancellationToken. /// - public static Task RunTask (this AsyncTask asyncTask, Func body) => - Task.Run (body, asyncTask.CancellationToken); + public static Task RunTask (this AsyncTask asyncTask, Func body, int maxConcurrencyLevel, TaskCreationOptions creationOptions = TaskCreationOptions.LongRunning) => + Task.Factory.StartNew (body, asyncTask.CancellationToken, creationOptions, GetTaskScheduler (maxConcurrencyLevel)); } } diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/AsyncTaskExtensionsTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/AsyncTaskExtensionsTests.cs new file mode 100644 index 0000000..ecfc866 --- /dev/null +++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/AsyncTaskExtensionsTests.cs @@ -0,0 +1,68 @@ +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.Android.Build.Tasks; +using NUnit.Framework; +using Xamarin.Build; + +namespace Microsoft.Android.Build.BaseTasks.Tests +{ + [TestFixture] + public class AsyncTaskExtensionsTests + { + const int Iterations = 32; + + [Test] + public async Task RunTask () + { + bool set = false; + await new AsyncTask ().RunTask (delegate { set = true; }); // delegate { } has void return type + Assert.IsTrue (set); + } + + [Test] + public async Task RunTaskOfT () + { + bool set = false; + Assert.IsTrue (await new AsyncTask ().RunTask (() => set = true), "RunTask should return true"); + Assert.IsTrue (set); + } + + [Test] + public async Task WhenAll () + { + bool set = false; + await new AsyncTask ().WhenAll (new [] { 0 }, _ => set = true); + Assert.IsTrue (set); + } + + [Test] + public async Task WhenAllWithLock () + { + var input = new int [Iterations]; + var output = new List (); + await new AsyncTask ().WhenAllWithLock (input, (i, l) => { + lock (l) output.Add (i); + }); + Assert.AreEqual (Iterations, output.Count); + } + + [Test] + public void ParallelForEach () + { + bool set = false; + new AsyncTask ().ParallelForEach (new [] { 0 }, _ => set = true); + Assert.IsTrue (set); + } + + [Test] + public void ParallelForEachWithLock () + { + var input = new int [Iterations]; + var output = new List (); + new AsyncTask ().ParallelForEachWithLock (input, (i, l) => { + lock (l) output.Add (i); + }); + Assert.AreEqual (Iterations, output.Count); + } + } +} From 316e73a4f0207f8cdbed118828bf4fcd23ac90f2 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 29 Jul 2021 09:15:13 -0500 Subject: [PATCH 2/2] Update src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs --- src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs b/src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs index d73667a..fce4bc6 100644 --- a/src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs +++ b/src/Microsoft.Android.Build.BaseTasks/AsyncTaskExtensions.cs @@ -118,7 +118,7 @@ static TaskScheduler GetTaskScheduler (int maxConcurrencyLevel) return pair.ConcurrentScheduler; } - static int DefaultMaxConcurrencyLevel => Environment.ProcessorCount; + static int DefaultMaxConcurrencyLevel => Math.Max (1, Environment.ProcessorCount - 1); static void LogErrorAndCancel (AsyncTask asyncTask, Exception exc) {