Make File.ReadAllText synchroneous since in office build we run out of threads

Make File.ReadAllText synchroneous since in office build we run out of threads to handle the async IO callback.

The real fix is to use the ActionBlock threading model for the frontend so we have fair scheduling... I have started some flights to test the perf impact of makign that the default.

Public/Src/Engine/Dll/FrontEndEngineImplementation.cs

@@ -8,6 +8,7 @@ using System.Diagnostics.CodeAnalysis;
 using System.Diagnostics.ContractsLight;
 using System.IO;
 using System.Linq;
+using System.Text;
 using System.Threading.Tasks;
 using BuildXL.Cache.ContentStore.Hashing;
 using BuildXL.Engine.Cache.Artifacts;
@@ -338,6 +339,41 @@ namespace BuildXL.Engine
             return RetrieveAndTrackFileAsync(physicalPath);
         }
 
+        /// <inheritdoc />
+        public override Possible<string, RecoverableExceptionFailure> GetFileContentSynchronous(AbsolutePath path)
+        {
+            // There is a big problem with the logic in this file.
+            // It is not using the IFileSystem to access the file because that doesn't have any way to get file handles,
+            // so there wouldn't be a way to synchroneously track files for graph caching without massive refactorings.
+            // Therefore any code dpeendent on this can't use the memory filesystem of unittests.
+
+            try
+            {
+                var physicalPath = path.ToString(PathTable, PathFormat.HostOs);
+                using (
+                    FileStream fs = FileUtilities.CreateFileStream(
+                        physicalPath,
+                        FileMode.Open,
+                        FileAccess.Read,
+                        FileShare.Delete | FileShare.Read))
+                using (var reader = new BinaryReader(fs))
+                {
+
+                    var bytes = reader.ReadBytes((int)fs.Length);
+                    var contentHash = ContentHashingUtilities.HashBytes(bytes);
+
+                    m_snapshotCollector?.RecordFile(path);
+                    m_inputTracker.RegisterAccessAndTrackFile(fs.SafeFileHandle, physicalPath, contentHash);
+
+                    return Encoding.UTF8.GetString(bytes); // File.ReadAllText defaults to UTF8 too.
+                }
+            }
+            catch (Exception e)
+            {
+                return new RecoverableExceptionFailure(new BuildXLException(e.Message, e));
+            }
+        }
+
         /// <inheritdoc />
         public override bool FileExists(AbsolutePath path)
         {

Public/Src/Engine/Dll/InputTracker.cs

@@ -14,10 +14,8 @@ using System.Threading;
 using System.Threading.Tasks;
 using BuildXL.Cache.ContentStore.Hashing;
 using BuildXL.Engine.Cache;
-using BuildXL.Engine.Cache.Fingerprints;
 using BuildXL.Engine.Tracing;
 using BuildXL.Native.IO;
-using BuildXL.Scheduler;
 using BuildXL.Storage;
 using BuildXL.Storage.ChangeTracking;
 using BuildXL.Storage.Fingerprints;
@@ -470,6 +468,15 @@ namespace BuildXL.Engine
             RegisterFile(path, hash);
         }
 
+        /// <summary>
+        /// Registers access to a file and trakcs the access.
+        /// </summary>
+        public void RegisterAccessAndTrackFile(SafeFileHandle handle, string path, ContentHash hash)
+        {
+            Analysis.IgnoreResult(FileChangeTracker.TryTrackChangesToFile(handle, path));
+            RegisterFile(path, hash);
+        }
+
         private void InvalidateUnchangedPaths()
         {
             // setting this dictionary to null because the rest of the code already knows how to handle it

Public/Src/FrontEnd/Script/Ambients/AmbientFile.cs

@@ -67,10 +67,22 @@ namespace BuildXL.FrontEnd.Script.Ambients
                         I($"Failed reading '{file.Path.ToString(context.PathTable)}' because the file is not a source file")));
             }
 
-            var possibleContent = context.FrontEndHost.Engine.GetFileContentAsync(file.Path).GetAwaiter().GetResult();
+            // This is a total HACK.
+            // Unfortunately we are forced to use synchronous IO here and blast a hack through to the engine to ensure this file is
+            // tracked for graph caching. The reason is that we use Task.Run to spawn frontend threads and for larger builds we can have
+            // saturated the threadpool. So when during evaluation we perform IO there is no thread available to continue the work once the
+            // OS returns the data. We have a mode where do use an ActionBlock ActionBlock to schedule the work. This can be turned on via `/enableEvaluationThrottling+`
+            // but this was never made the default because measuring at the time resulted in about a 2x slowdown for the office build.
+            // Since AB testing will take a long time since we need to get averages of metabuild duration now that most evaluation happens there.
+            // So to unblock for now we will perform synchronous IO :(
+
+            // There is also another small problem with this change. It is not using the IFileSystem to access the file because that doesn't have any way to get
+            // file handles, so the memory filesystem of unittests won't function.
+
+            var possibleContent = context.FrontEndHost.Engine.GetFileContentSynchronous(file.Path);
             if (possibleContent.Succeeded)
             {
-                return EvaluationResult.Create(possibleContent.Result.GetContentAsString());
+                return EvaluationResult.Create(possibleContent.Result);
             }
 
             throw new FileOperationException(

Public/Src/FrontEnd/Sdk/FrontEndEngineAbstraction.cs

@@ -82,6 +82,11 @@ namespace BuildXL.FrontEnd.Sdk
         /// </summary>
         public abstract Task<Possible<FileContent, RecoverableExceptionFailure>> GetFileContentAsync(AbsolutePath path);
 
+        /// <summary>
+        /// Reads file in a synchronously. Only use in cases where really needed.
+        /// </summary>
+        public abstract Possible<string, RecoverableExceptionFailure> GetFileContentSynchronous(AbsolutePath path);
+
         /// <summary>
         /// Determines whether the specified file exists
         /// </summary>

Public/Src/FrontEnd/Sdk/SimpleFrontEndEngineAbstraction.cs

@@ -109,6 +109,12 @@ namespace BuildXL.FrontEnd.Sdk
             return new Possible<FileContent, RecoverableExceptionFailure>(new RecoverableExceptionFailure(new BuildXLException(message, new FileNotFoundException(message))));
         }
 
+        /// <inheritdoc />
+        public override Possible<string, RecoverableExceptionFailure> GetFileContentSynchronous(AbsolutePath path)
+        {
+            return File.ReadAllText(path.ToString(m_pathTable));
+        }
+
         /// <inheritdoc />
         public override bool FileExists(AbsolutePath path)
         {

Public/Src/FrontEnd/SdkTesting/Helper/TestEngineAbstraction.cs

@@ -80,6 +80,12 @@ namespace BuildXL.FrontEnd.Script.Testing.Helper
             return FileContent.Invalid;
         }
 
+        /// <inheritdoc />
+        public override Possible<string, RecoverableExceptionFailure> GetFileContentSynchronous(AbsolutePath path)
+        {
+            return File.ReadAllText(path.ToString(m_pathTable));
+        }
+
         /// <inheritdoc />
         public override bool FileExists(AbsolutePath path)
         {

Public/Src/FrontEnd/UnitTests/Script.Ast/Consumers/Office/AmbientTests.cs

@@ -192,27 +192,7 @@ const relocated = p`D:/a/b/foo.cs`.relocate(d`D:/a/b`, d`D:/c/d`);
             Assert.Equal(CreateAbsolutePath(@"D:\path\to\a\file.out"), results["changeExtension"]);
             Assert.Equal(CreateAbsolutePath(@"D:\c\d\foo.cs"), results["relocated"]);
         }
-
+        
-        [FactIfSupported(requiresWindowsBasedOperatingSystem: true)]
-        public void TestAmbientFileUses()
-        {
-            const string Spec = @"
-// Any change will break Office.
-const file = f`D:/path/to/a/file.txt`;
-const filePath = file.path;
-const fileContent = File.readAllText(f`file.txt`);
-";
-            var results = Build()
-                .Spec(Spec)
-                .AddFile("file.txt", "Hello")
-                .EvaluateExpressionsWithNoErrors("file", "filePath", "fileContent");
-
-            Assert.IsType<FileArtifact>(results["file"]);
-            Assert.True(((FileArtifact) results["file"]).IsSourceFile);
-            Assert.Equal(CreateAbsolutePath(@"D:\path\to\a\file.txt"), results["filePath"]);
-            Assert.Equal("Hello", results["fileContent"]);
-        }
-
         [FactIfSupported(requiresWindowsBasedOperatingSystem: true)]
         public void TestAmbientDirectoryUses()
         {

Public/Src/FrontEnd/UnitTests/Script.Ast/Consumers/Office/AmbientTestsRealFileSystem.cs

@@ -0,0 +1,60 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using BuildXL.Pips;
+using BuildXL.Utilities;
+using BuildXL.FrontEnd.Script.Values;
+using Test.BuildXL.TestUtilities.Xunit;
+using Test.BuildXL.FrontEnd.Core;
+using Xunit;
+using Xunit.Abstractions;
+using LineInfo = TypeScript.Net.Utilities.LineInfo;
+
+namespace Test.DScript.Ast.Consumers.Office
+{
+    /// <summary>
+    /// Tests for API used by Office.
+    /// </summary>
+    /// <remarks>
+    /// Any change will break Office.
+    /// </remarks>
+    [Trait("Category", "Office")]
+    public sealed class AmbientTestsRealFileSystem : DsTest
+    {
+        public AmbientTestsRealFileSystem(ITestOutputHelper output)
+            : base(output, usePassThroughFileSystem: true)
+        {
+        }
+        
+        [FactIfSupported(requiresWindowsBasedOperatingSystem: true)]
+        public void TestAmbientFileUses()
+        {
+            FileSystem = new PassThroughMutableFileSystem(PathTable);
+            RelativeSourceRoot = System.IO.Path.Combine(TemporaryDirectory, Guid.NewGuid().ToString());
+
+            const string Spec = @"
+// Any change will break Office.
+const file = f`D:/path/to/a/file.txt`;
+const filePath = file.path;
+const fileContent = File.readAllText(f`file.txt`);
+";
+            var results = Build()
+                .Spec(Spec)
+                .AddFile("file.txt", "Hello")
+                .EvaluateExpressionsWithNoErrors("file", "filePath", "fileContent");
+
+            Assert.IsType<FileArtifact>(results["file"]);
+            Assert.True(((FileArtifact) results["file"]).IsSourceFile);
+            Assert.Equal(CreateAbsolutePath(@"D:\path\to\a\file.txt"), results["filePath"]);
+            Assert.Equal("Hello", results["fileContent"]);
+        }
+
+        private AbsolutePath CreateAbsolutePath(string path)
+        {
+            return AbsolutePath.Create(PathTable, path);
+        }
+    }
+}

config.microsoftInternal.dsc

@@ -56,7 +56,7 @@ export const pkgs = isMicrosoftInternal ? [
 
 ];
 
-// This contains facade modules for the packages that are only availalbe internally
+// This contains facade modules for the packages that are only available internally
 export const resolver = {
     kind: "SourceResolver",
     modules: [