Skip to content

Commit

Permalink
Improve conhost's scrolling performance (#16333)
Browse files Browse the repository at this point in the history
`EnableScrollbar()` and especially `SetScrollInfo()` are prohibitively
expensive functions nowadays. This improves throughput of good old
`type` in cmd.exe by ~10x, by briefly releasing the console lock.

## Validation Steps Performed
* `type`ing a file in `cmd` is as fast while the window is scrolling
  as it is while it isn't scrolling ✅
* Scrollbar pops in and out when scroll-forward is disabled ✅

(cherry picked from commit 71a6f26)
Service-Card-Id: 91152166
Service-Version: 1.19
  • Loading branch information
lhecker authored and DHowett committed Jan 29, 2024
1 parent 04edb11 commit 29895e1
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 101 deletions.
5 changes: 5 additions & 0 deletions src/host/consoleInformation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ void CONSOLE_INFORMATION::UnlockConsole() noexcept
_lock.unlock();
}

til::recursive_ticket_lock_suspension CONSOLE_INFORMATION::SuspendLock() noexcept
{
return _lock.suspend();
}

ULONG CONSOLE_INFORMATION::GetCSRecursionCount() const noexcept
{
return _lock.recursion_depth();
Expand Down
6 changes: 4 additions & 2 deletions src/host/renderData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,16 @@ std::vector<Viewport> RenderData::GetSelectionRects() noexcept
// they're done with any querying they need to do.
void RenderData::LockConsole() noexcept
{
::LockConsole();
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
gci.LockConsole();
}

// Method Description:
// - Unlocks the console after a call to RenderData::LockConsole.
void RenderData::UnlockConsole() noexcept
{
::UnlockConsole();
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
gci.UnlockConsole();
}

// Method Description:
Expand Down
54 changes: 12 additions & 42 deletions src/host/screenInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ SCREEN_INFORMATION::SCREEN_INFORMATION(
const TextAttribute popupAttributes,
const FontInfo fontInfo) :
OutputMode{ ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT },
ResizingWindow{ 0 },
WheelDelta{ 0 },
HWheelDelta{ 0 },
_textBuffer{ nullptr },
Expand Down Expand Up @@ -641,64 +640,35 @@ VOID SCREEN_INFORMATION::UpdateScrollBars()
return;
}

if (gci.Flags & CONSOLE_UPDATING_SCROLL_BARS)
if (gci.Flags & CONSOLE_UPDATING_SCROLL_BARS || ServiceLocator::LocateConsoleWindow() == nullptr)
{
return;
}

gci.Flags |= CONSOLE_UPDATING_SCROLL_BARS;

if (ServiceLocator::LocateConsoleWindow() != nullptr)
{
ServiceLocator::LocateConsoleWindow()->PostUpdateScrollBars();
}
LOG_IF_WIN32_BOOL_FALSE(ServiceLocator::LocateConsoleWindow()->PostUpdateScrollBars());
}

VOID SCREEN_INFORMATION::InternalUpdateScrollBars()
SCREEN_INFORMATION::ScrollBarState SCREEN_INFORMATION::FetchScrollBarState()
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
const auto pWindow = ServiceLocator::LocateConsoleWindow();

WI_ClearFlag(gci.Flags, CONSOLE_UPDATING_SCROLL_BARS);

if (!IsActiveScreenBuffer())
{
return;
}

ResizingWindow++;

if (pWindow != nullptr)
{
const auto buffer = GetBufferSize();

// If this is the main buffer, make sure we enable both of the scroll bars.
// The alt buffer likely disabled the scroll bars, this is the only
// way to re-enable it.
if (!_IsAltBuffer())
{
pWindow->EnableBothScrollBars();
}

pWindow->UpdateScrollBar(true,
_IsAltBuffer(),
_viewport.Height(),
gci.IsTerminalScrolling() ? _virtualBottom : buffer.BottomInclusive(),
_viewport.Top());
pWindow->UpdateScrollBar(false,
_IsAltBuffer(),
_viewport.Width(),
buffer.RightInclusive(),
_viewport.Left());
}

// Fire off an event to let accessibility apps know the layout has changed.
if (_pAccessibilityNotifier)
{
_pAccessibilityNotifier->NotifyConsoleLayoutEvent();
}

ResizingWindow--;
const auto buffer = GetBufferSize();
const auto isAltBuffer = _IsAltBuffer();
const auto maxSizeVer = gci.IsTerminalScrolling() ? _virtualBottom : buffer.BottomInclusive();
const auto maxSizeHor = buffer.RightInclusive();
return ScrollBarState{
.maxSize = { maxSizeHor, maxSizeVer },
.viewport = _viewport.ToExclusive(),
.isAltBuffer = isAltBuffer,
};
}

// Routine Description:
Expand Down
9 changes: 7 additions & 2 deletions src/host/screenInfo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,14 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
bool HasAccessibilityEventing() const noexcept;
void NotifyAccessibilityEventing(const til::CoordType sStartX, const til::CoordType sStartY, const til::CoordType sEndX, const til::CoordType sEndY);

struct ScrollBarState
{
til::size maxSize;
til::rect viewport;
bool isAltBuffer = false;
};
void UpdateScrollBars();
void InternalUpdateScrollBars();
ScrollBarState FetchScrollBarState();

bool IsMaximizedBoth() const;
bool IsMaximizedX() const;
Expand Down Expand Up @@ -158,7 +164,6 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
bool CursorIsDoubleWidth() const;

DWORD OutputMode;
WORD ResizingWindow; // > 0 if we should ignore WM_SIZE messages

short WheelDelta;
short HWheelDelta;
Expand Down
1 change: 1 addition & 0 deletions src/host/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ class CONSOLE_INFORMATION :

void LockConsole() noexcept;
void UnlockConsole() noexcept;
til::recursive_ticket_lock_suspension SuspendLock() noexcept;
bool IsConsoleLocked() const noexcept;
ULONG GetCSRecursionCount() const noexcept;

Expand Down
7 changes: 0 additions & 7 deletions src/interactivity/inc/IConsoleWindow.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,6 @@ namespace Microsoft::Console::Types
public:
virtual ~IConsoleWindow() = default;

virtual BOOL EnableBothScrollBars() = 0;
virtual int UpdateScrollBar(_In_ bool isVertical,
_In_ bool isAltBuffer,
_In_ UINT pageSize,
_In_ int maxSize,
_In_ int viewportPosition) = 0;

virtual bool IsInFullscreen() const = 0;

virtual void SetIsFullscreen(const bool fFullscreenEnabled) = 0;
Expand Down
14 changes: 0 additions & 14 deletions src/interactivity/onecore/ConsoleWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,6 @@
using namespace Microsoft::Console::Interactivity::OneCore;
using namespace Microsoft::Console::Types;

BOOL ConsoleWindow::EnableBothScrollBars() noexcept
{
return FALSE;
}

int ConsoleWindow::UpdateScrollBar(bool /*isVertical*/,
bool /*isAltBuffer*/,
UINT /*pageSize*/,
int /*maxSize*/,
int /*viewportPosition*/) noexcept
{
return 0;
}

bool ConsoleWindow::IsInFullscreen() const noexcept
{
return true;
Expand Down
3 changes: 0 additions & 3 deletions src/interactivity/onecore/ConsoleWindow.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ namespace Microsoft::Console::Interactivity::OneCore
{
public:
// Inherited via IConsoleWindow
BOOL EnableBothScrollBars() noexcept override;
int UpdateScrollBar(bool isVertical, bool isAltBuffer, UINT pageSize, int maxSize, int viewportPosition) noexcept override;

bool IsInFullscreen() const noexcept override;
void SetIsFullscreen(const bool fFullscreenEnabled) noexcept override;
void ChangeViewport(const til::inclusive_rect& NewWindow) override;
Expand Down
49 changes: 26 additions & 23 deletions src/interactivity/win32/window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ void Window::_CloseWindow() const
ShowWindow(hWnd, wShowWindow);

auto& siAttached = GetScreenInfo();
siAttached.InternalUpdateScrollBars();
siAttached.UpdateScrollBars();
}

return status;
Expand Down Expand Up @@ -591,7 +591,7 @@ void Window::_UpdateWindowSize(const til::size sizeNew)

if (WI_IsFlagClear(gci.Flags, CONSOLE_IS_ICONIC))
{
ScreenInfo.InternalUpdateScrollBars();
ScreenInfo.UpdateScrollBars();

SetWindowPos(GetWindowHandle(),
nullptr,
Expand Down Expand Up @@ -621,7 +621,7 @@ void Window::_UpdateWindowSize(const til::size sizeNew)
if (!IsInFullscreen() && !IsInMaximized())
{
// Figure out how big to make the window, given the desired client area size.
siAttached.ResizingWindow++;
_resizingWindow++;

// First get the buffer viewport size
const auto WindowDimensions = siAttached.GetViewport().Dimensions();
Expand Down Expand Up @@ -691,7 +691,7 @@ void Window::_UpdateWindowSize(const til::size sizeNew)
// If the change wasn't substantial, we may still need to update scrollbar positions. Note that PSReadLine
// scrolls the window via Console.SetWindowPosition, which ultimately calls down to SetConsoleWindowInfo,
// which ends up in this function.
siAttached.InternalUpdateScrollBars();
siAttached.UpdateScrollBars();
}

// MSFT: 12092729
Expand All @@ -716,7 +716,7 @@ void Window::_UpdateWindowSize(const til::size sizeNew)
// an additional Buffer message with the same size again and do nothing special.
ScreenBufferSizeChange(siAttached.GetActiveBuffer().GetBufferSize().Dimensions());

siAttached.ResizingWindow--;
_resizingWindow--;
}

LOG_IF_FAILED(ConsoleImeResizeCompStrView());
Expand Down Expand Up @@ -879,26 +879,29 @@ void Window::HorizontalScroll(const WORD wScrollCommand, const WORD wAbsoluteCha
LOG_IF_FAILED(ScreenInfo.SetViewportOrigin(true, NewOrigin, false));
}

BOOL Window::EnableBothScrollBars()
void Window::UpdateScrollBars(const SCREEN_INFORMATION::ScrollBarState& state)
{
return EnableScrollBar(_hWnd, SB_BOTH, ESB_ENABLE_BOTH);
}
// If this is the main buffer, make sure we enable both of the scroll bars.
// The alt buffer likely disabled the scroll bars, this is the only way to re-enable it.
if (!state.isAltBuffer)
{
EnableScrollBar(_hWnd, SB_BOTH, ESB_ENABLE_BOTH);
}

int Window::UpdateScrollBar(bool isVertical,
bool isAltBuffer,
UINT pageSize,
int maxSize,
int viewportPosition)
{
SCROLLINFO si;
si.cbSize = sizeof(si);
si.fMask = isAltBuffer ? SIF_ALL | SIF_DISABLENOSCROLL : SIF_ALL;
si.nPage = pageSize;
si.nMin = 0;
si.nMax = maxSize;
si.nPos = viewportPosition;

return SetScrollInfo(_hWnd, isVertical ? SB_VERT : SB_HORZ, &si, TRUE);
SCROLLINFO si{
.cbSize = sizeof(SCROLLINFO),
.fMask = static_cast<UINT>(state.isAltBuffer ? SIF_ALL | SIF_DISABLENOSCROLL : SIF_ALL),
};

si.nMax = state.maxSize.width;
si.nPage = state.viewport.width();
si.nPos = state.viewport.left;
SetScrollInfo(_hWnd, SB_HORZ, &si, TRUE);

si.nMax = state.maxSize.height;
si.nPage = state.viewport.height();
si.nPos = state.viewport.top;
SetScrollInfo(_hWnd, SB_VERT, &si, TRUE);
}

// Routine Description:
Expand Down
8 changes: 2 additions & 6 deletions src/interactivity/win32/window.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,7 @@ namespace Microsoft::Console::Interactivity::Win32
void HorizontalScroll(const WORD wScrollCommand,
const WORD wAbsoluteChange);

BOOL EnableBothScrollBars();
int UpdateScrollBar(bool isVertical,
bool isAltBuffer,
UINT pageSize,
int maxSize,
int viewportPosition);
void UpdateScrollBars(const SCREEN_INFORMATION::ScrollBarState& state);

void UpdateWindowSize(const til::size coordSizeInChars);
void UpdateWindowPosition(_In_ const til::point ptNewPos) const;
Expand Down Expand Up @@ -185,6 +180,7 @@ namespace Microsoft::Console::Interactivity::Win32

static void s_ReinitializeFontsForDPIChange();

WORD _resizingWindow = 0; // > 0 if we should ignore WM_SIZE messages
bool _fInDPIChange = false;

static void s_ConvertWindowPosToWindowRect(const LPWINDOWPOS lpWindowPos,
Expand Down
13 changes: 11 additions & 2 deletions src/interactivity/win32/windowproc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,16 @@ using namespace Microsoft::Console::Types;

case CM_UPDATE_SCROLL_BARS:
{
ScreenInfo.InternalUpdateScrollBars();
const auto state = ScreenInfo.FetchScrollBarState();

// EnableScrollbar() and especially SetScrollInfo() are prohibitively expensive functions nowadays.
// Unlocking early here improves throughput of good old `type` in cmd.exe by ~10x.
UnlockConsole();
Unlock = FALSE;

_resizingWindow++;
UpdateScrollBars(state);
_resizingWindow--;
break;
}

Expand Down Expand Up @@ -792,7 +801,7 @@ void Window::_HandleWindowPosChanged(const LPARAM lParam)
// CONSOLE_IS_ICONIC bit appropriately. doing so in the WM_SIZE handler is incorrect because the WM_SIZE
// comes after the WM_ERASEBKGND during SetWindowPos() processing, and the WM_ERASEBKGND needs to know if
// the console window is iconic or not.
if (!ScreenInfo.ResizingWindow && (lpWindowPos->cx || lpWindowPos->cy) && !IsIconic(hWnd))
if (!_resizingWindow && (lpWindowPos->cx || lpWindowPos->cy) && !IsIconic(hWnd))
{
// calculate the dimensions for the newly proposed window rectangle
til::rect rcNew;
Expand Down

0 comments on commit 29895e1

Please sign in to comment.