From b8aaabfa493bfa087a916a09229d266f9ace96b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Mon, 14 Oct 2024 10:21:12 +0200 Subject: [PATCH] perf fixes copy from dotutils and MSBuildStructuredLog (#10792) * adopt stream changes from dotutils * adopt BuildEventArgsReader changes from BinlogViewer * add ResourceUtilities methods without params, refactor * refactor conditional compilation to debug method attribute --- .../BinaryLogger/BuildEventArgsReader.cs | 22 +- .../Postprocessing/StreamExtensions.cs | 24 +- .../BinaryLogger/Postprocessing/SubStream.cs | 49 +++- .../Postprocessing/TransparentReadStream.cs | 62 ++++ src/Shared/ResourceUtilities.cs | 271 +++++++++++++++--- 5 files changed, 364 insertions(+), 64 deletions(-) diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs index 395263d6a53..9fd2cd14812 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs @@ -243,10 +243,11 @@ e is FormatException || (e is EndOfStreamException && _readStream.BytesCountAllowedToReadRemaining <= 0)) { hasError = true; - + int localSerializedEventLength = serializedEventLength; + Exception localException = e; string ErrorFactory() => ResourceUtilities.FormatResourceStringStripCodeAndKeyword("Binlog_ReaderMismatchedRead", - _recordNumber, serializedEventLength, e.GetType(), e.Message) + (_skipUnknownEvents + _recordNumber, localSerializedEventLength, localException.GetType(), localException.Message) + (_skipUnknownEvents ? " " + ResourceUtilities.GetResourceString("Binlog_ReaderSkippingRecord") : string.Empty); @@ -255,9 +256,11 @@ string ErrorFactory() => if (result == null && !hasError) { + int localSerializedEventLength = serializedEventLength; + BinaryLogRecordKind localRecordKind = recordKind; string ErrorFactory() => ResourceUtilities.FormatResourceStringStripCodeAndKeyword("Binlog_ReaderUnknownType", - _recordNumber, serializedEventLength, recordKind) + (_skipUnknownEvents + _recordNumber, localSerializedEventLength, localRecordKind) + (_skipUnknownEvents ? " " + ResourceUtilities.GetResourceString("Binlog_ReaderSkippingRecord") : string.Empty); @@ -266,9 +269,10 @@ string ErrorFactory() => if (_readStream.BytesCountAllowedToReadRemaining > 0) { + int localSerializedEventLength = serializedEventLength; string ErrorFactory() => ResourceUtilities.FormatResourceStringStripCodeAndKeyword( - "Binlog_ReaderUnderRead", _recordNumber, serializedEventLength, - serializedEventLength - _readStream.BytesCountAllowedToReadRemaining); + "Binlog_ReaderUnderRead", _recordNumber, localSerializedEventLength, + localSerializedEventLength - _readStream.BytesCountAllowedToReadRemaining); HandleError(ErrorFactory, _skipUnknownEventParts, ReaderErrorType.UnknownEventData, recordKind); } @@ -1437,9 +1441,9 @@ private void SetCommonFields(BuildEventArgs buildEventArgs, BuildEventArgsFields } } - private IEnumerable? ReadPropertyList() + private IList? ReadPropertyList() { - var properties = ReadStringDictionary(); + IDictionary? properties = ReadStringDictionary(); if (properties == null || properties.Count == 0) { return null; @@ -1530,7 +1534,7 @@ private ITaskItem ReadTaskItem() return taskItem; } - private IEnumerable? ReadProjectItems() + private IList? ReadProjectItems() { IList? list; @@ -1612,7 +1616,7 @@ private ITaskItem ReadTaskItem() return list; } - private IEnumerable? ReadTaskItemList() + private IList? ReadTaskItemList() { int count = ReadInt32(); if (count == 0) diff --git a/src/Build/Logging/BinaryLogger/Postprocessing/StreamExtensions.cs b/src/Build/Logging/BinaryLogger/Postprocessing/StreamExtensions.cs index d8eca6c3848..8a0cc2ed489 100644 --- a/src/Build/Logging/BinaryLogger/Postprocessing/StreamExtensions.cs +++ b/src/Build/Logging/BinaryLogger/Postprocessing/StreamExtensions.cs @@ -59,30 +59,28 @@ public static long SkipBytes(this Stream stream, long bytesCount, byte[] buffer) public static byte[] ReadToEnd(this Stream stream) { - if (stream.TryGetLength(out long length)) - { - using BinaryReader reader = new(stream, Encoding.UTF8, leaveOpen: true); - - return reader.ReadBytes((int)length); - } - - using var ms = new MemoryStream(); + MemoryStream ms = stream.TryGetLength(out long length) && length <= int.MaxValue ? new((int)length) : new(); stream.CopyTo(ms); - return ms.ToArray(); + byte[] buffer = ms.GetBuffer(); + return buffer.Length == ms.Length ? buffer : ms.ToArray(); } public static bool TryGetLength(this Stream stream, out long length) { try { - length = stream.Length; - return true; + if (stream.CanSeek) + { + length = stream.Length; + return true; + } } catch (NotSupportedException) { - length = 0; - return false; } + + length = 0; + return false; } public static Stream ToReadableSeekableStream(this Stream stream) diff --git a/src/Build/Logging/BinaryLogger/Postprocessing/SubStream.cs b/src/Build/Logging/BinaryLogger/Postprocessing/SubStream.cs index ff5caf9bfdf..8c6e0c6e2b8 100644 --- a/src/Build/Logging/BinaryLogger/Postprocessing/SubStream.cs +++ b/src/Build/Logging/BinaryLogger/Postprocessing/SubStream.cs @@ -3,6 +3,8 @@ using System; using System.IO; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Build.Shared; namespace Microsoft.Build.Logging @@ -40,7 +42,8 @@ public SubStream(Stream stream, long length) public override long Position { get => _position; set => throw new NotImplementedException(); } - public override void Flush() { } + public override void Flush() => _stream.Flush(); + public override Task FlushAsync(CancellationToken cancellationToken) => _stream.FlushAsync(cancellationToken); public override int Read(byte[] buffer, int offset, int count) { count = Math.Min((int)Math.Max(Length - _position, 0), count); @@ -48,6 +51,50 @@ public override int Read(byte[] buffer, int offset, int count) _position += read; return read; } + + public override int ReadByte() + { + if (Length - _position > 0) + { + int value = _stream.ReadByte(); + if (value >= 0) + { + _position++; + return value; + } + } + + return -1; + } + + public override async Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + count = Math.Min((int)Math.Max(Length - _position, 0), count); +#pragma warning disable CA1835 // Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync' + int read = await _stream.ReadAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false); +#pragma warning restore CA1835 // Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync' + _position += read; + return read; + } + +#if NET + public override int Read(Span buffer) + { + buffer = buffer.Slice(0, Math.Min((int)Math.Max(Length - _position, 0), buffer.Length)); + int read = _stream.Read(buffer); + _position += read; + return read; + } + + public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + buffer = buffer.Slice(0, Math.Min((int)Math.Max(Length - _position, 0), buffer.Length)); + int read = await _stream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false); + _position += read; + return read; + } +#endif + public override long Seek(long offset, SeekOrigin origin) => throw new NotImplementedException(); public override void SetLength(long value) => throw new NotImplementedException(); public override void Write(byte[] buffer, int offset, int count) => throw new NotImplementedException(); diff --git a/src/Build/Logging/BinaryLogger/Postprocessing/TransparentReadStream.cs b/src/Build/Logging/BinaryLogger/Postprocessing/TransparentReadStream.cs index 4dd9afa0300..ea3fcb3c9c7 100644 --- a/src/Build/Logging/BinaryLogger/Postprocessing/TransparentReadStream.cs +++ b/src/Build/Logging/BinaryLogger/Postprocessing/TransparentReadStream.cs @@ -3,6 +3,8 @@ using System; using System.IO; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Build.Shared; namespace Microsoft.Build.Logging @@ -75,6 +77,11 @@ public override void Flush() _stream.Flush(); } + public override Task FlushAsync(CancellationToken cancellationToken) + { + return _stream.FlushAsync(cancellationToken); + } + public override int Read(byte[] buffer, int offset, int count) { if (_position + count > _maxAllowedPosition) @@ -87,6 +94,61 @@ public override int Read(byte[] buffer, int offset, int count) return cnt; } + public override int ReadByte() + { + if (_position + 1 <= _maxAllowedPosition) + { + int value = _stream.ReadByte(); + if (value >= 0) + { + _position++; + return value; + } + } + + return -1; + } + + public override async Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + if (_position + count > _maxAllowedPosition) + { + count = (int)(_maxAllowedPosition - _position); + } + +#pragma warning disable CA1835 // Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync' + int cnt = await _stream.ReadAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false); +#pragma warning restore CA1835 // Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync' + _position += cnt; + return cnt; + } + +#if NET + public override int Read(Span buffer) + { + if (_position + buffer.Length > _maxAllowedPosition) + { + buffer = buffer.Slice(0, (int)(_maxAllowedPosition - _position)); + } + + int cnt = _stream.Read(buffer); + _position += cnt; + return cnt; + } + + public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { + if (_position + buffer.Length > _maxAllowedPosition) + { + buffer = buffer.Slice(0, (int)(_maxAllowedPosition - _position)); + } + + int cnt = await _stream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false); + _position += cnt; + return cnt; + } +#endif + public override long Seek(long offset, SeekOrigin origin) { if (origin != SeekOrigin.Current) diff --git a/src/Shared/ResourceUtilities.cs b/src/Shared/ResourceUtilities.cs index deaf884e4d5..58e02e85616 100644 --- a/src/Shared/ResourceUtilities.cs +++ b/src/Shared/ResourceUtilities.cs @@ -135,9 +135,7 @@ internal static string ExtractMessageCode(bool msbuildCodeOnly, string message, /// Resource string to get the MSBuild F1-keyword for. /// The MSBuild F1-help keyword string. private static string GetHelpKeyword(string resourceName) - { - return "MSBuild." + resourceName; - } + => "MSBuild." + resourceName; #if !BUILDINGAPPXTASKS /// @@ -146,17 +144,14 @@ private static string GetHelpKeyword(string resourceName) /// Resource string name. /// Resource string contents. internal static string GetResourceString(string resourceName) - { - string result = AssemblyResources.GetString(resourceName); - return result; - } + => AssemblyResources.GetString(resourceName); /// /// Loads the specified string resource and formats it with the arguments passed in. If the string resource has an MSBuild /// message code and help keyword associated with it, they too are returned. /// /// PERF WARNING: calling a method that takes a variable number of arguments is expensive, because memory is allocated for - /// the array of arguments -- do not call this method repeatedly in performance-critical scenarios + /// the array of arguments -- do not call this method repeatedly in performance-critical scenarios. /// /// This method is thread-safe. /// [out] The MSBuild message code, or null. @@ -172,6 +167,68 @@ internal static string FormatResourceStringStripCodeAndKeyword(out string code, return ExtractMessageCode(true /* msbuildCodeOnly */, FormatString(GetResourceString(resourceName), args), out code); } + // Overloads with 0-3 arguments to avoid array allocations. + + /// + /// Loads the specified string resource and formats it with the arguments passed in. If the string resource has an MSBuild + /// message code and help keyword associated with it, they too are returned. + /// + /// This method is thread-safe. + /// [out] The MSBuild message code, or null. + /// [out] The MSBuild F1-help keyword for the host IDE, or null. + /// Resource string to load. + /// The formatted resource string. + internal static string FormatResourceStringStripCodeAndKeyword(out string code, out string helpKeyword, string resourceName) + { + helpKeyword = GetHelpKeyword(resourceName); + return ExtractMessageCode(true, GetResourceString(resourceName), out code); + } + + /// + /// Loads the specified string resource and formats it with the arguments passed in. If the string resource has an MSBuild + /// message code and help keyword associated with it, they too are returned. + /// + /// [out] The MSBuild message code, or null. + /// [out] The MSBuild F1-help keyword for the host IDE, or null. + /// Resource string to load. + /// Argument for formatting the resource string. + internal static string FormatResourceStringStripCodeAndKeyword(out string code, out string helpKeyword, string resourceName, object arg1) + { + helpKeyword = GetHelpKeyword(resourceName); + return ExtractMessageCode(true, FormatString(GetResourceString(resourceName), arg1), out code); + } + + /// + /// Loads the specified string resource and formats it with the arguments passed in. If the string resource has an MSBuild + /// message code and help keyword associated with it, they too are returned. + /// + /// [out] The MSBuild message code, or null. + /// [out] The MSBuild F1-help keyword for the host IDE, or null. + /// Resource string to load. + /// First argument for formatting the resource string. + /// Second argument for formatting the resource string. + internal static string FormatResourceStringStripCodeAndKeyword(out string code, out string helpKeyword, string resourceName, object arg1, object arg2) + { + helpKeyword = GetHelpKeyword(resourceName); + return ExtractMessageCode(true, FormatString(GetResourceString(resourceName), arg1, arg2), out code); + } + + /// + /// Loads the specified string resource and formats it with the arguments passed in. If the string resource has an MSBuild + /// message code and help keyword associated with it, they too are returned. + /// + /// [out] The MSBuild message code, or null. + /// [out] The MSBuild F1-help keyword for the host IDE, or null. + /// Resource string to load. + /// First argument for formatting the resource string. + /// Second argument for formatting the resource string. + /// Third argument for formatting the resource string. + internal static string FormatResourceStringStripCodeAndKeyword(out string code, out string helpKeyword, string resourceName, object arg1, object arg2, object arg3) + { + helpKeyword = GetHelpKeyword(resourceName); + return ExtractMessageCode(true, FormatString(GetResourceString(resourceName), arg1, arg2, arg3), out code); + } + [Obsolete("Use GetResourceString instead.", true)] [EditorBrowsable(EditorBrowsableState.Never)] internal static string FormatResourceString(string resourceName) @@ -184,32 +241,117 @@ internal static string FormatResourceString(string resourceName) /// message code and help keyword associated with it, they are discarded. /// /// PERF WARNING: calling a method that takes a variable number of arguments is expensive, because memory is allocated for - /// the array of arguments -- do not call this method repeatedly in performance-critical scenarios + /// the array of arguments -- do not call this method repeatedly in performance-critical scenarios. /// /// This method is thread-safe. /// Resource string to load. /// Optional arguments for formatting the resource string. /// The formatted resource string. internal static string FormatResourceStringStripCodeAndKeyword(string resourceName, params object[] args) - { - string code; - string helpKeyword; + => FormatResourceStringStripCodeAndKeyword(out _, out _, resourceName, args); - return FormatResourceStringStripCodeAndKeyword(out code, out helpKeyword, resourceName, args); - } + // Overloads with 0-3 arguments to avoid array allocations. + + /// + /// Looks up a string in the resources. If the string resource has an MSBuild + /// message code and help keyword associated with it, they are discarded. + /// + /// This method is thread-safe. + /// Resource string to load. + /// The formatted resource string. + internal static string FormatResourceStringStripCodeAndKeyword(string resourceName) + => FormatResourceStringStripCodeAndKeyword(out _, out _, resourceName); + + /// + /// Looks up a string in the resources, and formats it with the argument passed in. If the string resource has an MSBuild + /// message code and help keyword associated with it, they are discarded. + /// + /// This method is thread-safe. + /// Resource string to load. + /// Argument for formatting the resource string. + /// The formatted resource string. + internal static string FormatResourceStringStripCodeAndKeyword(string resourceName, object arg1) + => FormatResourceStringStripCodeAndKeyword(out _, out _, resourceName, arg1); + + /// + /// Looks up a string in the resources, and formats it with the arguments passed in. If the string resource has an MSBuild + /// message code and help keyword associated with it, they are discarded. + /// + /// This method is thread-safe. + /// Resource string to load. + /// First argument for formatting the resource string. + /// Second argument for formatting the resource string. + /// The formatted resource string. + internal static string FormatResourceStringStripCodeAndKeyword(string resourceName, object arg1, object arg2) + => FormatResourceStringStripCodeAndKeyword(out _, out _, resourceName, arg1, arg2); + + /// + /// Looks up a string in the resources, and formats it with the arguments passed in. If the string resource has an MSBuild + /// message code and help keyword associated with it, they are discarded. + /// + /// This method is thread-safe. + /// Resource string to load. + /// First argument for formatting the resource string. + /// Second argument for formatting the resource string. + /// Third argument for formatting the resource string. + /// The formatted resource string. + internal static string FormatResourceStringStripCodeAndKeyword(string resourceName, object arg1, object arg2, object arg3) + => FormatResourceStringStripCodeAndKeyword(out _, out _, resourceName, arg1, arg2, arg3); /// /// Formats the resource string with the given arguments. - /// Ignores error codes and keywords + /// Ignores error codes and keywords. /// - /// - /// - /// + /// Resource string to load. + /// Optional arguments for formatting the resource string. + /// The formatted resource string. + /// the AssemblyResources.GetString() method is thread-safe. internal static string FormatResourceStringIgnoreCodeAndKeyword(string resourceName, params object[] args) - { - // NOTE: the AssemblyResources.GetString() method is thread-safe - return FormatString(GetResourceString(resourceName), args); - } + => FormatString(GetResourceString(resourceName), args); + + // Overloads with 0-3 arguments to avoid array allocations. + + /// + /// Formats the resource string. + /// Ignores error codes and keywords. + /// + /// Resource string to load. + /// The formatted resource string. + internal static string FormatResourceStringIgnoreCodeAndKeyword(string resourceName) + => GetResourceString(resourceName); + + /// + /// Formats the resource string with the given argument. + /// Ignores error codes and keywords. + /// + /// Resource string to load. + /// Argument for formatting the resource string. + /// The formatted resource string. + internal static string FormatResourceStringIgnoreCodeAndKeyword(string resourceName, object arg1) + => FormatString(GetResourceString(resourceName), arg1); + + /// + /// Formats the resource string with the given arguments. + /// Ignores error codes and keywords. + /// + /// Resource string to load. + /// First argument for formatting the resource string. + /// Second argument for formatting the resource string. + /// The formatted resource string. + internal static string FormatResourceStringIgnoreCodeAndKeyword(string resourceName, object arg1, object arg2) + => FormatString(GetResourceString(resourceName), arg1, arg2); + + /// + /// Formats the resource string with the given arguments. + /// Ignores error codes and keywords. + /// + /// Resource string to load. + /// First argument for formatting the resource string. + /// Second argument for formatting the resource string. + /// Third argument for formatting the resource string. + /// The formatted resource string. + internal static string FormatResourceStringIgnoreCodeAndKeyword(string resourceName, object arg1, object arg2, object arg3) + => FormatString(GetResourceString(resourceName), arg1, arg2, arg3); /// /// Formats the given string using the variable arguments passed in. @@ -227,33 +369,80 @@ internal static string FormatString(string unformatted, params object[] args) string formatted = unformatted; // NOTE: String.Format() does not allow a null arguments array - if ((args?.Length > 0)) + if (args?.Length > 0) { -#if DEBUG - // If you accidentally pass some random type in that can't be converted to a string, - // FormatResourceString calls ToString() which returns the full name of the type! - foreach (object param in args) - { - // Check it has a real implementation of ToString() and the type is not actually System.String - if (param != null) - { - if (string.Equals(param.GetType().ToString(), param.ToString(), StringComparison.Ordinal) && - param.GetType() != typeof(string)) - { - ErrorUtilities.ThrowInternalError("Invalid resource parameter type, was {0}", - param.GetType().FullName); - } - } - } -#endif + ValidateArgsIfDebug(args); + // Format the string, using the variable arguments passed in. // NOTE: all String methods are thread-safe - formatted = String.Format(CultureInfo.CurrentCulture, unformatted, args); + formatted = string.Format(CultureInfo.CurrentCulture, unformatted, args); } return formatted; } + // Overloads with 1-3 arguments to avoid array allocations. + + /// + /// Formats the given string using the variable arguments passed in. + /// + /// The string to format. + /// Argument for formatting the given string. + /// The formatted string. + internal static string FormatString(string unformatted, object arg1) + { + ValidateArgsIfDebug([arg1]); + return string.Format(CultureInfo.CurrentCulture, unformatted, arg1); + } + + /// + /// Formats the given string using the variable arguments passed in. + /// + /// The string to format. + /// First argument for formatting the given string. + /// Second argument for formatting the given string. + /// The formatted string. + internal static string FormatString(string unformatted, object arg1, object arg2) + { + ValidateArgsIfDebug([arg1, arg2]); + return string.Format(CultureInfo.CurrentCulture, unformatted, arg1, arg2); + } + + /// + /// Formats the given string using the variable arguments passed in. + /// + /// The string to format. + /// First argument for formatting the given string. + /// Second argument for formatting the given string. + /// Third argument for formatting the given string. + /// The formatted string. + internal static string FormatString(string unformatted, object arg1, object arg2, object arg3) + { + ValidateArgsIfDebug([arg1, arg2, arg3]); + return string.Format(CultureInfo.CurrentCulture, unformatted, arg1, arg2, arg3); + } + + [Conditional("DEBUG")] + private static void ValidateArgsIfDebug(object[] args) + { + // If you accidentally pass some random type in that can't be converted to a string, + // FormatResourceString calls ToString() which returns the full name of the type! + foreach (object param in args) + { + // Check it has a real implementation of ToString() and the type is not actually System.String + if (param != null) + { + if (string.Equals(param.GetType().ToString(), param.ToString(), StringComparison.Ordinal) && + param.GetType() != typeof(string)) + { + ErrorUtilities.ThrowInternalError( + "Invalid resource parameter type, was {0}", + param.GetType().FullName); + } + } + } + } + /// /// Verifies that a particular resource string actually exists in the string table. This will only be called in debug /// builds. It helps catch situations where a dev calls VerifyThrowXXX with a new resource string, but forgets to add the