From 3550dedd4459b89be2a0677388c0e4ec49ae4abb Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 00:58:12 +0200 Subject: [PATCH 01/11] Simplify implementation for ImageTagSet equality --- Xwt/Xwt.Drawing/Image.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index 64dd9aad8..ea0c89fc4 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -1009,12 +1009,7 @@ public string [] AsArray { public override bool Equals (object obj) { var other = obj as ImageTagSet; - if (other == null || tagsArray.Length != other.tagsArray.Length) - return false; - for (int n = 0; n < tagsArray.Length; n++) - if (tagsArray [n] != other.tagsArray [n]) - return false; - return true; + return other != null && tagsArray.SequenceEqual(other.tagsArray); } public override int GetHashCode () From 25cb1aa3cd01bbdaf4e80044624fc5e90ff3a9af Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 01:01:06 +0200 Subject: [PATCH 02/11] Reuse the same tag separator array We're targeting netstandard2.0 and don't have ReadOnlySpan or any of the new APIs that avoid allocating --- Xwt/Xwt.Drawing/Image.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index ea0c89fc4..52dc59e64 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -973,6 +973,7 @@ class ImageTagSet string[] tagsArray; public static readonly ImageTagSet Empty = new ImageTagSet (new string[0]); + static readonly char[] tagSeparators = { '~' }; public ImageTagSet (string [] tagsArray) { @@ -988,7 +989,7 @@ public bool IsEmpty { public ImageTagSet (string tags) { - tagsArray = tags.Split (new [] { '~' }, StringSplitOptions.RemoveEmptyEntries); + tagsArray = tags.Split (tagSeparators, StringSplitOptions.RemoveEmptyEntries); Array.Sort (AsArray); } From 30ee113fa5681dca4d4cc99770695a5edb9e78ef Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 01:29:52 +0200 Subject: [PATCH 03/11] Add a lookup table for most common tags used in images --- Xwt/Xwt.Drawing/Image.cs | 82 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 4 deletions(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index 52dc59e64..75427f0a7 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -967,13 +967,88 @@ public void ReleaseReference (bool disposing) public NativeImageRef NextRef { get; set; } } - class ImageTagSet + sealed class ImageTagCache + { + /* Some stats from an app using Xwt: +1474 dark +1304 contrast +1296 contrast~dark + 846 sel + 846 dark~sel + 132 disabled + 120 dark~disabled + 116 contrast~disabled + 116 contrast~dark~disabled + 22 error + 22 dark~error + 22 contrast~error + 22 contrast~dark~error + 14 contrast~dark~sel + 12 contrast~sel + 6 disabled~dark + 6 dark~contrast + 2 sel~error + 2 pressed~dark + 2 pressed + 2 hover~dark + 2 hover + 2 dark~sel~error + 2 contrast~sel~error + 2 active~sel + 2 active~dark~sel + 2 active~dark + 2 active~contrast~dark + 2 active~contrast + 2 active + */ + readonly string[] knownTags = new[] { + "~dark", + "~contrast", + "~contrast~dark", + "~sel", + "~dark~sel", + "~disabled", + "~dark~disabled", + "~contrast~disabled", + "~contrast~dark~disabled", + }; + + readonly string[][] knownTagArrays = new[] { + new[] { "dark", }, + new[] { "contrast", }, + new[] { "contrast", "dark", }, + new[] { "sel", }, + new[] { "dark", "sel", }, + new[] { "disabled", }, + new[] { "dark", "disabled", }, + new[] { "contrast", "disabled", }, + new[] { "contrast", "dark", "disabled", }, + }; + + static readonly char[] tagSeparators = { '~' }; + public bool GetTagArray(string tags, out string[] tagArray) + { + var index = Array.IndexOf(knownTags, tags); + tagArray = index >= 0 ? knownTagArrays[index] : SplitTags(tags); + return index >= 0; + } + + static string[] SplitTags(string tags) + { + var array = tags.Split(tagSeparators, StringSplitOptions.RemoveEmptyEntries); + Array.Sort(array); + + return array; + } + } + + sealed class ImageTagSet { string tags; string[] tagsArray; public static readonly ImageTagSet Empty = new ImageTagSet (new string[0]); - static readonly char[] tagSeparators = { '~' }; + static readonly ImageTagCache imageTagCache; public ImageTagSet (string [] tagsArray) { @@ -989,8 +1064,7 @@ public bool IsEmpty { public ImageTagSet (string tags) { - tagsArray = tags.Split (tagSeparators, StringSplitOptions.RemoveEmptyEntries); - Array.Sort (AsArray); + imageTagCache.GetTagArray(tags, out tagsArray); } public string AsString { From 1557d48d15c1400774598f2660979b9c95bdd5f9 Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 02:27:16 +0200 Subject: [PATCH 04/11] Remove unused tags string and improvements to caching We cache the ImageTagSet directly now. That should allow us to create fewer of these instances overall. For the app I'm testing on, we cover 97% of the image tags combinations In practice, cache hits will be better overall. The other improvement is to not have to sort/create imagetagsets every time, instead opting to use more specialized APIs. --- Xwt/Xwt.Drawing/Image.cs | 96 ++++++++++++++++++++++++---------------- 1 file changed, 58 insertions(+), 38 deletions(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index 75427f0a7..838cf541b 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -31,6 +31,7 @@ using System.Reflection; using System.IO; using System.Collections.Generic; +using System.Diagnostics; namespace Xwt.Drawing { @@ -268,7 +269,7 @@ static bool ParseImageHints (string baseName, string fileName, string ext, out i return false; } else i2 = fileName.Length; - tags = new ImageTagSet (fileName.Substring (i, i2 - i)); + tags = ImageTagSet.Parse (fileName.Substring (i, i2 - i)); return true; } else { @@ -288,7 +289,7 @@ static bool ParseImageHints (string baseName, string fileName, string ext, out i return false; } if (i2 + 2 < fileName.Length) - tags = new ImageTagSet (fileName.Substring (i2 + 2)); + tags = ImageTagSet.Parse (fileName.Substring (i2 + 2)); return true; } } @@ -307,18 +308,23 @@ public static Image CreateMultiSizeIcon (IEnumerable images) // If one of the images is themed, then the whole resulting image will be themed. // To create the new image, we group images with the same theme but different size, and we create a multi-size icon for those. // The resulting image is the combination of those multi-size icons. - var allThemes = allImages.OfType ().SelectMany (i => i.Images).Select (i => new ImageTagSet (i.Item2)).Distinct ().ToArray (); + var allThemes = allImages + .OfType () + .SelectMany (i => i.Images) + .Select(i => i.Item2) + .Distinct (TagSetEqualityComparer.Instance) + .ToArray (); List> newImages = new List> (); foreach (var ts in allThemes) { List multiSizeImages = new List (); foreach (var i in allImages) { if (i is ThemedImage) - multiSizeImages.Add (((ThemedImage)i).GetImage (ts.AsArray)); + multiSizeImages.Add (((ThemedImage)i).GetImage (ts)); else multiSizeImages.Add (i); } var img = CreateMultiSizeIcon (multiSizeImages); - newImages.Add (new Tuple (img, ts.AsArray)); + newImages.Add (new Tuple (img, ts)); } return new ThemedImage (newImages); } else { @@ -1000,6 +1006,9 @@ 2 active~dark 2 active~contrast~dark 2 active~contrast 2 active + + Keep in sync with knownTagArrays. + These tag items amount for 97% of the image tags found in images. */ readonly string[] knownTags = new[] { "~dark", @@ -1013,65 +1022,74 @@ 2 active "~contrast~dark~disabled", }; - readonly string[][] knownTagArrays = new[] { - new[] { "dark", }, - new[] { "contrast", }, - new[] { "contrast", "dark", }, - new[] { "sel", }, - new[] { "dark", "sel", }, - new[] { "disabled", }, - new[] { "dark", "disabled", }, - new[] { "contrast", "disabled", }, - new[] { "contrast", "dark", "disabled", }, + readonly ImageTagSet[] knownTagArrays = new[] { + new ImageTagSet(new[] { "dark", }), + new ImageTagSet(new[] { "contrast", }), + new ImageTagSet(new[] { "contrast", "dark", }), + new ImageTagSet(new[] { "sel", }), + new ImageTagSet(new[] { "dark", "sel", }), + new ImageTagSet(new[] { "disabled", }), + new ImageTagSet(new[] { "dark", "disabled", }), + new ImageTagSet(new[] { "contrast", "disabled", }), + new ImageTagSet(new[] { "contrast", "dark", "disabled", }), }; - static readonly char[] tagSeparators = { '~' }; - public bool GetTagArray(string tags, out string[] tagArray) + public ImageTagSet TryGetTagSet(string tags) { var index = Array.IndexOf(knownTags, tags); - tagArray = index >= 0 ? knownTagArrays[index] : SplitTags(tags); - return index >= 0; + return index >= 0 ? knownTagArrays[index] : null; } + } - static string[] SplitTags(string tags) - { - var array = tags.Split(tagSeparators, StringSplitOptions.RemoveEmptyEntries); - Array.Sort(array); + // As much as I don't like the duplication, it's simpler than accessing a static instance every time. + class TagSetEqualityComparer : IEqualityComparer + { + public static TagSetEqualityComparer Instance { get; } = new TagSetEqualityComparer(); + + public bool Equals(string[] x, string[] y) => x.SequenceEqual(y); - return array; + public int GetHashCode(string[] obj) + { + unchecked + { + int c = 0; + foreach (var s in obj) + c %= s.GetHashCode(); + return c; + } } } + [DebuggerDisplay("{DebuggerDisplay,nq}")] sealed class ImageTagSet { - string tags; string[] tagsArray; public static readonly ImageTagSet Empty = new ImageTagSet (new string[0]); static readonly ImageTagCache imageTagCache; + static readonly char[] tagSeparators = { '~' }; - public ImageTagSet (string [] tagsArray) + public static ImageTagSet Parse(string tags) { - this.tagsArray = tagsArray; - Array.Sort (tagsArray); + return imageTagCache.TryGetTagSet(tags) ?? Create(tags); } - public bool IsEmpty { - get { - return tagsArray.Length == 0; - } + static ImageTagSet Create(string tags) + { + var tagArray = tags.Split(tagSeparators, StringSplitOptions.RemoveEmptyEntries); + Array.Sort(tagArray); + + return new ImageTagSet(tagArray); } - public ImageTagSet (string tags) + public ImageTagSet (string [] tagsArray) { - imageTagCache.GetTagArray(tags, out tagsArray); + this.tagsArray = tagsArray; } - public string AsString { + public bool IsEmpty { get { - if (tags == null) - tags = string.Join ("~", tagsArray); - return tags; + return tagsArray.Length == 0; } } @@ -1096,6 +1114,8 @@ public override int GetHashCode () return c; } } + + string DebuggerDisplay => string.Join("~", tagsArray); } abstract class ImageLoader From 455d72bb6dea9a3f7d2e47a85b5f76e7eb3feae1 Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 02:57:36 +0200 Subject: [PATCH 05/11] Forgot to initialize --- Xwt/Xwt.Drawing/Image.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index 838cf541b..544396142 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -1066,7 +1066,7 @@ sealed class ImageTagSet string[] tagsArray; public static readonly ImageTagSet Empty = new ImageTagSet (new string[0]); - static readonly ImageTagCache imageTagCache; + static readonly ImageTagCache imageTagCache = new ImageTagCache(); static readonly char[] tagSeparators = { '~' }; public static ImageTagSet Parse(string tags) From 3dd2e281cba54899badcc938bbad20298742689d Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 03:28:08 +0200 Subject: [PATCH 06/11] Correct formatting of tags string --- Xwt/Xwt.Drawing/Image.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index 544396142..26f44d022 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -1011,15 +1011,15 @@ Keep in sync with knownTagArrays. These tag items amount for 97% of the image tags found in images. */ readonly string[] knownTags = new[] { - "~dark", - "~contrast", - "~contrast~dark", - "~sel", - "~dark~sel", - "~disabled", - "~dark~disabled", - "~contrast~disabled", - "~contrast~dark~disabled", + "dark", + "contrast", + "contrast~dark", + "sel", + "dark~sel", + "disabled", + "dark~disabled", + "contrast~disabled", + "contrast~dark~disabled", }; readonly ImageTagSet[] knownTagArrays = new[] { From cb9c628fa0549b129fd329117b43d86485483657 Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 04:00:38 +0200 Subject: [PATCH 07/11] Avoid allocating substrings --- Xwt/Xwt.Drawing/Image.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index 26f44d022..210d5220a 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -253,7 +253,7 @@ static bool ParseImageHints (string baseName, string fileName, string ext, out i tags = ImageTagSet.Empty; var firstDelimiter = fileName.IndexOfAny (tagDelimiters); - if (firstDelimiter <= 0 || fileName.Length <= baseName.Length + 1 || !fileName.Substring(0, firstDelimiter).Equals(baseName, StringComparison.Ordinal)) + if (firstDelimiter <= 0 || fileName.Length <= baseName.Length + 1 || string.Compare(fileName, 0, baseName, 0, firstDelimiter) != 0) return false; fileName = fileName.Substring (0, fileName.Length - ext.Length); From 7511a91b44157e80d23a6e2eb13c8b319a44ee4b Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 17:18:22 +0200 Subject: [PATCH 08/11] Reuse a cached array for the manifest resource names --- Xwt/Xwt.Drawing/Image.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index 210d5220a..bb122debe 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -32,6 +32,7 @@ using System.IO; using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.CompilerServices; namespace Xwt.Drawing { @@ -1144,9 +1145,11 @@ public override object LoadImage (string fileName) return img; } + ConditionalWeakTable resourceNamesCache = new ConditionalWeakTable(); public override IEnumerable GetAlternativeFiles (string fileName, string baseName, string ext) { - return assembly.GetManifestResourceNames ().Where (f => + var resourceNames = resourceNamesCache.GetValue(assembly, asm => asm.GetManifestResourceNames()); + return resourceNames.Where (f => f.StartsWith (baseName, StringComparison.Ordinal) && f.EndsWith (ext, StringComparison.Ordinal)); } From 681e4cda893ffba569d4164da611a5914055a48c Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 17:36:47 +0200 Subject: [PATCH 09/11] More caching in enumerating alternative files --- Xwt/Xwt.Drawing/Image.cs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index bb122debe..42fec9fc7 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -44,6 +44,7 @@ public class Image: XwtObject, IDisposable internal StyleSet styles; internal static int[] SupportedScales = { 2 }; + internal static string[] SupportedScalesTags = SupportedScales.Select(scale => "@" + scale + "x").ToArray(); internal Image () { @@ -1183,16 +1184,22 @@ public override object LoadImage (string fileName) public override IEnumerable GetAlternativeFiles (string fileName, string baseName, string ext) { - if (!Context.RegisteredStyles.Any ()) { - foreach (var s in Image.SupportedScales) { - var fn = baseName + "@" + s + "x" + ext; - if (File.Exists (fn)) - yield return fn; - } - } else { - var files = Directory.EnumerateFiles (Path.GetDirectoryName (fileName), Path.GetFileName (baseName) + "*" + ext); - foreach (var f in files) - yield return f; + if (!Context.RegisteredStyles.Any()) + { + return EnumerateFilesForRegisteredStyles(baseName, ext); + } + else + { + return Directory.EnumerateFiles(Path.GetDirectoryName(fileName), Path.GetFileName(baseName) + "*" + ext); + } + } + + IEnumerable EnumerateFilesForRegisteredStyles (string baseName, string ext) + { + foreach (var scaleTag in Image.SupportedScalesTags) { + var fn = baseName + scaleTag + ext; + if (File.Exists (fn)) + yield return fn; } } From be8d81a26dc18ebaaa52ee94dabb416631d19bb8 Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 18:20:44 +0200 Subject: [PATCH 10/11] Use specialized APIs to get the handles --- Xwt.XamMac/Xwt.Mac/ImageHandler.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Xwt.XamMac/Xwt.Mac/ImageHandler.cs b/Xwt.XamMac/Xwt.Mac/ImageHandler.cs index 1b71ed01a..b8c15d387 100644 --- a/Xwt.XamMac/Xwt.Mac/ImageHandler.cs +++ b/Xwt.XamMac/Xwt.Mac/ImageHandler.cs @@ -40,10 +40,10 @@ namespace Xwt.Mac { public class ImageHandler: ImageBackendHandler { - static readonly IntPtr sel_alloc = new Selector ("alloc").Handle; - static readonly IntPtr sel_release = new Selector ("release").Handle; - static readonly IntPtr sel_initWithIconRef = new Selector ("initWithIconRef:").Handle; - static readonly IntPtr cls_NSImage = new Class (typeof (NSImage)).Handle; + static readonly IntPtr sel_alloc = Selector.GetHandle ("alloc"); + static readonly IntPtr sel_release = Selector.GetHandle ("release"); + static readonly IntPtr sel_initWithIconRef = Selector.GetHandle ("initWithIconRef:"); + static readonly IntPtr cls_NSImage = Class.GetHandle(typeof (NSImage)); static Dictionary stockIcons = new Dictionary (); From b4ceca4ea7f6979db635212281be201c29ffe88b Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Tue, 21 Feb 2023 18:26:36 +0200 Subject: [PATCH 11/11] This shoudl be static --- Xwt/Xwt.Drawing/Image.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Xwt/Xwt.Drawing/Image.cs b/Xwt/Xwt.Drawing/Image.cs index 42fec9fc7..e7b796765 100644 --- a/Xwt/Xwt.Drawing/Image.cs +++ b/Xwt/Xwt.Drawing/Image.cs @@ -1146,7 +1146,7 @@ public override object LoadImage (string fileName) return img; } - ConditionalWeakTable resourceNamesCache = new ConditionalWeakTable(); + static ConditionalWeakTable resourceNamesCache = new ConditionalWeakTable(); public override IEnumerable GetAlternativeFiles (string fileName, string baseName, string ext) { var resourceNames = resourceNamesCache.GetValue(assembly, asm => asm.GetManifestResourceNames());