Skip to content

Commit

Permalink
Greatly reduce allocations in the conhost/OpenConsole startup path (#…
Browse files Browse the repository at this point in the history
…8489)

I was looking at conhost/OpenConsole and noticed it was being pretty
inefficient with allocations due to some usages of std::deque and
std::vector that didn't need to be done quite that way.

So this uses std::vector for the TextBuffer's storage of ROW objects,
which allows one allocation to contiguously reserve space for all the
ROWs - on Desktop this is 9001 ROW objects which means it saves 9000
allocations that the std::deque would have done.  Plus it has the
benefit of increasing locality of the ROW objects since deque is going
to chase pointers more often with its data structure.

Then, within each ROW there are CharRow and ATTR_ROW objects that use
std::vector today.  This changes them to use Boost's small_vector, which
is a variation of vector that allows for the so-called "small string
optimization."  Since we know the typical size of these vectors, we can
pre-reserve the right number of elements directly in the
CharRow/ATTR_ROW instances, avoiding any heap allocations at all for
constructing these objects.

There are a ton of variations on this "small_vector" concept out there
in the world - this one in Boost, LLVM has one called SmallVector,
Electronic Arts' STL has a small_vector, Facebook's folly library has
one...there are a silly number of these out there.  But Boost seems like
it's by far the easiest to consume in terms of integration into this
repo, the CI/CD pipeline, licensing, and stuff like that, so I went with
the boost version.

In terms of numbers, I measured the startup path of OpenConsole.exe on
my dev box for Release x64 configuration.  My box is an i7-6700k @ 4
Ghz, with 32 GB RAM, not that I think machine config matters much here:

|        | Allocation count    | Allocated bytes    | CPU usage (ms) |
| ------ | ------------------- | ------------------ | -------------- |
| Before | 29,461              | 4,984,640          | 103            |
| After  | 2,459 (-91%)        | 4,853,931 (-2.6%)  | 96 (-7%)       |

Along the way, I also fixed a dynamic initializer I happened to spot in
the registry code, and updated some docs.

## Validation Steps Performed
- Ran "runut", "runft" and "runuia" locally and confirmed results are
  the same as the main branch
- Profiled the before/after numbers in the Visual Studio profiler, for
  the numbers shown in the table

Co-authored-by: Austin Lamb <austinl@microsoft.com>
  • Loading branch information
Austin-Lamb and Austin Lamb authored Dec 16, 2020
1 parent 551cc9a commit 539a5dc
Show file tree
Hide file tree
Showing 24 changed files with 158 additions and 222 deletions.
14 changes: 14 additions & 0 deletions doc/TAEF.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,17 @@ Invoke-OpenConsoleTests
```

`Invoke-OpenConsoleTests` supports a number of options, which you can enumerate by running `Invoke-OpenConsoleTests -?`.


### Debugging Tests

If you want to debug a test, you can do so by using the TAEF /waitForDebugger flag, such as:

runut *Tests.dll /name:TextBufferTests::TestInsertCharacter /waitForDebugger

Replace the test name with the one you want to debug. Then, TAEF will begin executing the test and output something like this:

TAEF: Waiting for debugger - PID <some PID> @ IP <some IP address>

You can then attach to that PID in your debugger of choice. In Visual Studio, you can use Debug -> Attach To Process, or you could use WinDbg or whatever you want.
Once the debugger attaches, the test will execute and your breakpoints will be hit.
19 changes: 12 additions & 7 deletions src/buffer/out/AttrRow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,16 @@
// - attr - the default text attribute
// Return Value:
// - constructed object
// Note: will throw exception if unable to allocate memory for text attribute storage
ATTR_ROW::ATTR_ROW(const UINT cchRowWidth, const TextAttribute attr)
ATTR_ROW::ATTR_ROW(const UINT cchRowWidth, const TextAttribute attr) noexcept
{
_list.push_back(TextAttributeRun(cchRowWidth, attr));
try
{
_list.emplace_back(TextAttributeRun(cchRowWidth, attr));
}
catch (...)
{
FAIL_FAST_CAUGHT_EXCEPTION();
}
_cchRowWidth = cchRowWidth;
}

Expand All @@ -25,7 +31,7 @@ ATTR_ROW::ATTR_ROW(const UINT cchRowWidth, const TextAttribute attr)
void ATTR_ROW::Reset(const TextAttribute attr)
{
_list.clear();
_list.push_back(TextAttributeRun(_cchRowWidth, attr));
_list.emplace_back(TextAttributeRun(_cchRowWidth, attr));
}

// Routine Description:
Expand Down Expand Up @@ -402,7 +408,7 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// The original run was 3 long. The insertion run was 1 long. We need 1 more for the
// fact that an existing piece of the run was split in half (to hold the latter half).
const size_t cNewRun = _list.size() + newAttrs.size() + 1;
std::vector<TextAttributeRun> newRun;
decltype(_list) newRun;
newRun.reserve(cNewRun);

// We will start analyzing from the beginning of our existing run.
Expand Down Expand Up @@ -595,8 +601,7 @@ std::vector<TextAttributeRun> ATTR_ROW::PackAttrs(const std::vector<TextAttribut
{
if (runs.empty() || runs.back().GetAttributes() != attr)
{
const TextAttributeRun run(1, attr);
runs.push_back(run);
runs.emplace_back(TextAttributeRun(1, attr));
}
else
{
Expand Down
14 changes: 12 additions & 2 deletions src/buffer/out/AttrRow.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Revision History:

#pragma once

#include "boost/container/small_vector.hpp"
#include "TextAttributeRun.hpp"
#include "AttrRowIterator.hpp"

Expand All @@ -28,7 +29,16 @@ class ATTR_ROW final
public:
using const_iterator = typename AttrRowIterator;

ATTR_ROW(const UINT cchRowWidth, const TextAttribute attr);
ATTR_ROW(const UINT cchRowWidth, const TextAttribute attr)
noexcept;

~ATTR_ROW() = default;

ATTR_ROW(const ATTR_ROW&) = default;
ATTR_ROW& operator=(const ATTR_ROW&) = default;
ATTR_ROW(ATTR_ROW&&)
noexcept = default;
ATTR_ROW& operator=(ATTR_ROW&&) noexcept = default;

void Reset(const TextAttribute attr);

Expand Down Expand Up @@ -65,7 +75,7 @@ class ATTR_ROW final
friend class AttrRowIterator;

private:
std::vector<TextAttributeRun> _list;
boost::container::small_vector<TextAttributeRun, 1> _list;
size_t _cchRowWidth;

#ifdef UNIT_TESTING
Expand Down
26 changes: 0 additions & 26 deletions src/buffer/out/AttrRowIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,6 @@ bool AttrRowIterator::operator!=(const AttrRowIterator& it) const noexcept
return !(*this == it);
}

AttrRowIterator& AttrRowIterator::operator++() noexcept
{
_increment(1);
return *this;
}

AttrRowIterator AttrRowIterator::operator++(int) noexcept
{
auto copy = *this;
_increment(1);
return copy;
}

AttrRowIterator& AttrRowIterator::operator+=(const ptrdiff_t& movement)
{
if (!_exceeded)
Expand All @@ -74,19 +61,6 @@ AttrRowIterator& AttrRowIterator::operator-=(const ptrdiff_t& movement)
return this->operator+=(-movement);
}

AttrRowIterator& AttrRowIterator::operator--() noexcept
{
_decrement(1);
return *this;
}

AttrRowIterator AttrRowIterator::operator--(int) noexcept
{
auto copy = *this;
_decrement(1);
return copy;
}

const TextAttribute* AttrRowIterator::operator->() const
{
THROW_HR_IF(E_BOUNDS, _exceeded);
Expand Down
29 changes: 24 additions & 5 deletions src/buffer/out/AttrRowIterator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Author(s):

#pragma once

#include "boost/container/small_vector.hpp"
#include "TextAttribute.hpp"
#include "TextAttributeRun.hpp"

Expand All @@ -38,20 +39,38 @@ class AttrRowIterator final
bool operator==(const AttrRowIterator& it) const noexcept;
bool operator!=(const AttrRowIterator& it) const noexcept;

AttrRowIterator& operator++() noexcept;
AttrRowIterator operator++(int) noexcept;
AttrRowIterator& operator++() noexcept
{
_increment(1);
return *this;
}
AttrRowIterator operator++(int) noexcept
{
auto copy = *this;
_increment(1);
return copy;
}

AttrRowIterator& operator+=(const ptrdiff_t& movement);
AttrRowIterator& operator-=(const ptrdiff_t& movement);

AttrRowIterator& operator--() noexcept;
AttrRowIterator operator--(int) noexcept;
AttrRowIterator& operator--() noexcept
{
_decrement(1);
return *this;
}
AttrRowIterator operator--(int) noexcept
{
auto copy = *this;
_decrement(1);
return copy;
}

const TextAttribute* operator->() const;
const TextAttribute& operator*() const;

private:
std::vector<TextAttributeRun>::const_iterator _run;
boost::container::small_vector_base<TextAttributeRun>::const_iterator _run;
const ATTR_ROW* _pAttrRow;
size_t _currentAttributeIndex; // index of TextAttribute within the current TextAttributeRun
bool _exceeded;
Expand Down
11 changes: 7 additions & 4 deletions src/buffer/out/CharRow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@
// Return Value:
// - instantiated object
// Note: will through if unable to allocate char/attribute buffers
CharRow::CharRow(size_t rowWidth, ROW* const pParent) :
#pragma warning(push)
#pragma warning(disable : 26447) // small_vector's constructor says it can throw but it should not given how we use it. This suppresses this error for the AuditMode build.
CharRow::CharRow(size_t rowWidth, ROW* const pParent) noexcept :
_wrapForced{ false },
_doubleBytePadded{ false },
_data(rowWidth, value_type()),
_pParent{ FAIL_FAST_IF_NULL(pParent) }
{
}
#pragma warning(pop)

// Routine Description:
// - Sets the wrap status for the current row
Expand Down Expand Up @@ -141,7 +144,7 @@ typename CharRow::const_iterator CharRow::cend() const noexcept
// - The calculated left boundary of the internal string.
size_t CharRow::MeasureLeft() const noexcept
{
std::vector<value_type>::const_iterator it = _data.cbegin();
const_iterator it = _data.cbegin();
while (it != _data.cend() && it->IsSpace())
{
++it;
Expand All @@ -155,9 +158,9 @@ size_t CharRow::MeasureLeft() const noexcept
// - <none>
// Return Value:
// - The calculated right boundary of the internal string.
size_t CharRow::MeasureRight() const noexcept
size_t CharRow::MeasureRight() const
{
std::vector<value_type>::const_reverse_iterator it = _data.crbegin();
const_reverse_iterator it = _data.crbegin();
while (it != _data.crend() && it->IsSpace())
{
++it;
Expand Down
12 changes: 7 additions & 5 deletions src/buffer/out/CharRow.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Revision History:
#include "CharRowCellReference.hpp"
#include "CharRowCell.hpp"
#include "UnicodeStorage.hpp"
#include "boost/container/small_vector.hpp"

class ROW;

Expand All @@ -49,11 +50,12 @@ class CharRow final
public:
using glyph_type = typename wchar_t;
using value_type = typename CharRowCell;
using iterator = typename std::vector<value_type>::iterator;
using const_iterator = typename std::vector<value_type>::const_iterator;
using iterator = typename boost::container::small_vector_base<value_type>::iterator;
using const_iterator = typename boost::container::small_vector_base<value_type>::const_iterator;
using const_reverse_iterator = typename boost::container::small_vector_base<value_type>::const_reverse_iterator;
using reference = typename CharRowCellReference;

CharRow(size_t rowWidth, ROW* const pParent);
CharRow(size_t rowWidth, ROW* const pParent) noexcept;

void SetWrapForced(const bool wrap) noexcept;
bool WasWrapForced() const noexcept;
Expand All @@ -63,7 +65,7 @@ class CharRow final
void Reset() noexcept;
[[nodiscard]] HRESULT Resize(const size_t newSize) noexcept;
size_t MeasureLeft() const noexcept;
size_t MeasureRight() const noexcept;
size_t MeasureRight() const;
void ClearCell(const size_t column);
bool ContainsText() const noexcept;
const DbcsAttribute& DbcsAttrAt(const size_t column) const;
Expand Down Expand Up @@ -101,7 +103,7 @@ class CharRow final
bool _doubleBytePadded;

// storage for glyph data and dbcs attributes
std::vector<value_type> _data;
boost::container::small_vector<value_type, 120> _data;

// ROW that this CharRow belongs to
ROW* _pParent;
Expand Down
12 changes: 0 additions & 12 deletions src/buffer/out/CharRowCell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,6 @@
// default glyph value, used for resetting the character data portion of a cell
static constexpr wchar_t DefaultValue = UNICODE_SPACE;

CharRowCell::CharRowCell() noexcept :
_wch{ DefaultValue },
_attr{}
{
}

CharRowCell::CharRowCell(const wchar_t wch, const DbcsAttribute attr) noexcept :
_wch{ wch },
_attr{ attr }
{
}

// Routine Description:
// - "erases" the glyph. really sets it back to the default "empty" value
void CharRowCell::EraseChars() noexcept
Expand Down
14 changes: 10 additions & 4 deletions src/buffer/out/CharRowCell.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Author(s):
#pragma once

#include "DbcsAttribute.hpp"
#include "unicode.hpp"

#if (defined(_M_IX86) || defined(_M_AMD64))
// currently CharRowCell's fields use 3 bytes of memory, leaving the 4th byte in unused. this leads
Expand All @@ -27,8 +28,13 @@ Author(s):
class CharRowCell final
{
public:
CharRowCell() noexcept;
CharRowCell(const wchar_t wch, const DbcsAttribute attr) noexcept;
CharRowCell() noexcept = default;
CharRowCell(const wchar_t wch, const DbcsAttribute attr) noexcept
:
_wch(wch),
_attr(attr)
{
}

void EraseChars() noexcept;
void Reset() noexcept;
Expand All @@ -44,8 +50,8 @@ class CharRowCell final
friend constexpr bool operator==(const CharRowCell& a, const CharRowCell& b) noexcept;

private:
wchar_t _wch;
DbcsAttribute _attr;
wchar_t _wch{ UNICODE_SPACE };
DbcsAttribute _attr{};
};

#if (defined(_M_IX86) || defined(_M_AMD64))
Expand Down
4 changes: 2 additions & 2 deletions src/buffer/out/CharRowCellReference.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ class CharRowCellReference final
}

~CharRowCellReference() = default;
CharRowCellReference(const CharRowCellReference&) = default;
CharRowCellReference(CharRowCellReference&&) = default;
CharRowCellReference(const CharRowCellReference&) noexcept = default;
CharRowCellReference(CharRowCellReference&&) noexcept = default;

void operator=(const CharRowCellReference&) = delete;
void operator=(CharRowCellReference&&) = delete;
Expand Down
Loading

0 comments on commit 539a5dc

Please sign in to comment.