diff --git a/src/Build/BackEnd/BuildManager/BuildParameters.cs b/src/Build/BackEnd/BuildManager/BuildParameters.cs index 9ae5c5f5bde..dfa1a28e713 100644 --- a/src/Build/BackEnd/BuildManager/BuildParameters.cs +++ b/src/Build/BackEnd/BuildManager/BuildParameters.cs @@ -1,9 +1,9 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Frozen; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Threading; @@ -129,7 +129,7 @@ public class BuildParameters : ITranslatable /// /// The original process environment. /// - private Dictionary _buildProcessEnvironment; + private FrozenDictionary _buildProcessEnvironment; /// /// The environment properties for the build. @@ -282,9 +282,7 @@ internal BuildParameters(BuildParameters other, bool resetEnvironment = false) _enableRarNode = other._enableRarNode; _buildProcessEnvironment = resetEnvironment ? CommunicationsUtilities.GetEnvironmentVariables() - : other._buildProcessEnvironment != null - ? new Dictionary(other._buildProcessEnvironment) - : null; + : other._buildProcessEnvironment; _environmentProperties = other._environmentProperties != null ? new PropertyDictionary(other._environmentProperties) : null; _forwardingLoggers = other._forwardingLoggers != null ? new List(other._forwardingLoggers) : null; _globalProperties = other._globalProperties != null ? new PropertyDictionary(other._globalProperties) : null; @@ -356,8 +354,7 @@ public bool UseSynchronousLogging /// /// Gets the environment variables which were set when this build was created. /// - public IDictionary BuildProcessEnvironment => new ReadOnlyDictionary( - _buildProcessEnvironment ?? new Dictionary(0)); + public IDictionary BuildProcessEnvironment => BuildProcessEnvironmentInternal; /// /// The name of the culture to use during the build. @@ -720,6 +717,8 @@ internal int BuildId set => _buildId = value; } + internal FrozenDictionary BuildProcessEnvironmentInternal => _buildProcessEnvironment ?? FrozenDictionary.Empty; + /// /// Gets or sets the environment properties. /// diff --git a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs index 3f08179f500..25470b8d763 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Concurrent; +using System.Collections.Frozen; using System.Collections.Generic; using System.Globalization; using System.IO; @@ -1378,7 +1379,7 @@ private void InitializeOperatingEnvironment() else { // Restore the original build environment variables. - SetEnvironmentVariableBlock(_componentHost.BuildParameters.BuildProcessEnvironment); + SetEnvironmentVariableBlock(_componentHost.BuildParameters.BuildProcessEnvironmentInternal); } } @@ -1401,9 +1402,9 @@ private void RestoreOperatingEnvironment() /// /// Sets the environment block to the set of saved variables. /// - private void SetEnvironmentVariableBlock(IDictionary savedEnvironment) + private void SetEnvironmentVariableBlock(FrozenDictionary savedEnvironment) { - IDictionary currentEnvironment = CommunicationsUtilities.GetEnvironmentVariables(); + FrozenDictionary currentEnvironment = CommunicationsUtilities.GetEnvironmentVariables(); ClearVariablesNotInEnvironment(savedEnvironment, currentEnvironment); UpdateEnvironmentVariables(savedEnvironment, currentEnvironment); } @@ -1411,7 +1412,7 @@ private void SetEnvironmentVariableBlock(IDictionary savedEnviro /// /// Clears from the current environment any variables which do not exist in the saved environment /// - private void ClearVariablesNotInEnvironment(IDictionary savedEnvironment, IDictionary currentEnvironment) + private void ClearVariablesNotInEnvironment(FrozenDictionary savedEnvironment, FrozenDictionary currentEnvironment) { foreach (KeyValuePair entry in currentEnvironment) { @@ -1425,7 +1426,7 @@ private void ClearVariablesNotInEnvironment(IDictionary savedEnv /// /// Updates the current environment with values in the saved environment which differ or are not yet set. /// - private void UpdateEnvironmentVariables(IDictionary savedEnvironment, IDictionary currentEnvironment) + private void UpdateEnvironmentVariables(FrozenDictionary savedEnvironment, FrozenDictionary currentEnvironment) { foreach (KeyValuePair entry in savedEnvironment) { diff --git a/src/Build/BackEnd/Node/InProcNode.cs b/src/Build/BackEnd/Node/InProcNode.cs index 33d5fb6359d..4a18ea7df8a 100644 --- a/src/Build/BackEnd/Node/InProcNode.cs +++ b/src/Build/BackEnd/Node/InProcNode.cs @@ -1,9 +1,9 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; using System.Collections.Concurrent; -using System.Collections.Generic; +using System.Collections.Frozen; using System.Globalization; using System.Threading; using Microsoft.Build.BackEnd.Components.Caching; @@ -31,7 +31,7 @@ internal class InProcNode : INode, INodePacketFactory /// /// The environment at the time the build is started. /// - private IDictionary _savedEnvironment; + private FrozenDictionary _savedEnvironment; /// /// The current directory at the time the build is started. diff --git a/src/Build/BackEnd/Node/OutOfProcNode.cs b/src/Build/BackEnd/Node/OutOfProcNode.cs index f28b906077a..e5aeabc0fe6 100644 --- a/src/Build/BackEnd/Node/OutOfProcNode.cs +++ b/src/Build/BackEnd/Node/OutOfProcNode.cs @@ -1,8 +1,9 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; using System.Collections.Concurrent; +using System.Collections.Frozen; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; @@ -50,7 +51,7 @@ public class OutOfProcNode : INode, IBuildComponentHost, INodePacketFactory, INo /// /// The saved environment for the process. /// - private IDictionary _savedEnvironment; + private FrozenDictionary _savedEnvironment; /// /// The component factories. diff --git a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs index d2c22d49f76..7ae5c75c7c3 100644 --- a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs +++ b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs @@ -1,7 +1,8 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Frozen; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; @@ -131,7 +132,7 @@ internal class BuildRequestConfiguration : IEquatable /// /// Holds a snapshot of the environment at the time we blocked. /// - private Dictionary _savedEnvironmentVariables; + private FrozenDictionary _savedEnvironmentVariables; /// /// Holds a snapshot of the current working directory at the time we blocked. @@ -611,7 +612,7 @@ public Lookup BaseLookup /// /// Holds a snapshot of the environment at the time we blocked. /// - public Dictionary SavedEnvironmentVariables + public FrozenDictionary SavedEnvironmentVariables { get => _savedEnvironmentVariables; diff --git a/src/Build/BackEnd/Shared/BuildResult.cs b/src/Build/BackEnd/Shared/BuildResult.cs index 1ef8455bdf1..3a7ac7169dd 100644 --- a/src/Build/BackEnd/Shared/BuildResult.cs +++ b/src/Build/BackEnd/Shared/BuildResult.cs @@ -1,8 +1,9 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; using System.Collections.Concurrent; +using System.Collections.Frozen; using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -107,7 +108,7 @@ public class BuildResult : BuildResultBase, INodePacket, IBuildResults /// Snapshot of the environment from the configuration this results comes from. /// This should only be populated when the configuration for this result is moved between nodes. /// - private Dictionary? _savedEnvironmentVariables; + private FrozenDictionary? _savedEnvironmentVariables; /// /// When this key is in the dictionary , serialize the build result version. @@ -437,7 +438,7 @@ NodePacketType INodePacket.Type /// /// Holds a snapshot of the environment at the time we blocked. /// - Dictionary? IBuildResults.SavedEnvironmentVariables + FrozenDictionary? IBuildResults.SavedEnvironmentVariables { get => _savedEnvironmentVariables; @@ -655,6 +656,7 @@ void ITranslatable.Translate(ITranslator translator) } else { + IDictionary? savedEnvironmentVariables = _savedEnvironmentVariables; Dictionary additionalEntries = new(); if (translator.Mode == TranslationDirection.WriteToStream) @@ -663,7 +665,7 @@ void ITranslatable.Translate(ITranslator translator) additionalEntries.Add(SpecialKeyForVersion, String.Empty); // Serialize the special key together with _savedEnvironmentVariables dictionary using the workaround overload of TranslateDictionary: - translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase, ref additionalEntries, s_additionalEntriesKeys); + translator.TranslateDictionary(ref savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase, ref additionalEntries, s_additionalEntriesKeys); // Serialize version translator.Translate(ref _version); @@ -671,7 +673,8 @@ void ITranslatable.Translate(ITranslator translator) else if (translator.Mode == TranslationDirection.ReadFromStream) { // Read the dictionary using the workaround overload of TranslateDictionary: special keys (additionalEntriesKeys) would be read to additionalEntries instead of the _savedEnvironmentVariables dictionary. - translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase, ref additionalEntries, s_additionalEntriesKeys); + translator.TranslateDictionary(ref savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase, ref additionalEntries, s_additionalEntriesKeys); + _savedEnvironmentVariables = savedEnvironmentVariables?.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase); // If the special key SpecialKeyForVersion present in additionalEntries, also read a version, otherwise set it to 0. if (additionalEntries is not null && additionalEntries.ContainsKey(SpecialKeyForVersion)) diff --git a/src/Build/BackEnd/Shared/IBuildResults.cs b/src/Build/BackEnd/Shared/IBuildResults.cs index 8dbd46dc8ac..17131d84295 100644 --- a/src/Build/BackEnd/Shared/IBuildResults.cs +++ b/src/Build/BackEnd/Shared/IBuildResults.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Frozen; using System.Collections.Generic; using Microsoft.Build.Execution; @@ -32,7 +33,7 @@ internal interface IBuildResults /// /// Set of environment variables for the configuration this result came from /// - Dictionary SavedEnvironmentVariables { get; set; } + FrozenDictionary SavedEnvironmentVariables { get; set; } /// /// The current directory for the configuration this result came from diff --git a/src/Framework/BinaryTranslator.cs b/src/Framework/BinaryTranslator.cs index 266fb0cfe5b..cf029907c24 100644 --- a/src/Framework/BinaryTranslator.cs +++ b/src/Framework/BinaryTranslator.cs @@ -601,7 +601,7 @@ public void TranslateDictionary(ref Dictionary dictionary, IEqua /// This overload is needed for a workaround concerning serializing BuildResult with a version. /// It deserializes additional entries together with the main dictionary. /// - public void TranslateDictionary(ref Dictionary dictionary, IEqualityComparer comparer, ref Dictionary additionalEntries, HashSet additionalEntriesKeys) + public void TranslateDictionary(ref IDictionary dictionary, IEqualityComparer comparer, ref Dictionary additionalEntries, HashSet additionalEntriesKeys) { if (!TranslateNullable(dictionary)) { @@ -1383,7 +1383,7 @@ public void TranslateDictionary(ref Dictionary dictionary, IEqua /// This overload is needed for a workaround concerning serializing BuildResult with a version. /// It serializes additional entries together with the main dictionary. /// - public void TranslateDictionary(ref Dictionary dictionary, IEqualityComparer comparer, ref Dictionary additionalEntries, HashSet additionalEntriesKeys) + public void TranslateDictionary(ref IDictionary dictionary, IEqualityComparer comparer, ref Dictionary additionalEntries, HashSet additionalEntriesKeys) { // Translate whether object is null if ((dictionary is null) && ((additionalEntries is null) || (additionalEntries.Count == 0))) diff --git a/src/Framework/ITranslator.cs b/src/Framework/ITranslator.cs index c33ba195c03..cdae7d8cd43 100644 --- a/src/Framework/ITranslator.cs +++ b/src/Framework/ITranslator.cs @@ -318,7 +318,7 @@ void TranslateArray(ref T[] array) /// This overload is needed for a workaround concerning serializing BuildResult with a version. /// It serializes/deserializes additional entries together with the main dictionary. /// - void TranslateDictionary(ref Dictionary dictionary, IEqualityComparer comparer, ref Dictionary additionalEntries, HashSet additionalEntriesKeys); + void TranslateDictionary(ref IDictionary dictionary, IEqualityComparer comparer, ref Dictionary additionalEntries, HashSet additionalEntriesKeys); void TranslateDictionary(ref IDictionary dictionary, NodePacketCollectionCreator> collectionCreator); diff --git a/src/Shared/CommunicationsUtilities.cs b/src/Shared/CommunicationsUtilities.cs index 13d4f1bef25..8a3c9546504 100644 --- a/src/Shared/CommunicationsUtilities.cs +++ b/src/Shared/CommunicationsUtilities.cs @@ -7,9 +7,7 @@ using System.Globalization; using System.IO; using System.IO.Pipes; -#if NETFRAMEWORK using System.Runtime.InteropServices; -#endif #if FEATURE_SECURITY_PRINCIPAL_WINDOWS using System.Security.Principal; #endif @@ -23,6 +21,9 @@ #if !CLR2COMPATIBILITY using Microsoft.Build.Shared.Debugging; +using System.Collections; +using System.Collections.Frozen; +using Microsoft.NET.StringTools; #endif #if !FEATURE_APM using System.Threading.Tasks; @@ -246,6 +247,14 @@ internal static class CommunicationsUtilities /// private static long s_lastLoggedTicks = DateTime.UtcNow.Ticks; +#if !CLR2COMPATIBILITY + /// + /// A set of environment variables cached from the last time we called GetEnvironmentVariables. + /// Used to avoid allocations if the environment has not changed. + /// + private static EnvironmentState s_environmentState; +#endif + /// /// Delegate to debug the communication utilities. /// @@ -259,19 +268,21 @@ internal static int NodeConnectionTimeout get { return GetIntegerVariableOrDefault("MSBUILDNODECONNECTIONTIMEOUT", DefaultNodeConnectionTimeout); } } -#if NETFRAMEWORK /// /// Get environment block. /// [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] + [System.Runtime.Versioning.SupportedOSPlatform("windows")] internal static extern unsafe char* GetEnvironmentStrings(); /// /// Free environment block. /// [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] + [System.Runtime.Versioning.SupportedOSPlatform("windows")] internal static extern unsafe bool FreeEnvironmentStrings(char* pStrings); +#if NETFRAMEWORK /// /// Set environment variable P/Invoke. /// @@ -293,6 +304,16 @@ internal static void SetEnvironmentVariable(string name, string value) throw Marshal.GetExceptionForHR(Marshal.GetHRForLastWin32Error()); } } +#endif + +#if !CLR2COMPATIBILITY + /// + /// A container to atomically swap a cached set of environment variables and the block string used to create it. + /// The environment block property will only be set on Windows, since on Unix we need to directly call + /// Environment.GetEnvironmentVariables(). + /// + private sealed record class EnvironmentState(FrozenDictionary EnvironmentVariables, ReadOnlyMemory EnvironmentBlock = default); +#endif /// /// Returns key value pairs of environment variables in a new dictionary @@ -301,17 +322,19 @@ internal static void SetEnvironmentVariable(string name, string value) /// /// Copied from the BCL implementation to eliminate some expensive security asserts on .NET Framework. /// +#if CLR2COMPATIBILITY internal static Dictionary GetEnvironmentVariables() { -#if !CLR2COMPATIBILITY +#else + [System.Runtime.Versioning.SupportedOSPlatform("windows")] + private static FrozenDictionary GetEnvironmentVariablesWindows() + { // The DebugUtils static constructor can set the MSBUILDDEBUGPATH environment variable to propagate the debug path to out of proc nodes. // Need to ensure that constructor is called before this method returns in order to capture its env var write. // Otherwise the env var is not captured and thus gets deleted when RequiestBuilder resets the environment based on the cached results of this method. ErrorUtilities.VerifyThrowInternalNull(DebugUtils.ProcessInfoString, nameof(DebugUtils.DebugPath)); #endif - Dictionary table = new Dictionary(200, StringComparer.OrdinalIgnoreCase); // Razzle has 150 environment variables - unsafe { char* pEnvironmentBlock = null; @@ -332,6 +355,19 @@ internal static Dictionary GetEnvironmentVariables() } long stringBlockLength = pEnvironmentBlockEnd - pEnvironmentBlock; +#if !CLR2COMPATIBILITY + // Avoid allocating any objects if the environment still matches the last state. + // We speed this up by comparing the full block instead of individual key-value pairs. + ReadOnlySpan stringBlock = new(pEnvironmentBlock, (int)stringBlockLength); + EnvironmentState lastState = s_environmentState; + if (lastState?.EnvironmentBlock.Span.SequenceEqual(stringBlock) == true) + { + return lastState.EnvironmentVariables; + } +#endif + + Dictionary table = new(200, StringComparer.OrdinalIgnoreCase); // Razzle has 150 environment variables + // Copy strings out, parsing into pairs and inserting into the table. // The first few environment variable entries start with an '='! // The current working directory of every drive (except for those drives @@ -373,7 +409,12 @@ internal static Dictionary GetEnvironmentVariables() continue; } +#if !CLR2COMPATIBILITY + string key = Strings.WeakIntern(new ReadOnlySpan(pEnvironmentBlock + startKey, i - startKey)); +#else string key = new string(pEnvironmentBlock, startKey, i - startKey); +#endif + i++; // skip over '=' @@ -385,11 +426,25 @@ internal static Dictionary GetEnvironmentVariables() i++; } +#if !CLR2COMPATIBILITY + string value = Strings.WeakIntern(new ReadOnlySpan(pEnvironmentBlock + startValue, i - startValue)); +#else string value = new string(pEnvironmentBlock, startValue, i - startValue); +#endif // skip over 0 handled by for loop's i++ table[key] = value; } + +#if !CLR2COMPATIBILITY + // Update with the current state. + EnvironmentState currentState = + new(table.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase), stringBlock.ToArray()); + s_environmentState = currentState; + return currentState.EnvironmentVariables; +#else + return table; +#endif } finally { @@ -399,34 +454,78 @@ internal static Dictionary GetEnvironmentVariables() } } } - - return table; } -#else // NETFRAMEWORK - +#if NET /// /// Sets an environment variable using . /// internal static void SetEnvironmentVariable(string name, string value) => Environment.SetEnvironmentVariable(name, value); +#endif +#if !CLR2COMPATIBILITY /// - /// Returns key value pairs of environment variables in a new dictionary + /// Returns key value pairs of environment variables in a read-only dictionary /// with a case-insensitive key comparer. + /// + /// If the environment variables have not changed since the last time + /// this method was called, the same dictionary instance will be returned. /// - internal static Dictionary GetEnvironmentVariables() + internal static FrozenDictionary GetEnvironmentVariables() { - var vars = Environment.GetEnvironmentVariables(); + // Always call the native method on Windows, as we'll be able to avoid the internal + // string and Hashtable allocations caused by Environment.GetEnvironmentVariables(). + if (NativeMethodsShared.IsWindows) + { + return GetEnvironmentVariablesWindows(); + } + + IDictionary vars = Environment.GetEnvironmentVariables(); + + // Directly use the enumerator since Current will box DictionaryEntry. + IDictionaryEnumerator enumerator = vars.GetEnumerator(); - Dictionary table = new Dictionary(vars.Count, StringComparer.OrdinalIgnoreCase); - foreach (var key in vars.Keys) + // If every key-value pair matches the last state, return a cached dictionary. + FrozenDictionary lastEnvironmentVariables = s_environmentState?.EnvironmentVariables; + if (vars.Count == lastEnvironmentVariables?.Count) { - table[(string)key] = (string)vars[key]; + bool sameState = true; + + while (enumerator.MoveNext() && sameState) + { + DictionaryEntry entry = enumerator.Entry; + if (!lastEnvironmentVariables.TryGetValue((string)entry.Key, out string value) + || !string.Equals((string)entry.Value, value, StringComparison.Ordinal)) + { + sameState = false; + } + } + + if (sameState) + { + return lastEnvironmentVariables; + } } - return table; + + // Otherwise, allocate and update with the current state. + Dictionary table = new(vars.Count, StringComparer.OrdinalIgnoreCase); + + enumerator.Reset(); + while (enumerator.MoveNext()) + { + DictionaryEntry entry = enumerator.Entry; + string key = Strings.WeakIntern((string)entry.Key); + string value = Strings.WeakIntern((string)entry.Value); + table[key] = value; + } + + EnvironmentState newState = new(table.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase)); + s_environmentState = newState; + + return newState.EnvironmentVariables; } -#endif // NETFRAMEWORK +#endif /// /// Updates the environment to match the provided dictionary. @@ -436,7 +535,7 @@ internal static void SetEnvironment(IDictionary newEnvironment) if (newEnvironment != null) { // First, delete all no longer set variables - Dictionary currentEnvironment = GetEnvironmentVariables(); + IDictionary currentEnvironment = GetEnvironmentVariables(); foreach (KeyValuePair entry in currentEnvironment) { if (!newEnvironment.ContainsKey(entry.Key)) diff --git a/src/Shared/TranslatorHelpers.cs b/src/Shared/TranslatorHelpers.cs index b3c6a5573bd..8694a95611f 100644 --- a/src/Shared/TranslatorHelpers.cs +++ b/src/Shared/TranslatorHelpers.cs @@ -2,6 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +#if !TASKHOST +using System.Collections.Frozen; +#endif using System.Collections.Generic; using System.Configuration.Assemblies; using System.Globalization; @@ -172,6 +175,22 @@ public static void TranslateDictionary( translator.TranslateDictionary(ref dictionary, AdaptFactory(valueFactory), collectionCreator); } +#if !TASKHOST + public static void TranslateDictionary( + this ITranslator translator, + ref FrozenDictionary dictionary, + IEqualityComparer comparer) + { + IDictionary localDict = dictionary; + translator.TranslateDictionary(ref localDict, capacity => new Dictionary(capacity, comparer)); + + if (translator.Mode == TranslationDirection.ReadFromStream) + { + dictionary = localDict?.ToFrozenDictionary(comparer); + } + } +#endif + public static void TranslateHashSet( this ITranslator translator, ref HashSet hashSet,