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()
+                    control->_isInternalScrollBarUpdate = true;
-                        .RunAsync(CoreDispatcherPriority::Normal, [=]() {
-                            if (auto control2{ weakThis.get() })
-                            {
-                                control2->_isInternalScrollBarUpdate = true;
 
-                                auto scrollBar = control2->ScrollBar();
+                    auto scrollBar = control->ScrollBar();
-                                if (update.newValue.has_value())
+                    if (update.newValue.has_value())
-                                {
+                    {
-                                    scrollBar.Value(update.newValue.value());
+                        scrollBar.Value(update.newValue.value());
-                                }
+                    }
-                                scrollBar.Maximum(update.newMaximum);
+                    scrollBar.Maximum(update.newMaximum);
-                                scrollBar.Minimum(update.newMinimum);
+                    scrollBar.Minimum(update.newMinimum);
-                                scrollBar.ViewportSize(update.newViewportSize);
+                    scrollBar.ViewportSize(update.newViewportSize);
 
-                                control2->_isInternalScrollBarUpdate = false;
+                    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 };
+        _tsfTryRedrawCanvas->Run();
-        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);
-        }
     }
 
     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,6 +19,7 @@
     <ClCompile Include="UiaTextRange.cpp" />
     <ClCompile Include="SearchBoxControl.cpp" />
     <ClCompile Include="init.cpp" />
+    <ClCompile Include="ThrottledFunc.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="pch.h" />
@@ -26,7 +27,6 @@
     <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/ThreadSafeOptional.h

@@ -1,54 +0,0 @@
-// Copyright (c) Microsoft Corporation.
-// Licensed under the MIT license.
-
-#pragma once
-#include "pch.h"
-
-template<typename T>
-class ThreadSafeOptional
-{
-public:
-    template<class... Args>
-    bool Emplace(Args&&... args)
-    {
-        std::lock_guard guard{ _lock };
-
-        bool hadValue = _inner.has_value();
-        _inner.emplace(std::forward<Args>(args)...);
-        return !hadValue;
-    }
-
-    std::optional<T> Take()
-    {
-        std::lock_guard guard{ _lock };
-
-        std::optional<T> value;
-        _inner.swap(value);
-
-        return value;
-    }
-
-    // Method Description:
-    // - If the optional has a value, then call the specified function with a
-    //   reference to the value.
-    // - This method is always thread-safe. It can be called multiple times on
-    //   different threads.
-    // Arguments:
-    // - f: the function to call with a reference to the value
-    // Return Value:
-    // - <none>
-    template<typename F>
-    void ModifyValue(F f)
-    {
-        std::lock_guard guard{ _lock };
-
-        if (_inner.has_value())
-        {
-            f(_inner.value());
-        }
-    }
-
-private:
-    std::mutex _lock;
-    std::optional<T> _inner;
-};

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();
+        }
+    });
+}

src/cascadia/TerminalControl/ThrottledFunc.h

@@ -4,94 +4,139 @@ Licensed under the MIT license.
 
 Module Name:
 - ThrottledFunc.h
-
-Abstract:
-- This module defines a class to throttle function calls.
-- You create an instance of a `ThrottledFunc` with a function and the delay
-  between two function calls.
-- The function takes an argument of type `T`, the template argument of
-  `ThrottledFunc`.
-- Use the `Run` method to wait and then call the function.
 --*/
 
 #pragma once
 #include "pch.h"
 
-#include "ThreadSafeOptional.h"
+// Class Description:
-
+// - Represents a function that takes arguments and whose invocation is
-template<typename T>
+//   delayed by a specified duration and rate-limited such that if the code
-class ThrottledFunc : public std::enable_shared_from_this<ThrottledFunc<T>>
+//   tries to run the function while a call to the function is already
+//   pending, then the previous call with the previous arguments will be
+//   cancelled and the call will be made with the new arguments instead.
+// - The function will be run on the the specified dispatcher.
+template<typename... Args>
+class ThrottledFunc : public std::enable_shared_from_this<ThrottledFunc<Args...>>
 {
 public:
-    using Func = std::function<void(T arg)>;
+    using Func = std::function<void(Args...)>;
 
-    ThrottledFunc(Func func, winrt::Windows::Foundation::TimeSpan delay) :
+    ThrottledFunc(Func func, winrt::Windows::Foundation::TimeSpan delay, winrt::Windows::UI::Core::CoreDispatcher dispatcher) :
         _func{ func },
-        _delay{ delay }
+        _delay{ delay },
+        _dispatcher{ dispatcher }
     {
     }
 
     // Method Description:
-    // - Runs the function later with the specified argument, except if `Run`
+    // - Runs the function later with the specified arguments, except if `Run`
-    //   is called again before with a new argument, in which case the new
+    //   is called again before with new arguments, in which case the new
-    //   argument will be instead.
+    //   arguments will be used instead.
-    // - For more information, read the "Abstract" section in the header file.
+    // - For more information, read the class' documentation.
+    // - This method is always thread-safe. It can be called multiple times on
+    //   different threads.
     // Arguments:
     // - arg: the argument to pass to the function
     // Return Value:
     // - <none>
-    winrt::fire_and_forget Run(T arg)
+    template<typename... MakeArgs>
+    void Run(MakeArgs&&... args)
     {
-        if (!_pendingCallArg.Emplace(arg))
         {
-            // already pending
+            std::lock_guard guard{ _lock };
-            return;
-        }
 
-        auto weakThis = this->weak_from_this();
+            bool hadValue = _pendingRunArgs.has_value();
+            _pendingRunArgs.emplace(std::forward<MakeArgs>(args)...);
 
-        co_await winrt::resume_after(_delay);
+            if (hadValue)
-
-        if (auto self{ weakThis.lock() })
-        {
-            auto arg = self->_pendingCallArg.Take();
-            if (arg.has_value())
             {
-                self->_func(arg.value());
+                // already pending
-            }
+                return;
-            else
-            {
-                // should not happen
             }
         }
+
+        _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();
+
+                        std::optional<std::tuple<Args...>> args;
+                        {
+                            std::lock_guard guard{ self->_lock };
+                            self->_pendingRunArgs.swap(args);
+                        }
+                        std::apply(self->_func, args.value());
+                    }
+                });
+                timer.Start();
+            }
+        });
     }
 
     // Method Description:
-    // - Modifies the pending argument for the next function invocation, if
+    // - Modifies the pending arguments for the next function invocation, if
     //   there is one pending currently.
-    // - Let's say that you just called the `Run` method with argument A.
+    // - Let's say that you just called the `Run` method with some arguments.
-    //   After the delay specified in the constructor, the function R
+    //   After the delay specified in the constructor, the function specified
-    //   specified in the constructor will be called with argument A.
+    //   in the constructor will be called with these arguments.
-    // - By using this method, you can modify argument A before the function R
+    // - By using this method, you can modify the arguments before the function
-    //   is called with argument A.
+    //   is called.
-    // - You pass a function to this method which will take a reference to
+    // - You pass a function to this method which will take references to
-    //   argument A and will modify it.
+    //   the arguments (one argument corresponds to one reference to an
-    // - When there is no pending invocation of function R, this method will
+    //   argument) and will modify them.
+    // - When there is no pending invocation of the function, this method will
     //   not do anything.
     // - This method is always thread-safe. It can be called multiple times on
     //   different threads.
     // Arguments:
-    // - f: the function to call with a reference to the argument
+    // - f: the function to call with references to the arguments
     // Return Value:
     // - <none>
     template<typename F>
     void ModifyPending(F f)
     {
-        _pendingCallArg.ModifyValue(f);
+        std::lock_guard guard{ _lock };
+
+        if (_pendingRunArgs.has_value())
+        {
+            std::apply(f, _pendingRunArgs.value());
+        }
     }
 
 private:
     Func _func;
     winrt::Windows::Foundation::TimeSpan _delay;
-    ThreadSafeOptional<T> _pendingCallArg;
+    winrt::Windows::UI::Core::CoreDispatcher _dispatcher;
+
+    std::optional<std::tuple<Args...>> _pendingRunArgs;
+    std::mutex _lock;
+};
+
+// Class Description:
+// - Represents a function whose invocation is delayed by a specified duration
+//   and rate-limited such that if the code tries to run the function while a
+//   call to the function is already pending, the request will be ignored.
+// - The function will be run on the the specified dispatcher.
+template<>
+class ThrottledFunc<> : public std::enable_shared_from_this<ThrottledFunc<>>
+{
+public:
+    using Func = std::function<void()>;
+
+    ThrottledFunc(Func func, winrt::Windows::Foundation::TimeSpan delay, winrt::Windows::UI::Core::CoreDispatcher dispatcher);
+
+    void Run();
+
+private:
+    Func _func;
+    winrt::Windows::Foundation::TimeSpan _delay;
+    winrt::Windows::UI::Core::CoreDispatcher _dispatcher;
+
+    std::atomic_flag _isRunPending;
 };