mirror of
https://github.com/microsoft/PowerToys
synced 2025-09-04 08:25:11 +00:00
Lazy-fetch the settings for extensions (#38844)
This PR stops us from synchronously initializing the settings page for every extension (including built-in's) on startup. That incurs a small penalty that really adds up the more extensions a user has. Instead, we'll now only initialize the `CommandSettings` object when we first actually need it. From a relatively unscientific test, this saves approximately 10% on the initialization of builtin commands, and for my setup, it trims about 28% off extension initialization (across all built-in's / extensions): branch | Built-in load (ms) | Extension load (ms) | %Δ builtin | %Δ extensions | -- | -- | -- | -- | -- | main | 1455 | 6867.6 | | | this PR | 1309.2 | 4919 | -10.02% | -28.37% Closes #38321
This commit is contained in:
@@ -66,8 +66,10 @@ public sealed class CommandProviderWrapper
|
||||
DisplayName = provider.DisplayName;
|
||||
Icon = new(provider.Icon);
|
||||
Icon.InitializeProperties();
|
||||
|
||||
// Note: explicitly not InitializeProperties()ing the settings here. If
|
||||
// we do that, then we'd regress GH #38321
|
||||
Settings = new(provider.Settings, this, _taskScheduler);
|
||||
Settings.InitializeProperties();
|
||||
|
||||
Logger.LogDebug($"Initialized command provider {ProviderId}");
|
||||
}
|
||||
@@ -151,9 +153,11 @@ public sealed class CommandProviderWrapper
|
||||
Icon = new(model.Icon);
|
||||
Icon.InitializeProperties();
|
||||
|
||||
// Note: explicitly not InitializeProperties()ing the settings here. If
|
||||
// we do that, then we'd regress GH #38321
|
||||
Settings = new(model.Settings, this, _taskScheduler);
|
||||
Settings.InitializeProperties();
|
||||
|
||||
// We do need to explicitly initialize commands though
|
||||
InitializeCommands(commands, fallbacks, serviceProvider, pageContext);
|
||||
|
||||
Logger.LogDebug($"Loaded commands from {DisplayName} ({ProviderId})");
|
||||
@@ -194,21 +198,6 @@ public sealed class CommandProviderWrapper
|
||||
}
|
||||
}
|
||||
|
||||
/* This is a View/ExtensionHost piece
|
||||
* public void AllowSetForeground(bool allow)
|
||||
{
|
||||
if (!IsExtension)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
var iextn = extensionWrapper?.GetExtensionObject();
|
||||
unsafe
|
||||
{
|
||||
PInvoke.CoAllowSetForegroundWindow(iextn);
|
||||
}
|
||||
}*/
|
||||
|
||||
public override bool Equals(object? obj) => obj is CommandProviderWrapper wrapper && isValid == wrapper.isValid;
|
||||
|
||||
public override int GetHashCode() => _commandProvider.GetHashCode();
|
||||
|
@@ -2,18 +2,25 @@
|
||||
// The Microsoft Corporation licenses this file to you under the MIT license.
|
||||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
using ManagedCommon;
|
||||
using Microsoft.CmdPal.UI.ViewModels.Models;
|
||||
using Microsoft.CommandPalette.Extensions;
|
||||
|
||||
namespace Microsoft.CmdPal.UI.ViewModels;
|
||||
|
||||
public partial class CommandSettingsViewModel(ICommandSettings _unsafeSettings, CommandProviderWrapper provider, TaskScheduler mainThread)
|
||||
public partial class CommandSettingsViewModel(ICommandSettings? _unsafeSettings, CommandProviderWrapper provider, TaskScheduler mainThread)
|
||||
{
|
||||
private readonly ExtensionObject<ICommandSettings> _model = new(_unsafeSettings);
|
||||
|
||||
public ContentPageViewModel? SettingsPage { get; private set; }
|
||||
|
||||
public void InitializeProperties()
|
||||
public bool Initialized { get; private set; }
|
||||
|
||||
public bool HasSettings =>
|
||||
_model.Unsafe != null && // We have a settings model AND
|
||||
(!Initialized || SettingsPage != null); // we weren't initialized, OR we were, and we do have a settings page
|
||||
|
||||
private void UnsafeInitializeProperties()
|
||||
{
|
||||
var model = _model.Unsafe;
|
||||
if (model == null)
|
||||
@@ -27,4 +34,27 @@ public partial class CommandSettingsViewModel(ICommandSettings _unsafeSettings,
|
||||
SettingsPage.InitializeProperties();
|
||||
}
|
||||
}
|
||||
|
||||
public void SafeInitializeProperties()
|
||||
{
|
||||
try
|
||||
{
|
||||
UnsafeInitializeProperties();
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
Logger.LogError($"Failed to load settings page", ex: ex);
|
||||
}
|
||||
|
||||
Initialized = true;
|
||||
}
|
||||
|
||||
public void DoOnUiThread(Action action)
|
||||
{
|
||||
Task.Factory.StartNew(
|
||||
action,
|
||||
CancellationToken.None,
|
||||
TaskCreationOptions.None,
|
||||
mainThread);
|
||||
}
|
||||
}
|
||||
|
@@ -18,6 +18,8 @@ public partial class ProviderSettingsViewModel(
|
||||
IServiceProvider _serviceProvider) : ObservableObject
|
||||
{
|
||||
private readonly SettingsModel _settings = _serviceProvider.GetService<SettingsModel>()!;
|
||||
private readonly Lock _initializeSettingsLock = new();
|
||||
private Task? _initializeSettingsTask;
|
||||
|
||||
public string DisplayName => _provider.DisplayName;
|
||||
|
||||
@@ -34,6 +36,9 @@ public partial class ProviderSettingsViewModel(
|
||||
|
||||
public IconInfoViewModel Icon => _provider.Icon;
|
||||
|
||||
[ObservableProperty]
|
||||
public partial bool LoadingSettings { get; set; } = _provider.Settings?.HasSettings ?? false;
|
||||
|
||||
public bool IsEnabled
|
||||
{
|
||||
get => _providerSettings.IsEnabled;
|
||||
@@ -56,15 +61,60 @@ public partial class ProviderSettingsViewModel(
|
||||
}
|
||||
}
|
||||
|
||||
private void Provider_CommandsChanged(CommandProviderWrapper sender, CommandPalette.Extensions.IItemsChangedEventArgs args)
|
||||
/// <summary>
|
||||
/// Gets a value indicating whether returns true if we have a settings page
|
||||
/// that's initialized, or we are still working on initializing that
|
||||
/// settings page. If we don't have a settings object, or that settings
|
||||
/// object doesn't have a settings page, then we'll return false.
|
||||
/// </summary>
|
||||
public bool HasSettings
|
||||
{
|
||||
OnPropertyChanged(nameof(ExtensionSubtext));
|
||||
OnPropertyChanged(nameof(TopLevelCommands));
|
||||
get
|
||||
{
|
||||
if (_provider.Settings == null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
public bool HasSettings => _provider.Settings != null && _provider.Settings.SettingsPage != null;
|
||||
if (_provider.Settings.Initialized)
|
||||
{
|
||||
return _provider.Settings.HasSettings;
|
||||
}
|
||||
|
||||
public ContentPageViewModel? SettingsPage => HasSettings ? _provider?.Settings?.SettingsPage : null;
|
||||
// settings still need to be loaded.
|
||||
return LoadingSettings;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Gets will return the settings page, if we have one, and have initialized it.
|
||||
/// If we haven't initialized it, this will kick off a thread to start
|
||||
/// initializing it.
|
||||
/// </summary>
|
||||
public ContentPageViewModel? SettingsPage
|
||||
{
|
||||
get
|
||||
{
|
||||
if (_provider.Settings == null)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
if (_provider.Settings.Initialized)
|
||||
{
|
||||
LoadingSettings = false;
|
||||
return _provider.Settings.SettingsPage;
|
||||
}
|
||||
|
||||
// Don't load the settings if we're already working on it
|
||||
lock (_initializeSettingsLock)
|
||||
{
|
||||
_initializeSettingsTask ??= Task.Run(InitializeSettingsPage);
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
[field: AllowNull]
|
||||
public List<TopLevelViewModel> TopLevelCommands
|
||||
@@ -90,4 +140,30 @@ public partial class ProviderSettingsViewModel(
|
||||
}
|
||||
|
||||
private void Save() => SettingsModel.SaveSettings(_settings);
|
||||
|
||||
private void InitializeSettingsPage()
|
||||
{
|
||||
if (_provider.Settings == null)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
_provider.Settings.SafeInitializeProperties();
|
||||
_provider.Settings.DoOnUiThread(() =>
|
||||
{
|
||||
// Changing these properties will try to update XAML, and that has
|
||||
// to be handled on the UI thread, so we need to raise them on the
|
||||
// UI thread
|
||||
LoadingSettings = false;
|
||||
OnPropertyChanged(nameof(HasSettings));
|
||||
OnPropertyChanged(nameof(LoadingSettings));
|
||||
OnPropertyChanged(nameof(SettingsPage));
|
||||
});
|
||||
}
|
||||
|
||||
private void Provider_CommandsChanged(CommandProviderWrapper sender, CommandPalette.Extensions.IItemsChangedEventArgs args)
|
||||
{
|
||||
OnPropertyChanged(nameof(ExtensionSubtext));
|
||||
OnPropertyChanged(nameof(TopLevelCommands));
|
||||
}
|
||||
}
|
||||
|
@@ -45,6 +45,9 @@ public partial class TopLevelCommandManager : ObservableObject,
|
||||
|
||||
public async Task<bool> LoadBuiltinsAsync()
|
||||
{
|
||||
var s = new Stopwatch();
|
||||
s.Start();
|
||||
|
||||
_builtInCommands.Clear();
|
||||
|
||||
// Load built-In commands first. These are all in-proc, and
|
||||
@@ -64,6 +67,10 @@ public partial class TopLevelCommandManager : ObservableObject,
|
||||
}
|
||||
}
|
||||
|
||||
s.Stop();
|
||||
|
||||
Logger.LogDebug($"Loading built-ins took {s.ElapsedMilliseconds}ms");
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
@@ -113,7 +113,24 @@
|
||||
Visibility="{x:Bind ViewModel.HasSettings}" />
|
||||
|
||||
<Frame x:Name="SettingsFrame" Visibility="{x:Bind ViewModel.HasSettings}">
|
||||
|
||||
<controls:SwitchPresenter
|
||||
HorizontalAlignment="Stretch"
|
||||
TargetType="x:Boolean"
|
||||
Value="{x:Bind ViewModel.LoadingSettings, Mode=OneWay}">
|
||||
<controls:Case Value="True">
|
||||
<ProgressRing
|
||||
Width="36"
|
||||
Height="36"
|
||||
HorizontalAlignment="Center"
|
||||
VerticalAlignment="Center"
|
||||
IsIndeterminate="True" />
|
||||
</controls:Case>
|
||||
<controls:Case Value="False">
|
||||
<cmdpalUI:ContentPage ViewModel="{x:Bind ViewModel.SettingsPage, Mode=OneWay}" />
|
||||
</controls:Case>
|
||||
</controls:SwitchPresenter>
|
||||
|
||||
</Frame>
|
||||
|
||||
<TextBlock
|
||||
|
Reference in New Issue
Block a user