Skip to content

Commit

Permalink
Use memory pool for PolyTextOut items in GDI Renderer (#8619)
Browse files Browse the repository at this point in the history
Converts the poly text out string and width buffers to use a memory pool since we free/alloc those every frame and are just going to reuse them over and over. 

## PR Checklist
* [x] Supports #3075
* [x] I work here.
* [x] Profiled memory before/after. Tested manually with `big.txt`.

## Detailed Description of the Pull Request / Additional comments
- Sets up a PMR memory pool for the GDI Engine. It tends to alloc and free a bunch of little buffers during painting frames. The pool will likely hold onto that memory frame over frame, but we'd just be using it again and again and again anyway. So this way we avoid all the system memory allocator locks and syscalls.

## Validation Steps Performed
- Ran `big.txt` about 10x in the window. Checked WPR/WPA profile output before/after.
  • Loading branch information
miniksa committed Jan 5, 2021
1 parent f087d03 commit a8b4044
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 28 deletions.
1 change: 1 addition & 0 deletions .github/actions/spell-check/dictionary/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ otms
OUTLINETEXTMETRICW
overridable
PAGESCROLL
pmr
RETURNCMD
rfind
roundf
Expand Down
1 change: 1 addition & 0 deletions src/inc/LibraryIncludes.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <deque>
#include <list>
#include <memory>
#include <memory_resource>
#include <map>
#include <mutex>
#include <shared_mutex>
Expand Down
7 changes: 7 additions & 0 deletions src/renderer/gdi/gdirenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,13 @@ namespace Microsoft::Console::Render
COLORREF _lastBg;
bool _lastFontItalic;

// Memory pooling to save alloc/free work to the OS for things
// frequently created and dropped.
// It's important the pool is first so it can be given to the others on construction.
std::pmr::unsynchronized_pool_resource _pool;
std::pmr::vector<std::pmr::wstring> _polyStrings;
std::pmr::vector<std::pmr::basic_string<int>> _polyWidths;

[[nodiscard]] HRESULT _InvalidCombine(const RECT* const prc) noexcept;
[[nodiscard]] HRESULT _InvalidOffset(const POINT* const ppt) noexcept;
[[nodiscard]] HRESULT _InvalidRestrict() noexcept;
Expand Down
42 changes: 15 additions & 27 deletions src/renderer/gdi/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,11 @@ using namespace Microsoft::Console::Render;

const auto pPolyTextLine = &_pPolyText[_cPolyText];

auto pwsPoly = std::make_unique<wchar_t[]>(cchLine);
RETURN_IF_NULL_ALLOC(pwsPoly);
auto& polyString = _polyStrings.emplace_back(cchLine, UNICODE_NULL);

COORD const coordFontSize = _GetFontSize();

auto rgdxPoly = std::make_unique<int[]>(cchLine);
RETURN_IF_NULL_ALLOC(rgdxPoly);
auto& polyWidth = _polyWidths.emplace_back(cchLine, 0);

// Sum up the total widths the entire line/run is expected to take while
// copying the pixel widths into a structure to direct GDI how many pixels to use per character.
Expand All @@ -327,9 +325,9 @@ using namespace Microsoft::Console::Render;

// Our GDI renderer hasn't and isn't going to handle things above U+FFFF or sequences.
// So replace anything complicated with a replacement character for drawing purposes.
pwsPoly[i] = cluster.GetTextAsSingle();
rgdxPoly[i] = gsl::narrow<int>(cluster.GetColumns()) * coordFontSize.X;
cchCharWidths += rgdxPoly[i];
polyString[i] = cluster.GetTextAsSingle();
polyWidth[i] = gsl::narrow<int>(cluster.GetColumns()) * coordFontSize.X;
cchCharWidths += polyWidth[i];
}

// Detect and convert for raster font...
Expand All @@ -338,15 +336,15 @@ using namespace Microsoft::Console::Render;
// dispatch conversion into our codepage

// Find out the bytes required
int const cbRequired = WideCharToMultiByte(_fontCodepage, 0, pwsPoly.get(), (int)cchLine, nullptr, 0, nullptr, nullptr);
int const cbRequired = WideCharToMultiByte(_fontCodepage, 0, polyString.data(), (int)cchLine, nullptr, 0, nullptr, nullptr);

if (cbRequired != 0)
{
// Allocate buffer for MultiByte
auto psConverted = std::make_unique<char[]>(cbRequired);

// Attempt conversion to current codepage
int const cbConverted = WideCharToMultiByte(_fontCodepage, 0, pwsPoly.get(), (int)cchLine, psConverted.get(), cbRequired, nullptr, nullptr);
int const cbConverted = WideCharToMultiByte(_fontCodepage, 0, polyString.data(), (int)cchLine, psConverted.get(), cbRequired, nullptr, nullptr);

// If successful...
if (cbConverted != 0)
Expand All @@ -356,22 +354,22 @@ using namespace Microsoft::Console::Render;

if (cchRequired != 0)
{
auto pwsConvert = std::make_unique<wchar_t[]>(cchRequired);
std::pmr::wstring polyConvert(cchRequired, UNICODE_NULL, &_pool);

// Then do the actual conversion.
int const cchConverted = MultiByteToWideChar(CP_ACP, 0, psConverted.get(), cbRequired, pwsConvert.get(), cchRequired);
int const cchConverted = MultiByteToWideChar(CP_ACP, 0, psConverted.get(), cbRequired, polyConvert.data(), cchRequired);

if (cchConverted != 0)
{
// If all successful, use this instead.
pwsPoly.swap(pwsConvert);
polyString.swap(polyConvert);
}
}
}
}
}

pPolyTextLine->lpstr = pwsPoly.release();
pPolyTextLine->lpstr = polyString.data();
pPolyTextLine->n = gsl::narrow<UINT>(clusters.size());
pPolyTextLine->x = ptDraw.x;
pPolyTextLine->y = ptDraw.y;
Expand All @@ -380,7 +378,7 @@ using namespace Microsoft::Console::Render;
pPolyTextLine->rcl.top = pPolyTextLine->y;
pPolyTextLine->rcl.right = pPolyTextLine->rcl.left + ((SHORT)cchCharWidths * coordFontSize.X);
pPolyTextLine->rcl.bottom = pPolyTextLine->rcl.top + coordFontSize.Y;
pPolyTextLine->pdx = rgdxPoly.release();
pPolyTextLine->pdx = polyWidth.data();

if (trimLeft)
{
Expand Down Expand Up @@ -417,20 +415,10 @@ using namespace Microsoft::Console::Render;
hr = E_FAIL;
}

for (size_t iPoly = 0; iPoly < _cPolyText; iPoly++)
{
if (nullptr != _pPolyText[iPoly].lpstr)
{
delete[] _pPolyText[iPoly].lpstr;
_pPolyText[iPoly].lpstr = nullptr;
}
_polyStrings.clear();
_polyWidths.clear();

if (nullptr != _pPolyText[iPoly].pdx)
{
delete[] _pPolyText[iPoly].pdx;
_pPolyText[iPoly].pdx = nullptr;
}
}
ZeroMemory(_pPolyText, sizeof(_pPolyText));

_cPolyText = 0;
}
Expand Down
5 changes: 4 additions & 1 deletion src/renderer/gdi/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ GdiEngine::GdiEngine() :
_lastFontItalic(false),
_fPaintStarted(false),
_hfont(nullptr),
_hfontItalic(nullptr)
_hfontItalic(nullptr),
_pool{}, // It's important the pool is first so it can be given to the others on construction.
_polyStrings{ &_pool },
_polyWidths{ &_pool }
{
ZeroMemory(_pPolyText, sizeof(POLYTEXTW) * s_cPolyTextCache);
_rcInvalid = { 0 };
Expand Down

0 comments on commit a8b4044

Please sign in to comment.