From e8691c210dbd2ee79d9f06503402a8bd4a2e2720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=98=E9=9F=AC=20=E5=BC=A0?= Date: Sun, 23 May 2021 12:28:45 +0800 Subject: [PATCH 1/8] No need for the wrapped Lazy instance in image loading. --- Flow.Launcher/ResultListBox.xaml | 2 +- Flow.Launcher/ViewModel/ResultViewModel.cs | 92 ++++++---------------- 2 files changed, 27 insertions(+), 67 deletions(-) diff --git a/Flow.Launcher/ResultListBox.xaml b/Flow.Launcher/ResultListBox.xaml index 2f9d06d814e..0f70dce4103 100644 --- a/Flow.Launcher/ResultListBox.xaml +++ b/Flow.Launcher/ResultListBox.xaml @@ -42,7 +42,7 @@ + Source="{Binding Image}" /> diff --git a/Flow.Launcher/ViewModel/ResultViewModel.cs b/Flow.Launcher/ViewModel/ResultViewModel.cs index c91bbb1074f..dd4b351c31e 100644 --- a/Flow.Launcher/ViewModel/ResultViewModel.cs +++ b/Flow.Launcher/ViewModel/ResultViewModel.cs @@ -11,62 +11,12 @@ namespace Flow.Launcher.ViewModel { public class ResultViewModel : BaseModel { - public class LazyAsync : Lazy> - { - private readonly T defaultValue; - - private readonly Action _updateCallback; - public new T Value - { - get - { - if (!IsValueCreated) - { - _ = Exercute(); // manually use callback strategy - - return defaultValue; - } - - if (!base.Value.IsCompletedSuccessfully) - return defaultValue; - - return base.Value.Result; - - // If none of the variables captured by the local function are captured by other lambdas, - // the compiler can avoid heap allocations. - async ValueTask Exercute() - { - await base.Value.ConfigureAwait(false); - _updateCallback(); - } - - } - } - public LazyAsync(Func> factory, T defaultValue, Action updateCallback) : base(factory) - { - if (defaultValue != null) - { - this.defaultValue = defaultValue; - } - - _updateCallback = updateCallback; - } - } - public ResultViewModel(Result result, Settings settings) { if (result != null) { Result = result; - - Image = new LazyAsync( - SetImage, - ImageLoader.DefaultImage, - () => - { - OnPropertyChanged(nameof(Image)); - }); - } + } Settings = settings; } @@ -85,44 +35,54 @@ public ResultViewModel(Result result, Settings settings) ? Result.SubTitle : Result.SubTitleToolTip; - public LazyAsync Image { get; set; } + private bool ImageLoaded; + + private ImageSource image = ImageLoader.DefaultImage; - private async ValueTask SetImage() + public ImageSource Image + { + get + { + if (!ImageLoaded) + { + ImageLoaded = true; + LoadImage(); + } + return image; + } + private set => image = value; + } + private async void LoadImage() { var imagePath = Result.IcoPath; if (string.IsNullOrEmpty(imagePath) && Result.Icon != null) { try { - return Result.Icon(); + Image = Result.Icon(); + return; } catch (Exception e) { Log.Exception($"|ResultViewModel.Image|IcoPath is empty and exception when calling Icon() for result <{Result.Title}> of plugin <{Result.PluginDirectory}>", e); - return ImageLoader.DefaultImage; } } if (ImageLoader.CacheContainImage(imagePath)) + { // will get here either when icoPath has value\icon delegate is null\when had exception in delegate - return ImageLoader.Load(imagePath); + Image = ImageLoader.Load(imagePath); + return; + } - return await Task.Run(() => ImageLoader.Load(imagePath)); + Image = await Task.Run(() => ImageLoader.Load(imagePath)).ConfigureAwait(false); } public Result Result { get; } public override bool Equals(object obj) { - var r = obj as ResultViewModel; - if (r != null) - { - return Result.Equals(r.Result); - } - else - { - return false; - } + return obj is ResultViewModel r && Result.Equals(r.Result); } public override int GetHashCode() From 37f1a64495b848e5951cce83d57e375bb5fce5de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=98=E9=9F=AC=20=E5=BC=A0?= Date: Sun, 23 May 2021 22:54:52 +0800 Subject: [PATCH 2/8] avoid duplicate removing --- Flow.Launcher.Infrastructure/Image/ImageCache.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Flow.Launcher.Infrastructure/Image/ImageCache.cs b/Flow.Launcher.Infrastructure/Image/ImageCache.cs index bb7ec681781..4e95278c10c 100644 --- a/Flow.Launcher.Infrastructure/Image/ImageCache.cs +++ b/Flow.Launcher.Infrastructure/Image/ImageCache.cs @@ -26,7 +26,7 @@ public class ImageCache private const int MaxCached = 50; public ConcurrentDictionary Data { get; private set; } = new ConcurrentDictionary(); private const int permissibleFactor = 2; - + public void Initialization(Dictionary usage) { foreach (var key in usage.Keys) @@ -35,6 +35,8 @@ public void Initialization(Dictionary usage) } } + private volatile bool removing; + public ImageSource this[string path] { get @@ -62,11 +64,13 @@ public ImageSource this[string path] // To prevent the dictionary from drastically increasing in size by caching images, the dictionary size is not allowed to grow more than the permissibleFactor * maxCached size // This is done so that we don't constantly perform this resizing operation and also maintain the image cache size at the same time - if (Data.Count > permissibleFactor * MaxCached) + if (Data.Count > permissibleFactor * MaxCached && !removing) { + removing = true; // To delete the images from the data dictionary based on the resizing of the Usage Dictionary. foreach (var key in Data.OrderBy(x => x.Value.usage).Take(Data.Count - MaxCached).Select(x => x.Key)) Data.TryRemove(key, out _); + removing = false; } } } From 1c6d207595c941c8bfa3d99a12d7e5c208ffe007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=98=E9=9F=AC=20=E5=BC=A0?= Date: Mon, 24 May 2021 10:34:16 +0800 Subject: [PATCH 3/8] only modify property when async (avoid duplicate read) --- Flow.Launcher/ViewModel/ResultViewModel.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Flow.Launcher/ViewModel/ResultViewModel.cs b/Flow.Launcher/ViewModel/ResultViewModel.cs index dd4b351c31e..791aa6da98b 100644 --- a/Flow.Launcher/ViewModel/ResultViewModel.cs +++ b/Flow.Launcher/ViewModel/ResultViewModel.cs @@ -59,7 +59,7 @@ private async void LoadImage() { try { - Image = Result.Icon(); + image = Result.Icon(); return; } catch (Exception e) @@ -71,7 +71,7 @@ private async void LoadImage() if (ImageLoader.CacheContainImage(imagePath)) { // will get here either when icoPath has value\icon delegate is null\when had exception in delegate - Image = ImageLoader.Load(imagePath); + image = ImageLoader.Load(imagePath); return; } From 734a0cbfa1ac5fbfa47b4b6cdae13c8c20b2b051 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=98=E9=9F=AC=20=E5=BC=A0?= Date: Mon, 24 May 2021 10:35:18 +0800 Subject: [PATCH 4/8] Update comment --- Flow.Launcher/ViewModel/ResultViewModel.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Flow.Launcher/ViewModel/ResultViewModel.cs b/Flow.Launcher/ViewModel/ResultViewModel.cs index 791aa6da98b..4d57f825296 100644 --- a/Flow.Launcher/ViewModel/ResultViewModel.cs +++ b/Flow.Launcher/ViewModel/ResultViewModel.cs @@ -75,6 +75,7 @@ private async void LoadImage() return; } + // We need to modify the property not field here to trigger the OnPropertyChanged event Image = await Task.Run(() => ImageLoader.Load(imagePath)).ConfigureAwait(false); } From 710d2a89a2f861b396eb91bef2b9d0f1a79557ac Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Sat, 31 Jul 2021 16:15:49 +0800 Subject: [PATCH 5/8] Use ValueTask as return value to suppress potential image error --- Flow.Launcher/ViewModel/ResultViewModel.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Flow.Launcher/ViewModel/ResultViewModel.cs b/Flow.Launcher/ViewModel/ResultViewModel.cs index 4d57f825296..190f592d774 100644 --- a/Flow.Launcher/ViewModel/ResultViewModel.cs +++ b/Flow.Launcher/ViewModel/ResultViewModel.cs @@ -35,7 +35,7 @@ public ResultViewModel(Result result, Settings settings) ? Result.SubTitle : Result.SubTitleToolTip; - private bool ImageLoaded; + private volatile bool ImageLoaded; private ImageSource image = ImageLoader.DefaultImage; @@ -46,13 +46,13 @@ public ImageSource Image if (!ImageLoaded) { ImageLoaded = true; - LoadImage(); + _ = LoadImageAsync(); } return image; } private set => image = value; } - private async void LoadImage() + private async ValueTask LoadImageAsync() { var imagePath = Result.IcoPath; if (string.IsNullOrEmpty(imagePath) && Result.Icon != null) From a0835288e2335d03bd8aa985d1d63b55c3616dcb Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Sat, 31 Jul 2021 16:23:58 +0800 Subject: [PATCH 6/8] Use SemaphoreSlim to ensure cache only remove once --- .../Image/ImageCache.cs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/Flow.Launcher.Infrastructure/Image/ImageCache.cs b/Flow.Launcher.Infrastructure/Image/ImageCache.cs index 4e95278c10c..66104e80693 100644 --- a/Flow.Launcher.Infrastructure/Image/ImageCache.cs +++ b/Flow.Launcher.Infrastructure/Image/ImageCache.cs @@ -2,6 +2,7 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using System.Windows.Media; @@ -26,6 +27,7 @@ public class ImageCache private const int MaxCached = 50; public ConcurrentDictionary Data { get; private set; } = new ConcurrentDictionary(); private const int permissibleFactor = 2; + private SemaphoreSlim semaphore = new(1, 1); public void Initialization(Dictionary usage) { @@ -35,8 +37,6 @@ public void Initialization(Dictionary usage) } } - private volatile bool removing; - public ImageSource this[string path] { get @@ -62,15 +62,22 @@ public ImageSource this[string path] } ); - // To prevent the dictionary from drastically increasing in size by caching images, the dictionary size is not allowed to grow more than the permissibleFactor * maxCached size - // This is done so that we don't constantly perform this resizing operation and also maintain the image cache size at the same time - if (Data.Count > permissibleFactor * MaxCached && !removing) + SliceExtra(); + + async void SliceExtra() { - removing = true; - // To delete the images from the data dictionary based on the resizing of the Usage Dictionary. - foreach (var key in Data.OrderBy(x => x.Value.usage).Take(Data.Count - MaxCached).Select(x => x.Key)) - Data.TryRemove(key, out _); - removing = false; + // To prevent the dictionary from drastically increasing in size by caching images, the dictionary size is not allowed to grow more than the permissibleFactor * maxCached size + // This is done so that we don't constantly perform this resizing operation and also maintain the image cache size at the same time + if (Data.Count > permissibleFactor * MaxCached) + { + await semaphore.WaitAsync(); + // To delete the images from the data dictionary based on the resizing of the Usage Dictionary + // Double Check to avoid concurrent remove + if (Data.Count > permissibleFactor * MaxCached) + foreach (var key in Data.OrderBy(x => x.Value.usage).Take(Data.Count - MaxCached).Select(x => x.Key).ToArray()) + Data.TryRemove(key, out _); + semaphore.Release(); + } } } } From 653dc83bd08819308e9a66bf3badeb9e4dc847e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=BC=98=E9=9F=AC?= Date: Sun, 8 Aug 2021 13:45:18 +0800 Subject: [PATCH 7/8] fix error thrown in checking cache key when key is null --- Flow.Launcher.Infrastructure/Image/ImageCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Flow.Launcher.Infrastructure/Image/ImageCache.cs b/Flow.Launcher.Infrastructure/Image/ImageCache.cs index 66104e80693..4d11abe2328 100644 --- a/Flow.Launcher.Infrastructure/Image/ImageCache.cs +++ b/Flow.Launcher.Infrastructure/Image/ImageCache.cs @@ -84,7 +84,7 @@ async void SliceExtra() public bool ContainsKey(string key) { - return Data.ContainsKey(key) && Data[key].imageSource != null; + return key is not null && Data.ContainsKey(key) && Data[key].imageSource != null; } public int CacheSize() From 3623e119d2be16c1c10668340978cb74de886732 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=BC=98=E9=9F=AC?= Date: Sun, 8 Aug 2021 13:46:31 +0800 Subject: [PATCH 8/8] add configureAwait(false) in SliceExtra ImageCache --- Flow.Launcher.Infrastructure/Image/ImageCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Flow.Launcher.Infrastructure/Image/ImageCache.cs b/Flow.Launcher.Infrastructure/Image/ImageCache.cs index 4d11abe2328..13277c7d995 100644 --- a/Flow.Launcher.Infrastructure/Image/ImageCache.cs +++ b/Flow.Launcher.Infrastructure/Image/ImageCache.cs @@ -70,7 +70,7 @@ async void SliceExtra() // This is done so that we don't constantly perform this resizing operation and also maintain the image cache size at the same time if (Data.Count > permissibleFactor * MaxCached) { - await semaphore.WaitAsync(); + await semaphore.WaitAsync().ConfigureAwait(false); // To delete the images from the data dictionary based on the resizing of the Usage Dictionary // Double Check to avoid concurrent remove if (Data.Count > permissibleFactor * MaxCached)