Add a workaround for the tests hanging while loading MKL. (#1076)

* Add a workaround for the tests hanging while loading MKL.

The workaround is to ensure the MKL library is loaded very early in the test process, so it doesn't cause the deadlock.

Workaround #1073

Another deadlock also occurs when running TestAutoInference and TestPipelineSweeper in parallel. Marking these tests to not run in parallel anymore.

Workaround #1095

Moving back to the Azure Hosted VS2017 pool to run the tests now that we've narrowed the deadlocks down.

Author: eerhardt

.vsts-dotnet-ci.yml

@@ -10,7 +10,7 @@ phases:
     name: Windows_NT
     buildScript: build.cmd
     queue:
-      name: DotNetCore-Windows 
+      name: Hosted VS2017 
 
 - template: /build/ci/phase-template.yml
   parameters:

src/Microsoft.ML.PipelineInference/ColumnTypeInference.cs

@@ -132,10 +132,10 @@ public void Apply(IntermediateColumn[] columns)
                     {
                         if (!col.RawData.Skip(1)
                             .All(x =>
-                                {
-                                    bool value;
-                                    return Conversions.Instance.TryParse(ref x, out value);
-                                })
+                            {
+                                bool value;
+                                return Conversions.Instance.TryParse(ref x, out value);
+                            })
                             )
                         {
                             continue;
@@ -157,10 +157,10 @@ public void Apply(IntermediateColumn[] columns)
                     {
                         if (!col.RawData.Skip(1)
                             .All(x =>
-                                {
-                                    Single value;
-                                    return Conversions.Instance.TryParse(ref x, out value);
-                                })
+                            {
+                                Single value;
+                                return Conversions.Instance.TryParse(ref x, out value);
+                            })
                             )
                         {
                             continue;
@@ -240,7 +240,7 @@ private static InferenceResult InferTextFileColumnTypesCore(IHostEnvironment env
             ch.AssertValue(fileSource);
             ch.AssertValue(args);
 
-            if (args.ColumnCount==0)
+            if (args.ColumnCount == 0)
             {
                 ch.Error("Too many empty columns for automatic inference.");
                 return InferenceResult.Fail();

src/Microsoft.ML.PipelineInference/PurposeInference.cs

@@ -339,7 +339,7 @@ public static InferenceResult InferPurposes(IHostEnvironment env, IDataView data
                 if (dataRoles != null)
                 {
                     var items = dataRoles.Schema.GetColumnRoles();
-                    foreach(var item in items)
+                    foreach (var item in items)
                     {
                         Enum.TryParse(item.Key.Value, out ColumnPurpose purpose);
                         var col = cols.Find(x => x.ColumnName == item.Value.Name);

src/Microsoft.ML.PipelineInference/RecipeInference.cs

@@ -547,7 +547,7 @@ public static SuggestedRecipe.SuggestedLearner[] AllowedLearners(IHostEnvironmen
                     LearnerName = tt.Name
                 };
 
-                if (sl.PipelineNode != null && availableLearnersList.FirstOrDefault(l=> l.Name.Equals(sl.PipelineNode.GetEpName())) != null)
+                if (sl.PipelineNode != null && availableLearnersList.FirstOrDefault(l => l.Name.Equals(sl.PipelineNode.GetEpName())) != null)
                     learners.Add(sl);
             }
 

test/Microsoft.ML.Predictor.Tests/Global.cs

@@ -2,12 +2,16 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using Microsoft.ML.Runtime.Internal.Internallearn.Test;
 using Xunit;
 
-namespace Microsoft.ML.Runtime.Internal.Internallearn.Test
+namespace Microsoft.ML.Runtime.RunTests
 {
-    public sealed class GlobalRT
+    public sealed class Global
     {
+        // See https://github.com/dotnet/machinelearning/issues/1095
+        public const string AutoInferenceAndPipelineSweeperTestCollectionName = "TestPipelineSweeper and TestAutoInference should not be run at the same time since it causes deadlocks";
+
         [Fact(Skip = "Disabled")]
         public void AssertHandlerTest()
         {

test/Microsoft.ML.Predictor.Tests/TestAutoInference.cs

@@ -16,6 +16,7 @@
 
 namespace Microsoft.ML.Runtime.RunTests
 {
+    [Collection(Global.AutoInferenceAndPipelineSweeperTestCollectionName)]
     public sealed class TestAutoInference : BaseTestBaseline
     {
         public TestAutoInference(ITestOutputHelper helper)

test/Microsoft.ML.Predictor.Tests/TestPipelineSweeper.cs

@@ -15,16 +15,17 @@
 
 namespace Microsoft.ML.Runtime.RunTests
 {
+    [Collection(Global.AutoInferenceAndPipelineSweeperTestCollectionName)]
     public sealed class TestPipelineSweeper : BaseTestBaseline
     {
         public TestPipelineSweeper(ITestOutputHelper helper)
             : base(helper)
         {
         }
 
-        protected override void InitializeCore()
+        protected override void Initialize()
         {
-            base.InitializeCore();
+            base.Initialize();
             Env.ComponentCatalog.RegisterAssembly(typeof(AutoInference).Assembly);
         }
 

test/Microsoft.ML.Predictor.Tests/TestPredictors.cs

@@ -28,9 +28,9 @@ namespace Microsoft.ML.Runtime.RunTests
     /// </summary>
     public sealed partial class TestPredictors : BaseTestPredictors
     {
-        protected override void InitializeCore()
+        protected override void Initialize()
         {
-            base.InitializeCore();
+            base.Initialize();
             InitializeEnvironment(Env);
         }
 

test/Microsoft.ML.TestFramework/BaseTestBaseline.cs

@@ -9,7 +9,6 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
-using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Text;
 using System.Text.RegularExpressions;
@@ -22,23 +21,12 @@ namespace Microsoft.ML.Runtime.RunTests
     /// <summary>
     /// This is a base test class designed to support baseline comparison.
     /// </summary>
-    public abstract partial class BaseTestBaseline : BaseTestClass, IDisposable
+    public abstract partial class BaseTestBaseline : BaseTestClass
     {
         public const decimal Tolerance = 10_000_000;
-        private readonly ITestOutputHelper _output;
 
-        protected BaseTestBaseline(ITestOutputHelper helper) : base(helper)
+        protected BaseTestBaseline(ITestOutputHelper output) : base(output)
         {
-            _output = helper;
-            ITest test = (ITest)helper.GetType().GetField("test", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(helper);
-            FullTestName = test.TestCase.TestMethod.TestClass.Class.Name + "." + test.TestCase.TestMethod.Method.Name;
-            TestName = test.TestCase.TestMethod.Method.Name;
-            Init();
-        }
-
-        void IDisposable.Dispose()
-        {
-            Cleanup();
         }
 
         internal const string RawSuffix = ".raw";
@@ -93,11 +81,10 @@ void IDisposable.Dispose()
         private bool _normal;
         private bool _passed;
 
-        public string TestName { get; set; }
-        public string FullTestName { get; set; }
-
-        public void Init()
+        protected override void Initialize()
         {
+            base.Initialize();
+
             // Create the output and log directories.
             Contracts.Check(Directory.Exists(Path.Combine(RootDir, TestDir, "BaselineOutput")));
             string logDir = Path.Combine(OutDir, _logRootRelPath);
@@ -111,15 +98,17 @@ public void Init()
             _passed = true;
             Env = new ConsoleEnvironment(42, outWriter: LogWriter, errWriter: LogWriter)
                 .AddStandardComponents();
-            InitializeCore();
         }
 
-        public void Cleanup()
+        // This method is used by subclass to dispose of disposable objects
+        // such as LocalEnvironment.
+        // It is called as a first step in test clean up.
+        protected override void Cleanup()
         {
-            // REVIEW: Is there a way to determine whether the test completed normally?
-            // Requiring tests to call Done() is hokey.
+            if (Env != null)
+                Env.Dispose();
+            Env = null;
 
-            CleanupCore();
             Contracts.Assert(IsActive);
             Log("Test {0}: {1}: {2}", TestName,
                 _normal ? "completed normally" : "aborted",
@@ -128,20 +117,8 @@ public void Cleanup()
             Contracts.AssertValue(LogWriter);
             LogWriter.Dispose();
             LogWriter = null;
-        }
-
-        protected virtual void InitializeCore()
-        {
-        }
 
-        // This method is used by subclass to dispose of disposable objects
-        // such as LocalEnvironment.
-        // It is called as a first step in test clean up.
-        protected virtual void CleanupCore()
-        {
-            if (Env != null)
-                Env.Dispose();
-            Env = null;
+            base.Cleanup();
         }
 
         protected bool IsActive { get { return LogWriter != null; } }
@@ -210,15 +187,15 @@ protected void Log(string msg)
             Contracts.Assert(IsActive);
             Contracts.AssertValue(LogWriter);
             LogWriter.WriteLine(msg);
-            _output.WriteLine(msg);
+            Output.WriteLine(msg);
         }
 
         protected void Log(string fmt, params object[] args)
         {
             Contracts.Assert(IsActive);
             Contracts.AssertValue(LogWriter);
             LogWriter.WriteLine(fmt, args);
-            _output.WriteLine(fmt, args);
+            Output.WriteLine(fmt, args);
         }
 
         protected string GetBaselineDir(string subDir)
@@ -898,4 +875,4 @@ public void TestTimeRegex()
             Done();
         }
     }
-}
+}

test/Microsoft.ML.TestFramework/BaseTestClass.cs

@@ -3,19 +3,24 @@
 // See the LICENSE file in the project root for more information.
 
 using Microsoft.ML.Runtime.Internal.Internallearn.Test;
+using System;
 using System.Globalization;
 using System.IO;
+using System.Reflection;
 using System.Threading;
 using Xunit.Abstractions;
 
 namespace Microsoft.ML.TestFramework
 {
-    public class BaseTestClass
+    public class BaseTestClass : IDisposable
     {
         private readonly string _rootDir;
         private readonly string _outDir;
         private readonly string _dataRoot;
 
+        public string TestName { get; set; }
+        public string FullTestName { get; set; }
+
         static BaseTestClass() => GlobalBase.AssemblyInit();
 
         public BaseTestClass(ITestOutputHelper output)
@@ -31,6 +36,28 @@ public BaseTestClass(ITestOutputHelper output)
             Directory.CreateDirectory(_outDir);
             _dataRoot = Path.Combine(_rootDir, "test", "data");
             Output = output;
+
+            ITest test = (ITest)output.GetType().GetField("test", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(output);
+            FullTestName = test.TestCase.TestMethod.TestClass.Class.Name + "." + test.TestCase.TestMethod.Method.Name;
+            TestName = test.TestCase.TestMethod.Method.Name;
+
+            // write to the console when a test starts and stops so we can identify any test hangs/deadlocks in CI
+            Console.WriteLine($"Starting test: {FullTestName}");
+            Initialize();
+        }
+
+        void IDisposable.Dispose()
+        {
+            Cleanup();
+            Console.WriteLine($"Finished test: {FullTestName}");
+        }
+
+        protected virtual void Initialize()
+        {
+        }
+
+        protected virtual void Cleanup()
+        {
         }
 
         protected string RootDir => _rootDir;

test/Microsoft.ML.TestFramework/DataPipe/Parquet.cs

@@ -10,9 +10,9 @@ namespace Microsoft.ML.Runtime.RunTests
 {
     public sealed partial class TestParquet : TestDataPipeBase
     {
-        protected override void InitializeCore()
+        protected override void Initialize()
         {
-            base.InitializeCore();
+            base.Initialize();
             Env.ComponentCatalog.RegisterAssembly(typeof(ParquetLoader).Assembly);
         }
 

test/Microsoft.ML.TestFramework/GlobalBase.cs

@@ -11,6 +11,7 @@
 //    }
 
 using System;
+using System.Runtime.InteropServices;
 using Xunit;
 
 namespace Microsoft.ML.Runtime.Internal.Internallearn.Test
@@ -22,6 +23,28 @@ public static void AssemblyInit()
             System.Diagnostics.Debug.WriteLine("*** Setting test assertion handler");
             var prev = Contracts.SetAssertHandler(AssertHandler);
             Contracts.Check(prev == null, "Expected to replace null assertion handler!");
+
+            // HACK: ensure MklImports is loaded very early in the tests so it doesn't deadlock while loading it later.
+            // See https://github.com/dotnet/machinelearning/issues/1073
+            Mkl.PptrfInternal(Mkl.Layout.RowMajor, Mkl.UpLo.Up, 0, Array.Empty<double>());
+        }
+
+        private static class Mkl
+        {
+            public enum Layout
+            {
+                RowMajor = 101,
+                ColMajor = 102
+            }
+
+            public enum UpLo : byte
+            {
+                Up = (byte)'U',
+                Lo = (byte)'L'
+            }
+
+            [DllImport("MklImports", EntryPoint = "LAPACKE_dpptrf")]
+            public static extern int PptrfInternal(Layout layout, UpLo uplo, int n, double[] ap);
         }
 
         public static void AssemblyCleanup()