Improve performance and binary size of til::enumset (#11493)

This commit approximately doubles the performance of til::enumset
and reduces it's binary footprint by approximately 1kB.
Most of the binary size can be attributed to exception handling.

Unfortunately this commit removes assertions that the given values are less than
the number of bits in the `underlying_type`. However I believe this to be a good
trade-off as the tests previously only happened at runtime, while tests at
compile time would be highly preferable. Such tests are technically possible,
however MSVC fails to compile (valid) `static_assert`s containing
`static_cast`s over a parameter pack at the time of writing.
With future MSVC versions such checks can be added to this class.

This change was initially discussed in #10492, but was forgotten to
be considered before it was merged. Since the work was already done,
this commit re-introduces the optimization. It's free!

## PR Checklist
* [x] I work here.
* [x] Tests added/passed

## Validation Steps Performed
* Run `printf '\e]8;;http://example.com\e\\This is a link\e]8;;\e\\\n'` in WSL
* A wild dotted line appears ✔️
This commit is contained in:
Leonard Hecker 2021-11-23 19:44:58 +01:00 committed by GitHub
parent 80f8383860
commit f2386de422
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 100 additions and 137 deletions

View File

@ -3,130 +3,136 @@
#pragma once #pragma once
#include <bitset> #ifdef __cpp_concepts
#define TIL_ENUMSET_VARARG template<std::same_as<T>... Args>
#else
#define TIL_ENUMSET_VARARG template<typename... Args, typename = std::enable_if_t<std::conjunction_v<std::is_same<T, Args>...>>>
#endif
namespace til // Terminal Implementation Library. Also: "Today I Learned" namespace til // Terminal Implementation Library. Also: "Today I Learned"
{ {
// By design, this class hides several methods in the std::bitset class // til::enumset stores a fixed size array of boolean elements, the positions
// so they can be called with an enum parameter instead of a size_t, so // in the array being identified by values from a given enumerated type.
// we need to disable the "hides a non-virtual function" warning. // Position N corresponds to bit 1<<N in the UnderlyingType integer.
#pragma warning(push) //
#pragma warning(disable : 26434) // If you only need 32 positions for your T, UnderlyingType can be set uint32_t.
// It defaults to uintptr_t allowing you to set as many positions as a pointer has bits.
// til::enumset is a subclass of std::bitset, storing a fixed size array of // This class doesn't statically assert that your given position fits into UnderlyingType.
// boolean elements, the positions in the array being identified by values template<typename T, typename UnderlyingType = uintptr_t>
// from a given enumerated type. By default it holds the same number of class enumset
// bits as a size_t value.
template<typename Type, size_t Bits = std::numeric_limits<size_t>::digits>
class enumset : public std::bitset<Bits>
{ {
using _base = std::bitset<Bits>; static_assert(std::is_unsigned_v<UnderlyingType>);
public: public:
using reference = typename _base::reference;
enumset() = default;
// Method Description: // Method Description:
// - Constructs a new bitset with the given list of positions set to true. // - Constructs a new bitset with the given list of positions set to true.
template<typename... Args, typename = std::enable_if_t<std::conjunction_v<std::is_same<Type, Args>...>>> TIL_ENUMSET_VARARG
constexpr enumset(const Args... positions) noexcept : constexpr enumset(Args... positions) noexcept :
_base((... | (1ULL << static_cast<size_t>(positions)))) _data{ to_underlying(positions...) }
{ {
} }
// Method Description: // Method Description:
// - Returns the value of the bit at the given position. // - Returns the underlying bit positions as a copy.
constexpr bool operator[](const Type pos) const constexpr UnderlyingType bits() const noexcept
{ {
return _base::operator[](static_cast<size_t>(pos)); return _data;
}
// Method Description:
// - Returns a reference to the bit at the given position.
reference operator[](const Type pos)
{
return _base::operator[](static_cast<size_t>(pos));
} }
// Method Description: // Method Description:
// - Returns the value of the bit at the given position. // - Returns the value of the bit at the given position.
// Throws std::out_of_range if it is not a valid position // Throws std::out_of_range if it is not a valid position
// in the bitset. // in the bitset.
bool test(const Type pos) const constexpr bool test(const T pos) const noexcept
{ {
return _base::test(static_cast<size_t>(pos)); const auto mask = to_underlying(pos);
return (_data & mask) != 0;
} }
// Method Description: // Method Description:
// - Returns true if any of the bits are set to true. // - Returns true if any of the bits are set to true.
bool any() const noexcept constexpr bool any() const noexcept
{ {
return _base::any(); return _data != 0;
} }
// Method Description: // Method Description:
// - Returns true if any of the bits in the given positions are true. // - Returns true if any of the bits in the given positions are true.
template<typename... Args, typename = std::enable_if_t<std::conjunction_v<std::is_same<Type, Args>...>>> TIL_ENUMSET_VARARG
bool any(const Args... positions) const noexcept constexpr bool any(Args... positions) const noexcept
{ {
return (enumset{ positions... } & *this) != 0; const auto mask = to_underlying(positions...);
return (_data & mask) != 0;
} }
// Method Description: // Method Description:
// - Returns true if all of the bits are set to true. // - Returns true if all of the bits are set to true.
bool all() const noexcept constexpr bool all() const noexcept
{ {
return _base::all(); return _data == ~UnderlyingType{ 0 };
} }
// Method Description: // Method Description:
// - Returns true if all of the bits in the given positions are true. // - Returns true if all of the bits in the given positions are true.
template<typename... Args, typename = std::enable_if_t<std::conjunction_v<std::is_same<Type, Args>...>>> TIL_ENUMSET_VARARG
bool all(const Args... positions) const noexcept constexpr bool all(Args... positions) const noexcept
{ {
return (enumset{ positions... } & *this) == enumset{ positions... }; const auto mask = to_underlying(positions...);
return (_data & mask) == mask;
} }
// Method Description: // Method Description:
// - Sets the bit in the given position to the specified value. // - Sets all of the bits in the given positions to true.
enumset& set(const Type pos, const bool val = true) TIL_ENUMSET_VARARG
constexpr enumset& set(Args... positions) noexcept
{ {
_base::set(static_cast<size_t>(pos), val); _data |= to_underlying(positions...);
return *this; return *this;
} }
// Method Description: // Method Description:
// - Resets the bit in the given position to false. // - Sets the bit in the given position to the specified value.
enumset& reset(const Type pos) constexpr enumset& set(const T pos, const bool val) noexcept
{ {
_base::reset(static_cast<size_t>(pos)); const auto mask = to_underlying(pos);
// false == 0 --> UnderlyingType(-0) == 0b0000...
// true == 1 --> UnderlyingType(-1) == 0b1111...
#pragma warning(suppress : 4804) // '-': unsafe use of type 'bool' in operation
_data = (_data & ~mask) | (-val & mask);
return *this;
}
// Method Description:
// - Resets all of the bits in the given positions to false.
TIL_ENUMSET_VARARG
constexpr enumset& reset(Args... positions) noexcept
{
_data &= ~to_underlying(positions...);
return *this; return *this;
} }
// Method Description: // Method Description:
// - Flips the bit at the given position. // - Flips the bit at the given position.
enumset& flip(const Type pos) TIL_ENUMSET_VARARG
constexpr enumset& flip(Args... positions) noexcept
{ {
_base::flip(static_cast<size_t>(pos)); _data ^= to_underlying(positions...);
return *this; return *this;
} }
// Method Description: private:
// - Sets all of the bits in the given positions to true.
template<typename... Args> template<typename... Args>
enumset& set_all(const Args... positions) static constexpr UnderlyingType to_underlying(Args... positions) noexcept
{ {
return (..., set(positions)); return ((UnderlyingType{ 1 } << static_cast<UnderlyingType>(positions)) | ...);
} }
// Method Description: template<>
// - Resets all of the bits in the given positions to false. static constexpr UnderlyingType to_underlying() noexcept
template<typename... Args>
enumset& reset_all(const Args... positions)
{ {
return (..., reset(positions)); return 0;
} }
UnderlyingType _data{};
}; };
#pragma warning(pop)
} }

View File

@ -542,7 +542,7 @@ std::wstring TerminalInput::_GenerateSGRSequence(const COORD position,
// - delta: The scroll wheel delta of the input event // - delta: The scroll wheel delta of the input event
// Return value: // Return value:
// True iff the alternate buffer is active and alternate scroll mode is enabled and the event is a mouse wheel event. // True iff the alternate buffer is active and alternate scroll mode is enabled and the event is a mouse wheel event.
bool TerminalInput::_ShouldSendAlternateScroll(const unsigned int button, const short delta) const bool TerminalInput::_ShouldSendAlternateScroll(const unsigned int button, const short delta) const noexcept
{ {
return _mouseInputState.inAlternateBuffer && return _mouseInputState.inAlternateBuffer &&
_inputMode.test(Mode::AlternateScroll) && _inputMode.test(Mode::AlternateScroll) &&
@ -555,7 +555,7 @@ bool TerminalInput::_ShouldSendAlternateScroll(const unsigned int button, const
// - delta: The scroll wheel delta of the input event // - delta: The scroll wheel delta of the input event
// Return value: // Return value:
// True iff the input sequence was sent successfully. // True iff the input sequence was sent successfully.
bool TerminalInput::_SendAlternateScroll(const short delta) const bool TerminalInput::_SendAlternateScroll(const short delta) const noexcept
{ {
if (delta > 0) if (delta > 0)
{ {

View File

@ -250,13 +250,13 @@ const wchar_t* const CTRL_QUESTIONMARK_SEQUENCE = L"\x7F";
const wchar_t* const CTRL_ALT_SLASH_SEQUENCE = L"\x1b\x1f"; const wchar_t* const CTRL_ALT_SLASH_SEQUENCE = L"\x1b\x1f";
const wchar_t* const CTRL_ALT_QUESTIONMARK_SEQUENCE = L"\x1b\x7F"; const wchar_t* const CTRL_ALT_QUESTIONMARK_SEQUENCE = L"\x1b\x7F";
void TerminalInput::SetInputMode(const Mode mode, const bool enabled) void TerminalInput::SetInputMode(const Mode mode, const bool enabled) noexcept
{ {
// If we're changing a tracking mode, we always clear other tracking modes first. // If we're changing a tracking mode, we always clear other tracking modes first.
// We also clear out the last saved mouse position & button. // We also clear out the last saved mouse position & button.
if (mode == Mode::DefaultMouseTracking || mode == Mode::ButtonEventMouseTracking || mode == Mode::AnyEventMouseTracking) if (mode == Mode::DefaultMouseTracking || mode == Mode::ButtonEventMouseTracking || mode == Mode::AnyEventMouseTracking)
{ {
_inputMode.reset_all(Mode::DefaultMouseTracking, Mode::ButtonEventMouseTracking, Mode::AnyEventMouseTracking); _inputMode.reset(Mode::DefaultMouseTracking, Mode::ButtonEventMouseTracking, Mode::AnyEventMouseTracking);
_mouseInputState.lastPos = { -1, -1 }; _mouseInputState.lastPos = { -1, -1 };
_mouseInputState.lastButton = 0; _mouseInputState.lastButton = 0;
} }
@ -265,13 +265,13 @@ void TerminalInput::SetInputMode(const Mode mode, const bool enabled)
// when enabling a new encoding - not when disabling. // when enabling a new encoding - not when disabling.
if ((mode == Mode::Utf8MouseEncoding || mode == Mode::SgrMouseEncoding) && enabled) if ((mode == Mode::Utf8MouseEncoding || mode == Mode::SgrMouseEncoding) && enabled)
{ {
_inputMode.reset_all(Mode::Utf8MouseEncoding, Mode::SgrMouseEncoding); _inputMode.reset(Mode::Utf8MouseEncoding, Mode::SgrMouseEncoding);
} }
_inputMode.set(mode, enabled); _inputMode.set(mode, enabled);
} }
bool TerminalInput::GetInputMode(const Mode mode) const bool TerminalInput::GetInputMode(const Mode mode) const noexcept
{ {
return _inputMode.test(mode); return _inputMode.test(mode);
} }

View File

@ -52,8 +52,8 @@ namespace Microsoft::Console::VirtualTerminal
AlternateScroll AlternateScroll
}; };
void SetInputMode(const Mode mode, const bool enabled); void SetInputMode(const Mode mode, const bool enabled) noexcept;
bool GetInputMode(const Mode mode) const; bool GetInputMode(const Mode mode) const noexcept;
void ForceDisableWin32InputMode(const bool win32InputMode) noexcept; void ForceDisableWin32InputMode(const bool win32InputMode) noexcept;
#pragma region MouseInput #pragma region MouseInput
@ -127,8 +127,8 @@ namespace Microsoft::Console::VirtualTerminal
const short modifierKeyState, const short modifierKeyState,
const short delta); const short delta);
bool _ShouldSendAlternateScroll(const unsigned int button, const short delta) const; bool _ShouldSendAlternateScroll(const unsigned int button, const short delta) const noexcept;
bool _SendAlternateScroll(const short delta) const; bool _SendAlternateScroll(const short delta) const noexcept;
static constexpr unsigned int s_GetPressedButton(const MouseButtonState state) noexcept; static constexpr unsigned int s_GetPressedButton(const MouseButtonState state) noexcept;
#pragma endregion #pragma endregion

View File

@ -23,12 +23,12 @@ StateMachine::StateMachine(std::unique_ptr<IStateMachineEngine> engine) :
_ActionClear(); _ActionClear();
} }
void StateMachine::SetParserMode(const Mode mode, const bool enabled) void StateMachine::SetParserMode(const Mode mode, const bool enabled) noexcept
{ {
_parserMode.set(mode, enabled); _parserMode.set(mode, enabled);
} }
bool StateMachine::GetParserMode(const Mode mode) const bool StateMachine::GetParserMode(const Mode mode) const noexcept
{ {
return _parserMode.test(mode); return _parserMode.test(mode);
} }

View File

@ -48,8 +48,8 @@ namespace Microsoft::Console::VirtualTerminal
Ansi, Ansi,
}; };
void SetParserMode(const Mode mode, const bool enabled); void SetParserMode(const Mode mode, const bool enabled) noexcept;
bool GetParserMode(const Mode mode) const; bool GetParserMode(const Mode mode) const noexcept;
void ProcessCharacter(const wchar_t wch); void ProcessCharacter(const wchar_t wch);
void ProcessString(const std::wstring_view string); void ProcessString(const std::wstring_view string);

View File

@ -24,19 +24,19 @@ class EnumSetTests
{ {
Log::Comment(L"Default constructor with no bits set"); Log::Comment(L"Default constructor with no bits set");
til::enumset<Flags> flags; til::enumset<Flags> flags;
VERIFY_ARE_EQUAL(0b00000u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b00000u, flags.bits());
} }
{ {
Log::Comment(L"Constructor with bit 3 set"); Log::Comment(L"Constructor with bit 3 set");
til::enumset<Flags> flags{ Flags::Three }; til::enumset<Flags> flags{ Flags::Three };
VERIFY_ARE_EQUAL(0b01000u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b01000u, flags.bits());
} }
{ {
Log::Comment(L"Constructor with bits 0, 2, and 4 set"); Log::Comment(L"Constructor with bits 0, 2, and 4 set");
til::enumset<Flags> flags{ Flags::Zero, Flags::Two, Flags::Four }; til::enumset<Flags> flags{ Flags::Zero, Flags::Two, Flags::Four };
VERIFY_ARE_EQUAL(0b10101u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b10101u, flags.bits());
} }
} }
@ -53,39 +53,39 @@ class EnumSetTests
Log::Comment(L"Start with no bits set"); Log::Comment(L"Start with no bits set");
til::enumset<Flags> flags; til::enumset<Flags> flags;
VERIFY_ARE_EQUAL(0b00000u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b00000u, flags.bits());
Log::Comment(L"Set bit 2 to true"); Log::Comment(L"Set bit 2 to true");
flags.set(Flags::Two); flags.set(Flags::Two);
VERIFY_ARE_EQUAL(0b00100u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b00100u, flags.bits());
Log::Comment(L"Flip bit 4 to true"); Log::Comment(L"Flip bit 4 to true");
flags.flip(Flags::Four); flags.flip(Flags::Four);
VERIFY_ARE_EQUAL(0b10100u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b10100u, flags.bits());
Log::Comment(L"Set bit 0 to true"); Log::Comment(L"Set bit 0 to true");
flags.set(Flags::Zero); flags.set(Flags::Zero, true);
VERIFY_ARE_EQUAL(0b10101u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b10101u, flags.bits());
Log::Comment(L"Reset bit 2 to false, leaving 0 and 4 true"); Log::Comment(L"Reset bit 2 to false, leaving 0 and 4 true");
flags.reset(Flags::Two); flags.reset(Flags::Two);
VERIFY_ARE_EQUAL(0b10001u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b10001u, flags.bits());
Log::Comment(L"Set bit 0 to false, leaving 4 true"); Log::Comment(L"Set bit 0 to false, leaving 4 true");
flags.set(Flags::Zero, false); flags.set(Flags::Zero, false);
VERIFY_ARE_EQUAL(0b10000u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b10000u, flags.bits());
Log::Comment(L"Flip bit 4, leaving all bits false "); Log::Comment(L"Flip bit 4, leaving all bits false ");
flags.flip(Flags::Four); flags.flip(Flags::Four);
VERIFY_ARE_EQUAL(0b00000u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b00000u, flags.bits());
Log::Comment(L"Set bits 0, 3, and 2"); Log::Comment(L"Set bits 0, 3, and 2");
flags.set_all(Flags::Zero, Flags::Three, Flags::Two); flags.set(Flags::Zero, Flags::Three, Flags::Two);
VERIFY_ARE_EQUAL(0b01101u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b01101u, flags.bits());
Log::Comment(L"Reset bits 3, 4 (already reset), and 0, leaving 2 true"); Log::Comment(L"Reset bits 3, 4 (already reset), and 0, leaving 2 true");
flags.reset_all(Flags::Three, Flags::Four, Flags::Zero); flags.reset(Flags::Three, Flags::Four, Flags::Zero);
VERIFY_ARE_EQUAL(0b00100u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b00100u, flags.bits());
} }
TEST_METHOD(TestMethods) TEST_METHOD(TestMethods)
@ -101,15 +101,13 @@ class EnumSetTests
Log::Comment(L"Start with bits 0, 2, and 4 set"); Log::Comment(L"Start with bits 0, 2, and 4 set");
til::enumset<Flags> flags{ Flags::Zero, Flags::Two, Flags::Four }; til::enumset<Flags> flags{ Flags::Zero, Flags::Two, Flags::Four };
VERIFY_ARE_EQUAL(0b10101u, flags.to_ulong()); VERIFY_ARE_EQUAL(0b10101u, flags.bits());
Log::Comment(L"Test bits 1 and 2 with the test method"); Log::Comment(L"Test bits 1 through 4 with the test method");
VERIFY_IS_FALSE(flags.test(Flags::One)); VERIFY_IS_FALSE(flags.test(Flags::One));
VERIFY_IS_TRUE(flags.test(Flags::Two)); VERIFY_IS_TRUE(flags.test(Flags::Two));
VERIFY_IS_FALSE(flags.test(Flags::Three));
Log::Comment(L"Test bit 3 and 4 with the array operator"); VERIFY_IS_TRUE(flags.test(Flags::Four));
VERIFY_IS_FALSE(flags[Flags::Three]);
VERIFY_IS_TRUE(flags[Flags::Four]);
Log::Comment(L"Test if any bits are set"); Log::Comment(L"Test if any bits are set");
VERIFY_IS_TRUE(flags.any()); VERIFY_IS_TRUE(flags.any());
@ -125,45 +123,4 @@ class EnumSetTests
Log::Comment(L"Test if both bits 0 and 3 are set"); Log::Comment(L"Test if both bits 0 and 3 are set");
VERIFY_IS_FALSE(flags.all(Flags::Zero, Flags::Three)); VERIFY_IS_FALSE(flags.all(Flags::Zero, Flags::Three));
} }
TEST_METHOD(ArrayReferenceOperator)
{
enum class Flags
{
Zero,
One,
Two,
Three,
Four
};
Log::Comment(L"Start with no bits set");
til::enumset<Flags> flags;
VERIFY_ARE_EQUAL(0b00000u, flags.to_ulong());
Log::Comment(L"Test bit 3 reference is false");
auto reference = flags[Flags::Three];
VERIFY_IS_FALSE(reference);
VERIFY_ARE_EQUAL(0b00000u, flags.to_ulong());
Log::Comment(L"Set bit 3 reference to true");
flags.set(Flags::Three);
VERIFY_IS_TRUE(reference);
VERIFY_ARE_EQUAL(0b01000u, flags.to_ulong());
Log::Comment(L"Reset bit 3 reference to false");
flags.reset(Flags::Three);
VERIFY_IS_FALSE(reference);
VERIFY_ARE_EQUAL(0b00000u, flags.to_ulong());
Log::Comment(L"Flip bit 3 reference to true");
reference.flip();
VERIFY_IS_TRUE(reference);
VERIFY_ARE_EQUAL(0b01000u, flags.to_ulong());
Log::Comment(L"Flip bit 3 reference back to false");
reference.flip();
VERIFY_IS_FALSE(reference);
VERIFY_ARE_EQUAL(0b00000u, flags.to_ulong());
}
}; };