Fix AppInitialized latency metric

microsoft · Apr 20, 2023 · a248d8b · a248d8b
1 parent 210414e
commit a248d8b
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 67 deletions.
diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp
@@ -212,45 +212,7 @@ namespace winrt::TerminalApp::implementation
             _settings.GlobalSettings().ShowTabsInTitlebar(false);
         }
 
-        // These used to be in `TerminalPage::Initialized`, so that they started
-        // _after_ the Terminal window was started and displayed. These could
-        // theoretically move there again too. TODO:GH#14957 - evaluate moving
-        // this after the Page is initialized
-        {
-            // 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();
-
-            FILETIME creationTime, exitTime, kernelTime, userTime, now;
-            if (GetThreadTimes(GetCurrentThread(), &creationTime, &exitTime, &kernelTime, &userTime))
-            {
-                static constexpr auto asInteger = [](const FILETIME& f) {
-                    ULARGE_INTEGER i;
-                    i.LowPart = f.dwLowDateTime;
-                    i.HighPart = f.dwHighDateTime;
-                    return i.QuadPart;
-                };
-                static constexpr auto asSeconds = [](uint64_t v) {
-                    return v * 1e-7f;
-                };
-
-                GetSystemTimeAsFileTime(&now);
-
-                const auto latency = asSeconds(asInteger(now) - asInteger(creationTime));
-
-                TraceLoggingWrite(
-                    g_hTerminalAppProvider,
-                    "AppInitialized",
-                    TraceLoggingDescription("Event emitted once the app is initialized"),
-                    TraceLoggingFloat32(latency, "latency"),
-                    TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
-                    TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
-            }
-        }
-
         _ApplyLanguageSettingChange();
-        _ApplyStartupTaskStateChange();
 
         TraceLoggingWrite(
             g_hTerminalAppProvider,
@@ -423,33 +385,12 @@ namespace winrt::TerminalApp::implementation
     }
     CATCH_LOG()
 
-    // Function Description:
-    // Returns the current app package or nullptr.
-    // TRANSITIONAL
-    // Exists to work around a compiler bug. This function encapsulates the
-    // exception handling that we used to keep around calls to Package::Current,
-    // so that when it's called inside a coroutine and fails it doesn't explode
-    // terribly.
-    static winrt::Windows::ApplicationModel::Package GetCurrentPackageNoThrow() noexcept
-    {
-        try
-        {
-            return winrt::Windows::ApplicationModel::Package::Current();
-        }
-        catch (...)
-        {
-            // discard any exception -- literally pretend we're not in a package
-        }
-        return nullptr;
-    }
-
     fire_and_forget AppLogic::_ApplyStartupTaskStateChange()
     try
     {
         // First, make sure we're running in a packaged context. This method
         // won't work, and will crash mysteriously if we're running unpackaged.
-        const auto package{ GetCurrentPackageNoThrow() };
-        if (package == nullptr)
+        if (!IsPackaged())
         {
             co_return;
         }
@@ -549,6 +490,47 @@ namespace winrt::TerminalApp::implementation
         _SettingsChangedHandlers(*this, *ev);
     }
 
+    // This is a continuation of AppLogic::Create() and includes the more expensive parts.
+    void AppLogic::NotifyRootInitialized()
+    {
+        if (_notifyRootInitializedCalled.exchange(true, std::memory_order_relaxed))
+        {
+            return;
+        }
+
+        // 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.
+        _ApplyStartupTaskStateChange();
+        _ProcessLazySettingsChanges();
+
+        FILETIME creationTime, exitTime, kernelTime, userTime, now;
+        if (GetThreadTimes(GetCurrentThread(), &creationTime, &exitTime, &kernelTime, &userTime))
+        {
+            static constexpr auto asInteger = [](const FILETIME& f) {
+                ULARGE_INTEGER i;
+                i.LowPart = f.dwLowDateTime;
+                i.HighPart = f.dwHighDateTime;
+                return i.QuadPart;
+            };
+            static constexpr auto asSeconds = [](uint64_t v) {
+                return v * 1e-7f;
+            };
+
+            GetSystemTimeAsFileTime(&now);
+
+            const auto latency = asSeconds(asInteger(now) - asInteger(creationTime));
+
+            TraceLoggingWrite(
+                g_hTerminalAppProvider,
+                "AppInitialized",
+                TraceLoggingDescription("Event emitted once the app is initialized"),
+                TraceLoggingFloat32(latency, "latency"),
+                TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
+                TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
+        }
+    }
+
     // Method Description:
     // - Returns a pointer to the global shared settings.
     [[nodiscard]] CascadiaSettings AppLogic::GetSettings() const noexcept

diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h
@@ -45,14 +45,14 @@ namespace winrt::TerminalApp::implementation
         static const Microsoft::Terminal::Settings::Model::CascadiaSettings CurrentAppSettings();
 
         AppLogic();
-        ~AppLogic() = default;
 
         void Create();
         bool IsUwp() const noexcept;
         void RunAsUwp();
         bool IsRunningElevated() const noexcept;
         bool CanDragDrop() const noexcept;
         void ReloadSettings();
+        void NotifyRootInitialized();
 
         bool HasSettingsStartupActions() const noexcept;
 
@@ -82,6 +82,7 @@ namespace winrt::TerminalApp::implementation
         bool _isUwp{ false };
         bool _isElevated{ false };
         bool _canDragDrop{ false };
+        std::atomic<bool> _notifyRootInitializedCalled{ false };
 
         Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };
 

diff --git a/src/cascadia/TerminalApp/TerminalWindow.cpp b/src/cascadia/TerminalApp/TerminalWindow.cpp
@@ -3,16 +3,15 @@
 
 #include "pch.h"
 #include "TerminalWindow.h"
+
+#include "AppLogic.h"
 #include "../inc/WindowingBehavior.h"
-#include "TerminalWindow.g.cpp"
-#include "SettingsLoadEventArgs.g.cpp"
-#include "WindowProperties.g.cpp"
 
 #include <LibraryResources.h>
-#include <WtExeUtils.h>
-#include <wil/token_helpers.h>
 
-#include "../../types/inc/utils.hpp"
+#include "TerminalWindow.g.cpp"
+#include "SettingsLoadEventArgs.g.cpp"
+#include "WindowProperties.g.cpp"
 
 using namespace winrt::Windows::ApplicationModel;
 using namespace winrt::Windows::ApplicationModel::DataTransfer;
@@ -264,6 +263,8 @@ namespace winrt::TerminalApp::implementation
             {
                 _root->SetFullscreen(true);
             }
+
+            AppLogic::Current()->NotifyRootInitialized();
         });
         _root->Create();