From 8a58e01f3b4cc3d1908638665c4ae1783072d597 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 18 Apr 2023 02:45:16 -0500 Subject: [PATCH] [graphics] fix CA1307 and CA1309 for performance (#14627) Context: https://github.com/dotnet/maui/issues/12130 Context: https://github.com/angelru/CvSlowJittering While reviewing the above sample, I saw time spent doing culture-aware string "stuff" in Microsoft.Maui.Graphics: 77.22ms microsoft.maui!Microsoft.Maui.Graphics.MauiDrawable.SetDefaultBackgroundColor() 42.55ms System.Private.CoreLib!System.String.ToLower() In c40c6e77, I added code analysis rules to the `.editorconfig`, but it appears we ignored these warnings in dbaeee99. These are generally easy to fix, we should have just addressed these instead of adding `$(NoWarn)`. In the future, I will look into adding `CA1311`: https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1311 This would find `string.ToLower()` calls and recommend `string.ToLowerInvariant()` instead. I fixed the one place I saw it affecting performance in `dotnet-trace`. --- src/Core/src/Graphics/MauiDrawable.Android.cs | 2 +- src/Graphics/src/Graphics/Font.cs | 2 +- src/Graphics/src/Graphics/FontSource.cs | 10 +++++++--- src/Graphics/src/Graphics/Graphics.csproj | 2 +- src/Graphics/src/Graphics/PathBuilder.cs | 12 +++++++++++- src/Graphics/src/Graphics/PdfExportContext.cs | 2 +- .../MaciOS/Text/NSAttributedStringExtension.cs | 7 ++++--- .../src/Graphics/Text/XmlAttributedTextReader.cs | 8 ++++---- .../src/Graphics/Text/XmlAttributedTextWriter.cs | 4 ++-- 9 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/Core/src/Graphics/MauiDrawable.Android.cs b/src/Core/src/Graphics/MauiDrawable.Android.cs index 82bbb72145ff..499a450abbc2 100644 --- a/src/Core/src/Graphics/MauiDrawable.Android.cs +++ b/src/Core/src/Graphics/MauiDrawable.Android.cs @@ -520,7 +520,7 @@ void SetDefaultBackgroundColor() if (_context.Theme.ResolveAttribute(global::Android.Resource.Attribute.WindowBackground, background, true)) { var resource = _context.Resources.GetResourceTypeName(background.ResourceId); - var type = resource?.ToLower(); + var type = resource?.ToLowerInvariant(); if (type == "color") { diff --git a/src/Graphics/src/Graphics/Font.cs b/src/Graphics/src/Graphics/Font.cs index 9a31558bf6a0..d5ec5dabe798 100644 --- a/src/Graphics/src/Graphics/Font.cs +++ b/src/Graphics/src/Graphics/Font.cs @@ -45,7 +45,7 @@ public Font(string name, int weight = FontWeights.Normal, FontStyleType styleTyp public FontStyleType StyleType { get; private set; } public bool Equals(IFont other) - => StyleType == other.StyleType && Weight == other.Weight && Name.Equals(other.Name); + => StyleType == other.StyleType && Weight == other.Weight && Name.Equals(other.Name, StringComparison.Ordinal); public override bool Equals(object obj) => obj is IFont font && Equals(font); diff --git a/src/Graphics/src/Graphics/FontSource.cs b/src/Graphics/src/Graphics/FontSource.cs index 912d25484394..24d0eca66258 100644 --- a/src/Graphics/src/Graphics/FontSource.cs +++ b/src/Graphics/src/Graphics/FontSource.cs @@ -17,12 +17,16 @@ public FontSource(string filename, int weight = FontWeights.Normal, FontStyleTyp public readonly FontStyleType FontStyleType; public bool Equals(FontSource other) - => Name.Equals(other.Name) + => Name.Equals(other.Name, StringComparison.Ordinal) && Weight.Equals(other.Weight) && FontStyleType.Equals(other.FontStyleType); - public override int GetHashCode() - => Name.GetHashCode() ^ Weight.GetHashCode() ^ FontStyleType.GetHashCode(); + public override int GetHashCode() => Name.GetHashCode( +#if !NETSTANDARD2_0 + StringComparison.Ordinal +#endif + ) + ^ Weight.GetHashCode() ^ FontStyleType.GetHashCode(); public override bool Equals(object? obj) => obj is FontSource other && Equals(other); diff --git a/src/Graphics/src/Graphics/Graphics.csproj b/src/Graphics/src/Graphics/Graphics.csproj index 70f3e6376e51..08e1041bea10 100644 --- a/src/Graphics/src/Graphics/Graphics.csproj +++ b/src/Graphics/src/Graphics/Graphics.csproj @@ -13,7 +13,7 @@ Microsoft.Maui.Graphics false true - $(NoWarn);CA1307;CA1309;CS1591;RS0026;RS0027;RS0041 + $(NoWarn);CS1591;RS0026;RS0027;RS0041 diff --git a/src/Graphics/src/Graphics/PathBuilder.cs b/src/Graphics/src/Graphics/PathBuilder.cs index 241cb89697dd..d2f80c86c6ac 100644 --- a/src/Graphics/src/Graphics/PathBuilder.cs +++ b/src/Graphics/src/Graphics/PathBuilder.cs @@ -33,7 +33,7 @@ private bool NextBoolValue { string vValueAsString = _commandStack.Pop(); - if ("1".Equals(vValueAsString)) + if ("1".Equals(vValueAsString, StringComparison.Ordinal)) { return true; } @@ -112,11 +112,21 @@ public PathF BuildPath(string pathAsString) #if DEBUG_PATH System.Diagnostics.Debug.WriteLine(aPathString); #endif +#if NETSTANDARD2_0 pathAsString = pathAsString.Replace("Infinity", "0"); +#else + pathAsString = pathAsString.Replace("Infinity", "0", StringComparison.Ordinal); +#endif pathAsString = Regex.Replace(pathAsString, "([a-zA-Z])", " $1 "); +#if NETSTANDARD2_0 pathAsString = pathAsString.Replace("-", " -"); pathAsString = pathAsString.Replace(" E -", "E-"); pathAsString = pathAsString.Replace(" e -", "e-"); +#else + pathAsString = pathAsString.Replace("-", " -", StringComparison.Ordinal); + pathAsString = pathAsString.Replace(" E -", "E-", StringComparison.Ordinal); + pathAsString = pathAsString.Replace(" e -", "e-", StringComparison.Ordinal); +#endif #if DEBUG_PATH System.Diagnostics.Debug.WriteLine(aPathString); #endif diff --git a/src/Graphics/src/Graphics/PdfExportContext.cs b/src/Graphics/src/Graphics/PdfExportContext.cs index 360c7cb67bfa..ad0bc6d92d3c 100644 --- a/src/Graphics/src/Graphics/PdfExportContext.cs +++ b/src/Graphics/src/Graphics/PdfExportContext.cs @@ -19,7 +19,7 @@ protected PdfExportContext( { if (defaultWidth <= 0 || defaultHeight <= 0) { - if ("en-US".Equals(Thread.CurrentThread.CurrentCulture.Name)) + if ("en-US".Equals(Thread.CurrentThread.CurrentCulture.Name, StringComparison.Ordinal)) { // Letter defaultWidth = 612; diff --git a/src/Graphics/src/Graphics/Platforms/MaciOS/Text/NSAttributedStringExtension.cs b/src/Graphics/src/Graphics/Platforms/MaciOS/Text/NSAttributedStringExtension.cs index f0fc45e741cb..d94b36c42f82 100644 --- a/src/Graphics/src/Graphics/Platforms/MaciOS/Text/NSAttributedStringExtension.cs +++ b/src/Graphics/src/Graphics/Platforms/MaciOS/Text/NSAttributedStringExtension.cs @@ -5,9 +5,10 @@ using NSFont = UIKit.UIFont; using NSColor = UIKit.UIColor; #endif +using System; using System.Collections.Generic; -using Microsoft.Maui.Graphics.Text; using System.IO; +using Microsoft.Maui.Graphics.Text; using Foundation; namespace Microsoft.Maui.Graphics.Platform @@ -67,10 +68,10 @@ private static bool HandleAttributes( formatAttributes.SetFontName(fontName); else { - if (fontName.Contains("Italic")) + if (fontName.Contains("Italic", StringComparison.Ordinal)) formatAttributes.SetItalic(true); - if (fontName.Contains("Bold")) + if (fontName.Contains("Bold", StringComparison.Ordinal)) formatAttributes.SetBold(true); } } diff --git a/src/Graphics/src/Graphics/Text/XmlAttributedTextReader.cs b/src/Graphics/src/Graphics/Text/XmlAttributedTextReader.cs index 6fd8c76f1953..4b6a55856ec3 100644 --- a/src/Graphics/src/Graphics/Text/XmlAttributedTextReader.cs +++ b/src/Graphics/src/Graphics/Text/XmlAttributedTextReader.cs @@ -71,12 +71,12 @@ protected void ElementStarted() { string elementName = _reader.Name; - if (XmlNames.Content.Equals(elementName)) + if (XmlNames.Content.Equals(elementName, StringComparison.Ordinal)) { _inContent = true; _contentEncoded = ReadBool(XmlNames.Encoded); } - else if (XmlNames.Run.Equals(elementName)) + else if (XmlNames.Run.Equals(elementName, StringComparison.Ordinal)) { ReadRun(); } @@ -85,7 +85,7 @@ protected void ElementStarted() protected void ElementEnded() { string elementName = _reader.Name; - if (XmlNames.Content.Equals(elementName)) + if (XmlNames.Content.Equals(elementName, StringComparison.Ordinal)) _inContent = false; } @@ -121,7 +121,7 @@ private void ReadRun() var attributeName = _reader.Name; var attributeValue = _reader.Value; - if (!(XmlNames.Start.Equals(attributeName) || XmlNames.Length.Equals(attributeName))) + if (!(XmlNames.Start.Equals(attributeName, StringComparison.Ordinal) || XmlNames.Length.Equals(attributeName, StringComparison.Ordinal))) { if (Enum.TryParse(attributeName, out TextAttribute key)) attributes[key] = attributeValue; diff --git a/src/Graphics/src/Graphics/Text/XmlAttributedTextWriter.cs b/src/Graphics/src/Graphics/Text/XmlAttributedTextWriter.cs index b6109d359b64..d5e76d905234 100644 --- a/src/Graphics/src/Graphics/Text/XmlAttributedTextWriter.cs +++ b/src/Graphics/src/Graphics/Text/XmlAttributedTextWriter.cs @@ -23,7 +23,7 @@ public void Write( { if (attributedText != null && !string.IsNullOrEmpty(attributedText.Text)) { - bool encode = attributedText.Text.Contains("]]"); + bool encode = attributedText.Text.IndexOf("]]", StringComparison.Ordinal) != -1; writer.Write($"<{XmlNames.AttributedText}>"); if (encode) @@ -83,7 +83,7 @@ private void Write( { currentAttributes.TryGetValue(key, out var value); - if (!string.Equals(value, defaultValue)) + if (!string.Equals(value, defaultValue, StringComparison.Ordinal)) WriteAttribute(writer, key.ToString(), value); }