Call UpdateJumplist only if the settings changed

src/cascadia/TerminalApp/AppLogic.cpp

@@ -297,6 +297,11 @@ namespace winrt::TerminalApp::implementation
             {
                 _root->SetFullscreen(true);
             }
+
+            // Both LoadSettings and ReloadSettings are supposed to call this function,
+            // but LoadSettings skips it, so that the UI starts up faster.
+            // Now that the UI is present we can do them with a less significant UX impact.
+            _ProcessLazySettingsChanges();
         });
         _root->Create();
 
@@ -886,8 +891,27 @@ namespace winrt::TerminalApp::implementation
 
         // Register for directory change notification.
         _RegisterSettingsChange();
+    }
+
+    // Call this function after loading your _settings.
+    // It handles any CPU intensive settings updates (like updating the Jumplist)
+    // which should thus only occur if the settings file actually changed.
+    void AppLogic::_ProcessLazySettingsChanges()
+    {
+        const auto hash = _settings.Hash();
+        const auto applicationState = ApplicationState::SharedInstance();
+        const auto cachedHash = applicationState.SettingsHash();
+
+        // The hash might be empty if LoadAll() failed and we're dealing with the defaults settings object.
+        // In that case we can just wait until the user fixed their settings or CascadiaSettings fixed
+        // itself and either will soon trigger a settings reload.
+        if (hash.empty() || hash == cachedHash)
+        {
+            return;
+        }
 
         Jumplist::UpdateJumplist(_settings);
+        applicationState.SettingsHash(hash);
     }
 
     // Method Description:
@@ -1058,8 +1082,7 @@ namespace winrt::TerminalApp::implementation
         _ApplyLanguageSettingChange();
         _RefreshThemeRoutine();
         _ApplyStartupTaskStateChange();
-
-        Jumplist::UpdateJumplist(_settings);
+        _ProcessLazySettingsChanges();
 
         _SettingsChangedHandlers(*this, nullptr);
     }

src/cascadia/TerminalApp/AppLogic.h

@@ -192,6 +192,7 @@ namespace winrt::TerminalApp::implementation
         void _OnLoaded(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs);
 
         [[nodiscard]] HRESULT _TryLoadSettings() noexcept;
+        void _ProcessLazySettingsChanges();
         void _RegisterSettingsChange();
         fire_and_forget _DispatchReloadSettings();
         void _ReloadSettings();

src/cascadia/TerminalSettingsModel/ApplicationState.h

@@ -35,6 +35,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
 // It provides X with the following arguments:
 //   (source, type, function name, JSON key, ...variadic construction arguments)
 #define MTSM_APPLICATION_STATE_FIELDS(X)                                                                                                                                  \
+    X(FileSource::Shared, winrt::hstring, SettingsHash, "settingsHash")                                                                                                   \
     X(FileSource::Shared, std::unordered_set<winrt::guid>, GeneratedProfiles, "generatedProfiles")                                                                        \
     X(FileSource::Local, Windows::Foundation::Collections::IVector<Model::WindowLayout>, PersistedWindowLayouts, "persistedWindowLayouts")                                \
     X(FileSource::Shared, Windows::Foundation::Collections::IVector<hstring>, RecentCommands, "recentCommands")                                                           \

src/cascadia/TerminalSettingsModel/ApplicationState.idl

@@ -33,13 +33,10 @@ namespace Microsoft.Terminal.Settings.Model
 
         Boolean IsStatePath(String filename);
 
-        Windows.Foundation.Collections.IVector<WindowLayout> PersistedWindowLayouts { get; set; };
-
-        Windows.Foundation.Collections.IVector<String> RecentCommands { get; set; };
-
-        Windows.Foundation.Collections.IVector<InfoBarMessage> DismissedMessages { get; set; };
-
-        Windows.Foundation.Collections.IVector<String> AllowedCommandlines { get; set; };
-
+        String SettingsHash;
+        Windows.Foundation.Collections.IVector<WindowLayout> PersistedWindowLayouts;
+        Windows.Foundation.Collections.IVector<String> RecentCommands;
+        Windows.Foundation.Collections.IVector<InfoBarMessage> DismissedMessages;
+        Windows.Foundation.Collections.IVector<String> AllowedCommandlines;
     }
 }

src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

@@ -43,6 +43,11 @@ winrt::com_ptr<Profile> Model::implementation::CreateChild(const winrt::com_ptr<
     return profile;
 }
 
+winrt::hstring CascadiaSettings::Hash() const noexcept
+{
+    return _hash;
+}
+
 Model::CascadiaSettings CascadiaSettings::Copy() const
 {
     const auto settings{ winrt::make_self<CascadiaSettings>() };

src/cascadia/TerminalSettingsModel/CascadiaSettings.h

@@ -113,12 +113,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         explicit CascadiaSettings(SettingsLoader&& loader);
 
         // user settings
+        winrt::hstring Hash() const noexcept;
         Model::CascadiaSettings Copy() const;
         Model::GlobalAppSettings GlobalSettings() const;
         winrt::Windows::Foundation::Collections::IObservableVector<Model::Profile> AllProfiles() const noexcept;
         winrt::Windows::Foundation::Collections::IObservableVector<Model::Profile> ActiveProfiles() const noexcept;
         Model::ActionMap ActionMap() const noexcept;
-        void WriteSettingsToDisk() const;
+        void WriteSettingsToDisk();
         Json::Value ToJson() const;
         Model::Profile ProfileDefaults() const;
         Model::Profile CreateNewProfile();
@@ -146,6 +147,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     private:
         static const std::filesystem::path& _settingsPath();
         static const std::filesystem::path& _releaseSettingsPath();
+        static winrt::hstring _calculateHash(std::string_view settings, const FILETIME& lastWriteTime);
 
         winrt::com_ptr<implementation::Profile> _createNewProfile(const std::wstring_view& name) const;
         Model::Profile _getProfileForCommandLine(const winrt::hstring& commandLine) const;
@@ -162,6 +164,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         void _validateThemeExists();
 
         // user settings
+        winrt::hstring _hash;
         winrt::com_ptr<implementation::GlobalAppSettings> _globals = winrt::make_self<implementation::GlobalAppSettings>();
         winrt::com_ptr<implementation::Profile> _baseLayerProfile = winrt::make_self<implementation::Profile>();
         winrt::Windows::Foundation::Collections::IObservableVector<Model::Profile> _allProfiles = winrt::single_threaded_observable_vector<Model::Profile>();

src/cascadia/TerminalSettingsModel/CascadiaSettings.idl

@@ -20,12 +20,14 @@ namespace Microsoft.Terminal.Settings.Model
         static String ApplicationVersion { get; };
 
         static void ExportFile(String path, String content);
-        
+
         CascadiaSettings(String userJSON, String inboxJSON);
 
         CascadiaSettings Copy();
         void WriteSettingsToDisk();
 
+        String Hash { get; };
+
         GlobalAppSettings GlobalSettings { get; };
 
         Profile ProfileDefaults { get; };

src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp

@@ -793,7 +793,8 @@ void SettingsLoader::_executeGenerator(const IDynamicProfileGenerator& generator
 Model::CascadiaSettings CascadiaSettings::LoadAll()
 try
 {
-    auto settingsString = ReadUTF8FileIfExists(_settingsPath()).value_or(std::string{});
+    FILETIME lastWriteTime{};
+    auto settingsString = ReadUTF8FileIfExists(_settingsPath(), false, &lastWriteTime).value_or(std::string{});
     auto firstTimeSetup = settingsString.empty();
 
     // If it's the firstTimeSetup and a preview build, then try to
@@ -874,6 +875,12 @@ try
             settings->_warnings.Append(SettingsLoadWarnings::FailedToWriteToSettings);
         }
     }
+    else
+    {
+        // lastWriteTime is only valid if mustWriteToDisk is false.
+        // Additionally WriteSettingsToDisk() updates the _hash for us already.
+        settings->_hash = _calculateHash(settingsString, lastWriteTime);
+    }
 
     return *settings;
 }
@@ -1015,6 +1022,15 @@ const std::filesystem::path& CascadiaSettings::_releaseSettingsPath()
     return path;
 }
 
+// Returns a has (approximately) uniquely identifying the settings.json contents on disk.
+winrt::hstring CascadiaSettings::_calculateHash(std::string_view settings, const FILETIME& lastWriteTime)
+{
+    const auto fileHash = til::hash(settings);
+    const ULARGE_INTEGER fileTime{ lastWriteTime.dwLowDateTime, lastWriteTime.dwHighDateTime };
+    const auto hash = fmt::format(L"{:016x}-{:016x}", fileHash, fileTime.QuadPart);
+    return winrt::hstring{ hash };
+}
+
 // function Description:
 // - Returns the full path to the settings file, either within the application
 //   package, or in its unpackaged location. This path is under the "Local
@@ -1058,7 +1074,7 @@ winrt::hstring CascadiaSettings::DefaultSettingsPath()
 // - <none>
 // Return Value:
 // - <none>
-void CascadiaSettings::WriteSettingsToDisk() const
+void CascadiaSettings::WriteSettingsToDisk()
 {
     const auto settingsPath = _settingsPath();
 
@@ -1067,8 +1083,11 @@ void CascadiaSettings::WriteSettingsToDisk() const
     wbuilder.settings_["indentation"] = "    ";
     wbuilder.settings_["enableYAMLCompatibility"] = true; // suppress spaces around colons
 
+    FILETIME lastWriteTime{};
     const auto styledString{ Json::writeString(wbuilder, ToJson()) };
-    WriteUTF8FileAtomic(settingsPath, styledString);
+    WriteUTF8FileAtomic(settingsPath, styledString, &lastWriteTime);
+
+    _hash = _calculateHash(styledString, lastWriteTime);
 
     // Persists the default terminal choice
     // GH#10003 - Only do this if _currentDefaultTerminal was actually initialized.

src/cascadia/TerminalSettingsModel/FileUtils.cpp

@@ -107,7 +107,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model
     }
     // Tries to read a file somewhat atomically without locking it.
     // Strips the UTF8 BOM if it exists.
-    std::string ReadUTF8File(const std::filesystem::path& path, const bool elevatedOnly)
+    std::string ReadUTF8File(const std::filesystem::path& path, const bool elevatedOnly, FILETIME* lastWriteTime)
     {
         // From some casual observations we can determine that:
         // * ReadFile() always returns the requested amount of data (unless the file is smaller)
@@ -179,18 +179,23 @@ namespace winrt::Microsoft::Terminal::Settings::Model
                 buffer.erase(0, Utf8Bom.size());
             }
 
+            if (lastWriteTime)
+            {
+                THROW_IF_WIN32_BOOL_FALSE(GetFileTime(file.get(), nullptr, nullptr, lastWriteTime));
+            }
+
             return buffer;
         }
 
         THROW_WIN32_MSG(ERROR_READ_FAULT, "file size changed while reading");
     }
 
     // Same as ReadUTF8File, but returns an empty optional, if the file couldn't be opened.
-    std::optional<std::string> ReadUTF8FileIfExists(const std::filesystem::path& path, const bool elevatedOnly)
+    std::optional<std::string> ReadUTF8FileIfExists(const std::filesystem::path& path, const bool elevatedOnly, FILETIME* lastWriteTime)
     {
         try
         {
-            return { ReadUTF8File(path, elevatedOnly) };
+            return { ReadUTF8File(path, elevatedOnly, lastWriteTime) };
         }
         catch (const wil::ResultException& exception)
         {
@@ -203,9 +208,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model
         }
     }
 
-    void WriteUTF8File(const std::filesystem::path& path,
-                       const std::string_view& content,
-                       const bool elevatedOnly)
+    void WriteUTF8File(const std::filesystem::path& path, const std::string_view& content, const bool elevatedOnly, FILETIME* lastWriteTime)
     {
         SECURITY_ATTRIBUTES sa;
         // stash the security descriptor here, so it will stay in context until
@@ -277,10 +280,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model
         {
             THROW_WIN32_MSG(ERROR_WRITE_FAULT, "failed to write whole file");
         }
+
+        if (lastWriteTime)
+        {
+            THROW_IF_WIN32_BOOL_FALSE(GetFileTime(file.get(), nullptr, nullptr, lastWriteTime));
+        }
     }
 
-    void WriteUTF8FileAtomic(const std::filesystem::path& path,
-                             const std::string_view& content)
+    void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view& content, FILETIME* lastWriteTime)
     {
         // GH#10787: rename() will replace symbolic links themselves and not the path they point at.
         // It's thus important that we first resolve them before generating temporary path.
@@ -298,15 +305,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model
             //   * resolve the link manually, which might be less accurate and more prone to race conditions
             //   * write to the file directly, which lets the system resolve the symbolic link but leaves the write non-atomic
             // The latter is chosen, as this is an edge case and our 'atomic' writes are only best-effort.
-            WriteUTF8File(path, content);
+            WriteUTF8File(path, content, false, lastWriteTime);
             return;
         }
 
         auto tmpPath = resolvedPath;
         tmpPath += L".tmp";
 
         // Writing to a file isn't atomic, but...
-        WriteUTF8File(tmpPath, content);
+        WriteUTF8File(tmpPath, content, false, lastWriteTime);
 
         // renaming one is (supposed to be) atomic.
         // Wait... "supposed to be"!? Well it's technically not always atomic,

src/cascadia/TerminalSettingsModel/FileUtils.h

@@ -5,8 +5,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model
 {
     std::filesystem::path GetBaseSettingsPath();
     std::filesystem::path GetReleaseSettingsPath();
-    std::string ReadUTF8File(const std::filesystem::path& path, const bool elevatedOnly = false);
-    std::optional<std::string> ReadUTF8FileIfExists(const std::filesystem::path& path, const bool elevatedOnly = false);
-    void WriteUTF8File(const std::filesystem::path& path, const std::string_view& content, const bool elevatedOnly = false);
-    void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view& content);
+    std::string ReadUTF8File(const std::filesystem::path& path, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr);
+    std::optional<std::string> ReadUTF8FileIfExists(const std::filesystem::path& path, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr);
+    void WriteUTF8File(const std::filesystem::path& path, const std::string_view& content, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr);
+    void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view& content, FILETIME* lastWriteTime = nullptr);
 }