Skip to content

Commit

Permalink
Cache VT buffer line string to avoid (de)alloc on every paint (#6840)
Browse files Browse the repository at this point in the history
A lot of time was spent between each individual line in the VT paint
engine in allocating some scratch space to assemble the clusters then
deallocating it only to have the next line do that again. Now we just
hold onto that memory space since it should be approximately the size of
a single line wide and will be used over and over and over as painting
continues.

## Validation Steps Performed
- Run `time cat big.txt` under WPR. Checked before and after perf
  metrics.
  • Loading branch information
miniksa authored Jul 8, 2020
1 parent 99c33e0 commit 91f9211
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
21 changes: 10 additions & 11 deletions src/renderer/vt/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,17 +334,17 @@ using namespace Microsoft::Console::Types;
{
RETURN_IF_FAILED(_MoveCursor(coord));

std::wstring wstr;
wstr.reserve(clusters.size());
_bufferLine.clear();
_bufferLine.reserve(clusters.size());

short totalWidth = 0;
for (const auto& cluster : clusters)
{
wstr.append(cluster.GetText());
_bufferLine.append(cluster.GetText());
RETURN_IF_FAILED(ShortAdd(totalWidth, gsl::narrow<short>(cluster.GetColumns()), &totalWidth));
}

RETURN_IF_FAILED(VtEngine::_WriteTerminalAscii(wstr));
RETURN_IF_FAILED(VtEngine::_WriteTerminalAscii(_bufferLine));

// Update our internal tracker of the cursor's position
_lastText.X += totalWidth;
Expand All @@ -371,21 +371,21 @@ using namespace Microsoft::Console::Types;
return S_OK;
}

std::wstring unclusteredString;
unclusteredString.reserve(clusters.size());
_bufferLine.clear();
_bufferLine.reserve(clusters.size());
short totalWidth = 0;
for (const auto& cluster : clusters)
{
unclusteredString.append(cluster.GetText());
_bufferLine.append(cluster.GetText());
RETURN_IF_FAILED(ShortAdd(totalWidth, static_cast<short>(cluster.GetColumns()), &totalWidth));
}
const size_t cchLine = unclusteredString.size();
const size_t cchLine = _bufferLine.size();

bool foundNonspace = false;
size_t lastNonSpace = 0;
for (size_t i = 0; i < cchLine; i++)
{
if (unclusteredString.at(i) != L'\x20')
if (_bufferLine.at(i) != L'\x20')
{
lastNonSpace = i;
foundNonspace = true;
Expand Down Expand Up @@ -479,8 +479,7 @@ using namespace Microsoft::Console::Types;
RETURN_IF_FAILED(_MoveCursor(coord));

// Write the actual text string
std::wstring wstr = std::wstring(unclusteredString.data(), cchActual);
RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8(wstr));
RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8({ _bufferLine.data(), cchActual }));

// GH#4415, GH#5181
// If the renderer told us that this was a wrapped line, then mark
Expand Down
3 changes: 2 additions & 1 deletion src/renderer/vt/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
_newBottomLine{ false },
_deferredCursorPos{ INVALID_COORDS },
_inResizeRequest{ false },
_trace{}
_trace{},
_bufferLine{}
{
#ifndef UNIT_TESTING
// When unit testing, we can instantiate a VtEngine without a pipe.
Expand Down
3 changes: 3 additions & 0 deletions src/renderer/vt/vtrenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ namespace Microsoft::Console::Render

bool _WillWriteSingleChar() const;

// buffer space for these two functions to build their lines
// so they don't have to alloc/free in a tight loop
std::wstring _bufferLine;
[[nodiscard]] HRESULT _PaintUtf8BufferLine(std::basic_string_view<Cluster> const clusters,
const COORD coord,
const bool lineWrapped) noexcept;
Expand Down

0 comments on commit 91f9211

Please sign in to comment.