Update _TerminalCursorPositionChanged to use ThrottledFunc (#6492)

* Update _TerminalCursorPositionChanged to use ThrottledFunc.
* Rename previous ThrottledFunc to ThrottledArgFunc because now
  ThrottledFunc is for functions that do not take an argument.
* Update ThrottledFunc and ThrottledArgFunc to accept a CoreDispatcher
  on which the function should be called for convenience.
* Don't use coroutines/winrt::fire_and_forget in
  ThrottledFunc/ThrottledArgFunc because they are too slow (see PR).

_AdjustCursorPosition went from 17% of samples to 3% in performance
testing.

src/cascadia/TerminalControl/TermControl.cpp

@@ -31,6 +31,9 @@ using namespace winrt::Windows::ApplicationModel::DataTransfer;
 // The updates are throttled to limit power usage.
 constexpr const auto ScrollBarUpdateInterval = std::chrono::milliseconds(8);
 
+// The minimum delay between updating the TSF input control.
+constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(100);
+
 namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 {
     // Helper static function to ensure that all ambiguous-width glyphs are reported as narrow.
@@ -124,31 +127,36 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
             }
         });
 
+        _tsfTryRedrawCanvas = std::make_shared<ThrottledFunc<>>(
+            [weakThis = get_weak()]() {
+                if (auto control{ weakThis.get() })
+                {
+                    control->TSFInputControl().TryRedrawCanvas();
+                }
+            },
+            TsfRedrawInterval,
+            Dispatcher());
+
         _updateScrollBar = std::make_shared<ThrottledFunc<ScrollBarUpdate>>(
             [weakThis = get_weak()](const auto& update) {
                 if (auto control{ weakThis.get() })
                 {
-                    control->Dispatcher()
-                        .RunAsync(CoreDispatcherPriority::Normal, [=]() {
-                            if (auto control2{ weakThis.get() })
-                            {
-                                control2->_isInternalScrollBarUpdate = true;
-
-                                auto scrollBar = control2->ScrollBar();
-                                if (update.newValue.has_value())
-                                {
-                                    scrollBar.Value(update.newValue.value());
-                                }
-                                scrollBar.Maximum(update.newMaximum);
-                                scrollBar.Minimum(update.newMinimum);
-                                scrollBar.ViewportSize(update.newViewportSize);
-
-                                control2->_isInternalScrollBarUpdate = false;
-                            }
-                        });
+                    control->_isInternalScrollBarUpdate = true;
+
+                    auto scrollBar = control->ScrollBar();
+                    if (update.newValue.has_value())
+                    {
+                        scrollBar.Value(update.newValue.value());
+                    }
+                    scrollBar.Maximum(update.newMaximum);
+                    scrollBar.Minimum(update.newMinimum);
+                    scrollBar.ViewportSize(update.newViewportSize);
+
+                    control->_isInternalScrollBarUpdate = false;
                 }
             },
-            ScrollBarUpdateInterval);
+            ScrollBarUpdateInterval,
+            Dispatcher());
 
         static constexpr auto AutoScrollUpdateInterval = std::chrono::microseconds(static_cast<int>(1.0 / 30.0 * 1000000));
         _autoScrollTimer.Interval(AutoScrollUpdateInterval);
@@ -2047,42 +2055,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
     //   to be where the current cursor position is.
     // Arguments:
     // - N/A
-    winrt::fire_and_forget TermControl::_TerminalCursorPositionChanged()
+    void TermControl::_TerminalCursorPositionChanged()
     {
-        bool expectedFalse{ false };
-        if (!_coroutineDispatchStateUpdateInProgress.compare_exchange_weak(expectedFalse, true))
-        {
-            // somebody's already in here.
-            return;
-        }
-
-        if (_closing.load())
-        {
-            return;
-        }
-
-        auto dispatcher{ Dispatcher() }; // cache a strong ref to this in case TermControl dies
-        auto weakThis{ get_weak() };
-
-        // Muffle 2: Muffle Harder
-        // If we're the lucky coroutine who gets through, we'll still wait 100ms to clog
-        // the atomic above so we don't service the cursor update too fast. If we get through
-        // and finish processing the update quickly but similar requests are still beating
-        // down the door above in the atomic, we may still update the cursor way more than
-        // is visible to anyone's eye, which is a waste of effort.
-        static constexpr auto CursorUpdateQuiesceTime{ std::chrono::milliseconds(100) };
-        co_await winrt::resume_after(CursorUpdateQuiesceTime);
-
-        co_await winrt::resume_foreground(dispatcher);
-
-        if (auto control{ weakThis.get() })
-        {
-            if (!_closing.load())
-            {
-                TSFInputControl().TryRedrawCanvas();
-            }
-            _coroutineDispatchStateUpdateInProgress.store(false);
-        }
+        _tsfTryRedrawCanvas->Run();
     }
 
     hstring TermControl::Title()

src/cascadia/TerminalControl/TermControl.h

@@ -136,6 +136,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         FontInfoDesired _desiredFont;
         FontInfo _actualFont;
 
+        std::shared_ptr<ThrottledFunc<>> _tsfTryRedrawCanvas;
+
         struct ScrollBarUpdate
         {
             std::optional<double> newValue;
@@ -210,7 +212,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         void _RefreshSizeUnderLock();
         void _TerminalTitleChanged(const std::wstring_view& wstr);
         void _TerminalScrollPositionChanged(const int viewTop, const int viewHeight, const int bufferSize);
-        winrt::fire_and_forget _TerminalCursorPositionChanged();
+        void _TerminalCursorPositionChanged();
 
         void _MouseScrollHandler(const double mouseDelta, const Windows::Foundation::Point point, const bool isLeftButtonPressed);
         void _MouseZoomHandler(const double delta);
@@ -246,12 +248,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         void _FontInfoHandler(const IInspectable& sender, const FontInfoEventArgs& eventArgs);
 
         winrt::fire_and_forget _AsyncCloseConnection();
-
-        // this atomic is to be used as a guard against dispatching billions of coroutines for
-        // routine state changes that might happen millions of times a second.
-        // Unbounded main dispatcher use leads to massive memory leaks and intense slowdowns
-        // on the UI thread.
-        std::atomic<bool> _coroutineDispatchStateUpdateInProgress{ false };
     };
 }
 

src/cascadia/TerminalControl/TerminalControl.vcxproj

@@ -43,7 +43,6 @@
     <ClInclude Include="TermControlAutomationPeer.h">
       <DependentUpon>TermControlAutomationPeer.idl</DependentUpon>
     </ClInclude>
-    <ClInclude Include="ThreadSafeOptional.h" />
     <ClInclude Include="ThrottledFunc.h" />
     <ClInclude Include="TSFInputControl.h">
       <DependentUpon>TSFInputControl.xaml</DependentUpon>
@@ -61,6 +60,7 @@
     <ClCompile Include="TermControl.cpp">
       <DependentUpon>TermControl.xaml</DependentUpon>
     </ClCompile>
+    <ClCompile Include="ThrottledFunc.cpp" />
     <ClCompile Include="TSFInputControl.cpp">
       <DependentUpon>TSFInputControl.xaml</DependentUpon>
     </ClCompile>

src/cascadia/TerminalControl/TerminalControl.vcxproj.filters

@@ -19,14 +19,14 @@
     <ClCompile Include="UiaTextRange.cpp" />
     <ClCompile Include="SearchBoxControl.cpp" />
     <ClCompile Include="init.cpp" />
+    <ClCompile Include="ThrottledFunc.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="pch.h" />
     <ClInclude Include="TermControl.h" />
     <ClInclude Include="TermControlAutomationPeer.h" />
     <ClInclude Include="XamlUiaTextRange.h" />
     <ClInclude Include="TermControlUiaProvider.hpp" />
-    <ClInclude Include="ThreadSafeOptional.h" />
     <ClInclude Include="ThrottledFunc.h" />
     <ClInclude Include="UiaTextRange.hpp" />
   </ItemGroup>

src/cascadia/TerminalControl/ThrottledFunc.cpp

@@ -0,0 +1,54 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#include "pch.h"
+
+#include "ThrottledFunc.h"
+
+using namespace winrt::Windows::Foundation;
+using namespace winrt::Windows::UI::Core;
+using namespace winrt::Windows::UI::Xaml;
+
+ThrottledFunc<>::ThrottledFunc(ThrottledFunc::Func func, TimeSpan delay, CoreDispatcher dispatcher) :
+    _func{ func },
+    _delay{ delay },
+    _dispatcher{ dispatcher },
+    _isRunPending{}
+{
+}
+
+// Method Description:
+// - Runs the function later, except if `Run` is called again before
+//   with a new argument, in which case the request will be ignored.
+// - For more information, read the class' documentation.
+// - This method is always thread-safe. It can be called multiple times on
+//   different threads.
+// Arguments:
+// - <none>
+// Return Value:
+// - <none>
+void ThrottledFunc<>::Run()
+{
+    if (_isRunPending.test_and_set())
+    {
+        // already pending
+        return;
+    }
+
+    _dispatcher.RunAsync(CoreDispatcherPriority::Low, [weakThis = this->weak_from_this()]() {
+        if (auto self{ weakThis.lock() })
+        {
+            DispatcherTimer timer;
+            timer.Interval(self->_delay);
+            timer.Tick([=](auto&&...) {
+                if (auto self{ weakThis.lock() })
+                {
+                    timer.Stop();
+                    self->_isRunPending.clear();
+                    self->_func();
+                }
+            });
+            timer.Start();
+        }
+    });
+}