Skip to content

Commit

Permalink
[graphics] fix CA1307 and CA1309 for performance (#14627)
Browse files Browse the repository at this point in the history
Context: #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 c40c6e7, I added code analysis rules to the `.editorconfig`, but
it appears we ignored these warnings in dbaeee9.

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`.
  • Loading branch information
jonathanpeppers authored Apr 18, 2023
1 parent 3269fd0 commit 8a58e01
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/Core/src/Graphics/MauiDrawable.Android.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
{
Expand Down
2 changes: 1 addition & 1 deletion src/Graphics/src/Graphics/Font.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 7 additions & 3 deletions src/Graphics/src/Graphics/FontSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/Graphics/src/Graphics/Graphics.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<Product>Microsoft.Maui.Graphics</Product>
<IsTrimmable>false</IsTrimmable>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<NoWarn>$(NoWarn);CA1307;CA1309;CS1591;RS0026;RS0027;RS0041</NoWarn>
<NoWarn>$(NoWarn);CS1591;RS0026;RS0027;RS0041</NoWarn>
</PropertyGroup>

<Import Project="$(MauiSrcDirectory)MultiTargeting.targets" />
Expand Down
12 changes: 11 additions & 1 deletion src/Graphics/src/Graphics/PathBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ private bool NextBoolValue
{
string vValueAsString = _commandStack.Pop();

if ("1".Equals(vValueAsString))
if ("1".Equals(vValueAsString, StringComparison.Ordinal))
{
return true;
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Graphics/src/Graphics/PdfExportContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/Graphics/src/Graphics/Text/XmlAttributedTextReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/Graphics/src/Graphics/Text/XmlAttributedTextWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}

Expand Down

0 comments on commit 8a58e01

Please sign in to comment.