Skip to content

Commit

Permalink
Cache the size viewport structure inside TextBuffer (#6841)
Browse files Browse the repository at this point in the history
Looking up the size of the viewport from the underlying dimensions of
the structures seemed like a good idea at the time (so it would only be
in one place), but it turns out to be more of a perf cost than we
expected. Not necessarily on any one hot path, but if we sort by
functions in WPR, it was the top consumer on the Terminal side. This
instead saves the size as a member of the `TextBuffer` and serves that
out. It only changes when it is constructed or resized traditionally, so
it's easy to update/keep track of. It impacted conhost/conpty to a
lesser degree but was still noticeable.

## Validation Steps Performed
- Run `time cat big.txt` under WPR. Checked before and after perf
  metrics.

## PR Checklist
* [x] Closes perf itch
* [x] I work here
* [x] Manual test
* [x] Documentation irrelevant.
* [x] Schema irrelevant.
* [x] Am core contributor.
  • Loading branch information
miniksa authored Jul 9, 2020
1 parent 313568d commit 9e44df0
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 12 deletions.
4 changes: 2 additions & 2 deletions src/buffer/out/search.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ wchar_t Search::_ApplySensitivity(const wchar_t wch) const noexcept
// - Helper to increment a coordinate in respect to the associated screen buffer
// Arguments
// - coord - Updated by function to increment one position (will wrap X and Y direction)
void Search::_IncrementCoord(COORD& coord) const
void Search::_IncrementCoord(COORD& coord) const noexcept
{
_uiaData.GetTextBuffer().GetSize().IncrementInBoundsCircular(coord);
}
Expand All @@ -270,7 +270,7 @@ void Search::_IncrementCoord(COORD& coord) const
// - Helper to decrement a coordinate in respect to the associated screen buffer
// Arguments
// - coord - Updated by function to decrement one position (will wrap X and Y direction)
void Search::_DecrementCoord(COORD& coord) const
void Search::_DecrementCoord(COORD& coord) const noexcept
{
_uiaData.GetTextBuffer().GetSize().DecrementInBoundsCircular(coord);
}
Expand Down
4 changes: 2 additions & 2 deletions src/buffer/out/search.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ class Search final
bool _CompareChars(const std::wstring_view one, const std::wstring_view two) const noexcept;
void _UpdateNextPosition();

void _IncrementCoord(COORD& coord) const;
void _DecrementCoord(COORD& coord) const;
void _IncrementCoord(COORD& coord) const noexcept;
void _DecrementCoord(COORD& coord) const noexcept;

static COORD s_GetInitialAnchor(Microsoft::Console::Types::IUiaData& uiaData, const Direction dir);

Expand Down
20 changes: 16 additions & 4 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ TextBuffer::TextBuffer(const COORD screenBufferSize,
_cursor{ cursorSize, *this },
_storage{},
_unicodeStorage{},
_renderTarget{ renderTarget }
_renderTarget{ renderTarget },
_size{}
{
// initialize ROWs
for (size_t i = 0; i < static_cast<size_t>(screenBufferSize.Y); ++i)
{
_storage.emplace_back(static_cast<SHORT>(i), screenBufferSize.X, _currentAttributes, this);
}

_UpdateSize();
}

// Routine Description:
Expand Down Expand Up @@ -623,7 +626,7 @@ COORD TextBuffer::GetLastNonSpaceCharacter(std::optional<const Microsoft::Consol
// Return Value:
// - Coordinate position in screen coordinates of the character just before the cursor.
// - NOTE: Will return 0,0 if already in the top left corner
COORD TextBuffer::_GetPreviousFromCursor() const
COORD TextBuffer::_GetPreviousFromCursor() const noexcept
{
COORD coordPosition = GetCursor().GetPosition();

Expand Down Expand Up @@ -652,9 +655,15 @@ const SHORT TextBuffer::GetFirstRowIndex() const noexcept
{
return _firstRow;
}
const Viewport TextBuffer::GetSize() const

const Viewport TextBuffer::GetSize() const noexcept
{
return _size;
}

void TextBuffer::_UpdateSize()
{
return Viewport::FromDimensions({ 0, 0 }, { gsl::narrow<SHORT>(_storage.at(0).size()), gsl::narrow<SHORT>(_storage.size()) });
_size = Viewport::FromDimensions({ 0, 0 }, { gsl::narrow<SHORT>(_storage.at(0).size()), gsl::narrow<SHORT>(_storage.size()) });
}

void TextBuffer::_SetFirstRowIndex(const SHORT FirstRowIndex) noexcept
Expand Down Expand Up @@ -845,6 +854,9 @@ void TextBuffer::Reset()
// Also take advantage of the row ID refresh loop to resize the rows in the X dimension
// and cleanup the UnicodeStorage characters that might fall outside the resized buffer.
_RefreshRowIDs(newSize.X);

// Update the cached size value
_UpdateSize();
}
CATCH_RETURN();

Expand Down
6 changes: 4 additions & 2 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class TextBuffer final

const SHORT GetFirstRowIndex() const noexcept;

const Microsoft::Console::Types::Viewport GetSize() const;
const Microsoft::Console::Types::Viewport GetSize() const noexcept;

void ScrollRows(const SHORT firstRow, const SHORT size, const SHORT delta);

Expand Down Expand Up @@ -177,6 +177,8 @@ class TextBuffer final
std::optional<std::reference_wrapper<PositionInformation>> positionInfo);

private:
void _UpdateSize();
Microsoft::Console::Types::Viewport _size;
std::deque<ROW> _storage;
Cursor _cursor;

Expand All @@ -193,7 +195,7 @@ class TextBuffer final

void _SetFirstRowIndex(const SHORT FirstRowIndex) noexcept;

COORD _GetPreviousFromCursor() const;
COORD _GetPreviousFromCursor() const noexcept;

void _SetWrapOnCurrentRow();
void _AdjustWrapOnCurrentRow(const bool fSet);
Expand Down
2 changes: 1 addition & 1 deletion src/types/ScreenInfoUiaProviderBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ IFACEMETHODIMP ScreenInfoUiaProviderBase::get_SupportedTextSelection(_Out_ Suppo

#pragma endregion

const COORD ScreenInfoUiaProviderBase::_getScreenBufferCoords() const
const COORD ScreenInfoUiaProviderBase::_getScreenBufferCoords() const noexcept
{
return _getTextBuffer().GetSize().Dimensions();
}
Expand Down
2 changes: 1 addition & 1 deletion src/types/ScreenInfoUiaProviderBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ namespace Microsoft::Console::Types
// mechanism for multi-threaded code.
std::unordered_map<EVENTID, bool> _signalFiringMapping{};

const COORD _getScreenBufferCoords() const;
const COORD _getScreenBufferCoords() const noexcept;
const TextBuffer& _getTextBuffer() const noexcept;
const Viewport _getViewport() const noexcept;
void _LockConsole() noexcept;
Expand Down

0 comments on commit 9e44df0

Please sign in to comment.