From 378f87cc52c5cc41af33176f290d53ba84b89193 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Wed, 9 Oct 2024 22:10:56 +0200 Subject: [PATCH 1/6] Seal the class for performance benefit --- .../Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs index 091d63efb7a..729b0b409bd 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs @@ -18,7 +18,7 @@ namespace MS.Internal.Xaml.Parser /// Class that provides helper functions for the parser/Xaml Reader /// to process Bracket Characters specified on a Markup Extension Property /// - internal class SpecialBracketCharacters : ISupportInitialize + internal sealed class SpecialBracketCharacters : ISupportInitialize { private string _startChars; private string _endChars; From 35c95cd922e7234e1fbb2b3ff96375a505c7dab3 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Wed, 9 Oct 2024 22:42:53 +0200 Subject: [PATCH 2/6] Use SearchValues over SortedSet, remove conversions ToString --- .../Xaml/Parser/SpecialBracketCharacters.cs | 120 +++++++++++++++++- 1 file changed, 117 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs index 729b0b409bd..ca18549c4aa 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs @@ -7,10 +7,11 @@ // Description: Data model for the Bracket characters specified on a Markup Extension property. using System; -using System.Collections; -using System.Collections.Generic; -using System.ComponentModel; using System.Text; +using System.Buffers; +using System.Windows.Markup; +using System.ComponentModel; +using System.Collections.Generic; namespace MS.Internal.Xaml.Parser { @@ -20,6 +21,118 @@ namespace MS.Internal.Xaml.Parser /// internal sealed class SpecialBracketCharacters : ISupportInitialize { +#if !NETFX + /// + /// Stores characters that cannot be specified in . + /// + private static readonly SearchValues s_restrictedCharSet = SearchValues.Create('=', ',', '\'', '"', '{', '}', '\\'); + + private string _startChars; + private string _endChars; + private bool _initializing; + private StringBuilder _startCharactersStringBuilder; + private StringBuilder _endCharactersStringBuilder; + + internal SpecialBracketCharacters() + { + BeginInit(); + } + + internal SpecialBracketCharacters(IReadOnlyDictionary attributeList) + { + BeginInit(); + if (attributeList != null && attributeList.Count > 0) + { + Tokenize(attributeList); + } + } + + internal void AddBracketCharacters(char openingBracket, char closingBracket) + { + if (_initializing) + { + _startCharactersStringBuilder.Append(openingBracket); + _endCharactersStringBuilder.Append(closingBracket); + } + else + { + throw new InvalidOperationException(); + } + } + + private void Tokenize(IReadOnlyDictionary attributeList) + { + if (_initializing) + { + foreach (char openingBracket in attributeList.Keys) + { + char closingBracket = attributeList[openingBracket]; + + if (IsValidBracketCharacter(openingBracket, closingBracket)) + { + _startCharactersStringBuilder.Append(openingBracket); + _endCharactersStringBuilder.Append(closingBracket); + } + } + } + } + + private static bool IsValidBracketCharacter(char openingBracket, char closingBracket) + { + if (openingBracket == closingBracket) + throw new InvalidOperationException("Opening bracket character cannot be the same as closing bracket character."); + else if (char.IsLetterOrDigit(openingBracket) || char.IsLetterOrDigit(closingBracket) || char.IsWhiteSpace(openingBracket) || char.IsWhiteSpace(closingBracket)) + throw new InvalidOperationException("Bracket characters cannot be alpha-numeric or whitespace."); + else if (s_restrictedCharSet.Contains(openingBracket) || s_restrictedCharSet.Contains(closingBracket)) + throw new InvalidOperationException("Bracket characters cannot be one of the following: '=' , ',', '\'', '\"', '{ ', ' }', '\\'"); + + return true; + } + + internal bool IsSpecialCharacter(char ch) + { + return _startChars.Contains(ch) || _endChars.Contains(ch); + } + + internal bool StartsEscapeSequence(char ch) + { + return _startChars.Contains(ch); + } + + internal bool EndsEscapeSequence(char ch) + { + return _endChars.Contains(ch); + } + + internal bool Match(char start, char end) + { + return _endChars.IndexOf(end, StringComparison.Ordinal) == _startChars.IndexOf(start, StringComparison.Ordinal); + } + + internal string StartBracketCharacters + { + get { return _startChars; } + } + + internal string EndBracketCharacters + { + get { return _endChars; } + } + + public void BeginInit() + { + _initializing = true; + _startCharactersStringBuilder = new StringBuilder(); + _endCharactersStringBuilder = new StringBuilder(); + } + + public void EndInit() + { + _startChars = _startCharactersStringBuilder.ToString(); + _endChars = _endCharactersStringBuilder.ToString(); + _initializing = false; + } +#else private string _startChars; private string _endChars; private readonly static ISet _restrictedCharSet = new SortedSet((new char[] { '=', ',', '\'', '"', '{', '}', '\\' })); @@ -134,5 +247,6 @@ public void EndInit() _endChars = _endCharactersStringBuilder.ToString(); _initializing = false; } +#endif } } From f2424918420ff3e030e59cc0623d726922933560 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Wed, 9 Oct 2024 23:08:08 +0200 Subject: [PATCH 3/6] Properly implement ISupportInitialize; fix "memory leak" with unreleased StringBuilders --- .../Xaml/Parser/SpecialBracketCharacters.cs | 118 ++++++++++-------- 1 file changed, 66 insertions(+), 52 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs index ca18549c4aa..f9c11922cb5 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs @@ -27,9 +27,10 @@ internal sealed class SpecialBracketCharacters : ISupportInitialize /// private static readonly SearchValues s_restrictedCharSet = SearchValues.Create('=', ',', '\'', '"', '{', '}', '\\'); + private bool _initializing; + private string _startChars; private string _endChars; - private bool _initializing; private StringBuilder _startCharactersStringBuilder; private StringBuilder _endCharactersStringBuilder; @@ -41,7 +42,8 @@ internal SpecialBracketCharacters() internal SpecialBracketCharacters(IReadOnlyDictionary attributeList) { BeginInit(); - if (attributeList != null && attributeList.Count > 0) + + if (attributeList?.Count > 0) { Tokenize(attributeList); } @@ -49,30 +51,26 @@ internal SpecialBracketCharacters(IReadOnlyDictionary attributeList) internal void AddBracketCharacters(char openingBracket, char closingBracket) { - if (_initializing) - { - _startCharactersStringBuilder.Append(openingBracket); - _endCharactersStringBuilder.Append(closingBracket); - } - else - { + if (!_initializing) throw new InvalidOperationException(); - } + + _startCharactersStringBuilder.Append(openingBracket); + _endCharactersStringBuilder.Append(closingBracket); } private void Tokenize(IReadOnlyDictionary attributeList) { - if (_initializing) + if (!_initializing) + throw new InvalidOperationException(); + + foreach (char openingBracket in attributeList.Keys) { - foreach (char openingBracket in attributeList.Keys) - { - char closingBracket = attributeList[openingBracket]; + char closingBracket = attributeList[openingBracket]; - if (IsValidBracketCharacter(openingBracket, closingBracket)) - { - _startCharactersStringBuilder.Append(openingBracket); - _endCharactersStringBuilder.Append(closingBracket); - } + if (IsValidBracketCharacter(openingBracket, closingBracket)) + { + _startCharactersStringBuilder.Append(openingBracket); + _endCharactersStringBuilder.Append(closingBracket); } } } @@ -121,6 +119,9 @@ internal string EndBracketCharacters public void BeginInit() { + if (_initializing) + throw new InvalidOperationException(); + _initializing = true; _startCharactersStringBuilder = new StringBuilder(); _endCharactersStringBuilder = new StringBuilder(); @@ -128,15 +129,28 @@ public void BeginInit() public void EndInit() { + if (!_initializing) + throw new InvalidOperationException(); + _startChars = _startCharactersStringBuilder.ToString(); _endChars = _endCharactersStringBuilder.ToString(); + + // Clear the references, otherwise we could consider this a memory leak + _startCharactersStringBuilder = null; + _endCharactersStringBuilder = null; + _initializing = false; } #else + /// + /// Stores characters that cannot be specified in . + /// + private static readonly ISet s_restrictedCharSet = new SortedSet(new char[] { '=', ',', '\'', '"', '{', '}', '\\' }); + + private bool _initializing; + private string _startChars; private string _endChars; - private readonly static ISet _restrictedCharSet = new SortedSet((new char[] { '=', ',', '\'', '"', '{', '}', '\\' })); - private bool _initializing; private StringBuilder _startCharactersStringBuilder; private StringBuilder _endCharactersStringBuilder; @@ -145,10 +159,11 @@ internal SpecialBracketCharacters() BeginInit(); } - internal SpecialBracketCharacters(IReadOnlyDictionary attributeList) + internal SpecialBracketCharacters(IReadOnlyDictionary attributeList) { BeginInit(); - if (attributeList != null && attributeList.Count > 0) + + if (attributeList?.Count > 0) { Tokenize(attributeList); } @@ -156,30 +171,26 @@ internal SpecialBracketCharacters(IReadOnlyDictionary attributeList) internal void AddBracketCharacters(char openingBracket, char closingBracket) { - if (_initializing) - { - _startCharactersStringBuilder.Append(openingBracket); - _endCharactersStringBuilder.Append(closingBracket); - } - else - { + if (!_initializing) throw new InvalidOperationException(); - } + + _startCharactersStringBuilder.Append(openingBracket); + _endCharactersStringBuilder.Append(closingBracket); } - private void Tokenize(IReadOnlyDictionary attributeList) + private void Tokenize(IReadOnlyDictionary attributeList) { - if (_initializing) + if (!_initializing) + throw new InvalidOperationException(); + + foreach (char openingBracket in attributeList.Keys) { - foreach (char openingBracket in attributeList.Keys) + char closingBracket = attributeList[openingBracket]; + + if (IsValidBracketCharacter(openingBracket, closingBracket)) { - char closingBracket = attributeList[openingBracket]; - string errorMessage = string.Empty; - if (IsValidBracketCharacter(openingBracket, closingBracket)) - { - _startCharactersStringBuilder.Append(openingBracket); - _endCharactersStringBuilder.Append(closingBracket); - } + _startCharactersStringBuilder.Append(openingBracket); + _endCharactersStringBuilder.Append(closingBracket); } } } @@ -187,21 +198,13 @@ private void Tokenize(IReadOnlyDictionary attributeList) private bool IsValidBracketCharacter(char openingBracket, char closingBracket) { if (openingBracket == closingBracket) - { throw new InvalidOperationException("Opening bracket character cannot be the same as closing bracket character."); - } else if (char.IsLetterOrDigit(openingBracket) || char.IsLetterOrDigit(closingBracket) || char.IsWhiteSpace(openingBracket) || char.IsWhiteSpace(closingBracket)) - { throw new InvalidOperationException("Bracket characters cannot be alpha-numeric or whitespace."); - } - else if (_restrictedCharSet.Contains(openingBracket) || _restrictedCharSet.Contains(closingBracket)) - { + else if (s_restrictedCharSet.Contains(openingBracket) || s_restrictedCharSet.Contains(closingBracket)) throw new InvalidOperationException("Bracket characters cannot be one of the following: '=' , ',', '\'', '\"', '{ ', ' }', '\\'"); - } - else - { - return true; - } + + return true; } internal bool IsSpecialCharacter(char ch) @@ -236,6 +239,9 @@ internal string EndBracketCharacters public void BeginInit() { + if (_initializing) + throw new InvalidOperationException(); + _initializing = true; _startCharactersStringBuilder = new StringBuilder(); _endCharactersStringBuilder = new StringBuilder(); @@ -243,8 +249,16 @@ public void BeginInit() public void EndInit() { + if (!_initializing) + throw new InvalidOperationException(); + _startChars = _startCharactersStringBuilder.ToString(); _endChars = _endCharactersStringBuilder.ToString(); + + // Clear the references, otherwise we could consider this a memory leak + _startCharactersStringBuilder = null; + _endCharactersStringBuilder = null; + _initializing = false; } #endif From 4a5da8e938694eb520f466a155a954dd13478894 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Wed, 9 Oct 2024 23:08:45 +0200 Subject: [PATCH 4/6] Swap ISet to SortedSet for the NetFX implementation for additional perf --- .../Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs index f9c11922cb5..94d7977f279 100644 --- a/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs +++ b/src/Microsoft.DotNet.Wpf/src/Shared/MS/Internal/Xaml/Parser/SpecialBracketCharacters.cs @@ -145,7 +145,7 @@ public void EndInit() /// /// Stores characters that cannot be specified in . /// - private static readonly ISet s_restrictedCharSet = new SortedSet(new char[] { '=', ',', '\'', '"', '{', '}', '\\' }); + private static readonly SortedSet s_restrictedCharSet = new(new char[] { '=', ',', '\'', '"', '{', '}', '\\' }); private bool _initializing; From cacb24648d8af4931b3d673e498c4d915f52e454 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Wed, 9 Oct 2024 23:15:47 +0200 Subject: [PATCH 5/6] Save possibly a triple look-up on MasterBracketCharacterCache --- .../System/Windows/Markup/ParserContext.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Markup/ParserContext.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Markup/ParserContext.cs index 79e3d3358ef..7659848d3c7 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Markup/ParserContext.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Markup/ParserContext.cs @@ -198,13 +198,14 @@ internal ParserContext(ParserContext parserContext) /// internal Dictionary InitBracketCharacterCacheForType(Type type) { - if (!MasterBracketCharacterCache.ContainsKey(type)) + if (!MasterBracketCharacterCache.TryGetValue(type, out Dictionary map)) { - Dictionary map = BuildBracketCharacterCacheForType(type); + map = BuildBracketCharacterCacheForType(type); + MasterBracketCharacterCache.Add(type, map); } - return MasterBracketCharacterCache[type]; + return map; } /// @@ -846,9 +847,9 @@ internal Freezable TryGetFreezable(string value) /// Looks up all properties via reflection on the given type, and scans through the attributes on all of them /// to build a cache of properties which have MarkupExtensionBracketCharactersAttribute set on them. /// - private Dictionary BuildBracketCharacterCacheForType(Type extensionType) + private static Dictionary BuildBracketCharacterCacheForType(Type extensionType) { - Dictionary cache = new Dictionary(StringComparer.OrdinalIgnoreCase); + Dictionary cache = new(StringComparer.OrdinalIgnoreCase); PropertyInfo[] propertyInfo = extensionType.GetProperties(BindingFlags.Public | BindingFlags.Instance); Type constructorArgumentType = null; Type markupExtensionBracketCharacterType = null; @@ -859,6 +860,7 @@ private Dictionary BuildBracketCharacterCacheF string constructorArgumentName = null; IList customAttributes = CustomAttributeData.GetCustomAttributes(property); SpecialBracketCharacters bracketCharacters = null; + foreach (CustomAttributeData attributeData in customAttributes) { Type attributeType = attributeData.AttributeType; @@ -877,10 +879,7 @@ private Dictionary BuildBracketCharacterCacheF } else if (attributeType.IsAssignableFrom(markupExtensionBracketCharacterType)) { - if (bracketCharacters == null) - { - bracketCharacters = new SpecialBracketCharacters(); - } + bracketCharacters ??= new SpecialBracketCharacters(); bracketCharacters.AddBracketCharacters((char)attributeData.ConstructorArguments[0].Value, (char)attributeData.ConstructorArguments[1].Value); } From da45061cd8b3967f898061fcae128c57dd45f528 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Wed, 9 Oct 2024 23:26:26 +0200 Subject: [PATCH 6/6] Mark BuildBracketCharacterCacheForType as static, rewrite it to make more sense --- .../System/Xaml/XamlSchemaContext.cs | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/System.Xaml/System/Xaml/XamlSchemaContext.cs b/src/Microsoft.DotNet.Wpf/src/System.Xaml/System/Xaml/XamlSchemaContext.cs index 9f27c75a174..73a86e09858 100644 --- a/src/Microsoft.DotNet.Wpf/src/System.Xaml/System/Xaml/XamlSchemaContext.cs +++ b/src/Microsoft.DotNet.Wpf/src/System.Xaml/System/Xaml/XamlSchemaContext.cs @@ -564,25 +564,22 @@ internal Dictionary InitBracketCharacterCacheF /// Looks up all properties via reflection on the given type, and scans through the attributes on all of them /// to build a cache of properties which have MarkupExtensionBracketCharactersAttribute set on them. /// - private Dictionary BuildBracketCharacterCacheForType(XamlType type) + private static Dictionary BuildBracketCharacterCacheForType(XamlType type) { - Dictionary map = new Dictionary(StringComparer.OrdinalIgnoreCase); - ICollection members = type.GetAllMembers(); - foreach (XamlMember member in members) - { - string constructorArgumentName = member.ConstructorArgument; - string propertyName = member.Name; - IReadOnlyDictionary markupExtensionBracketCharactersList = member.MarkupExtensionBracketCharacters; - SpecialBracketCharacters splBracketCharacters = markupExtensionBracketCharactersList != null && markupExtensionBracketCharactersList.Count > 0 - ? new SpecialBracketCharacters(markupExtensionBracketCharactersList) - : null; - if (splBracketCharacters != null) + Dictionary map = new(StringComparer.OrdinalIgnoreCase); + + foreach (XamlMember member in type.GetAllMembers()) + { + if (member.MarkupExtensionBracketCharacters?.Count > 0) { - splBracketCharacters.EndInit(); - map.Add(propertyName, splBracketCharacters); - if (!string.IsNullOrEmpty(constructorArgumentName)) + SpecialBracketCharacters specialBracketChars = new(member.MarkupExtensionBracketCharacters); + specialBracketChars.EndInit(); + + map.Add(member.Name, specialBracketChars); + + if (!string.IsNullOrEmpty(member.ConstructorArgument)) { - map.Add(constructorArgumentName, splBracketCharacters); + map.Add(member.ConstructorArgument, specialBracketChars); } } }