Reduce Transient Allocations during Bulk Text Output (microsoft#8617)

Make a few changes to memory usage throughout the application to reduce transient allocations from the `big.txt` test from ~213,000 to ~53,000.

## PR Checklist
* [x] Supports microsoft#3075
* [x] I work here.
* [x] Tested manually and WPR'd. Test suite should still pass.
* [x] Am core contributor

## Detailed Description of the Pull Request / Additional comments

Transient allocations are those that are new'd, used, then delete'd. Going back and forth to the system allocator for things we're just going to throw away or use rapidly again is a performance detriment. Not only is it a bunch of time to go ask the system with a syscall, it also hits a whole bunch of locks on the allocators. This PR identifies a few places where we were accidentally allocating and didn't mean to or were allocating and freeing just to turn around and allocate again. I chose other strategies to avoid this.

## Validation Steps Performed
- Ran `big.txt` sample (~6MB file) before and after. Observed heap allocations with WPR.

src/buffer/out/AttrRow.cpp

@@ -184,15 +184,15 @@ size_t ATTR_ROW::FindAttrIndex(const size_t index, size_t* const pApplies) const
 // Routine Description:
 // - Finds the hyperlink IDs present in this row and returns them
 // Return value:
-// - An unordered set containing the hyperlink IDs present in this row
-std::unordered_set<uint16_t> ATTR_ROW::GetHyperlinks()
+// - The hyperlink IDs present in this row
+std::vector<uint16_t> ATTR_ROW::GetHyperlinks()
 {
-    std::unordered_set<uint16_t> ids;
+    std::vector<uint16_t> ids;
     for (const auto& run : _list)
     {
         if (run.GetAttributes().IsHyperlink())
         {
-            ids.emplace(run.GetAttributes().GetHyperlinkId());
+            ids.emplace_back(run.GetAttributes().GetHyperlinkId());
         }
     }
     return ids;

src/buffer/out/AttrRow.hpp

@@ -20,7 +20,6 @@ Revision History:
 
 #pragma once
 
-#include "boost/container/small_vector.hpp"
 #include "TextAttributeRun.hpp"
 #include "AttrRowIterator.hpp"
 
@@ -51,7 +50,7 @@ class ATTR_ROW final
     size_t FindAttrIndex(const size_t index,
                          size_t* const pApplies) const;
 
-    std::unordered_set<uint16_t> GetHyperlinks();
+    std::vector<uint16_t> GetHyperlinks();
 
     bool SetAttrToEnd(const UINT iStart, const TextAttribute attr);
     void ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAttribute& replaceWith) noexcept;

src/buffer/out/AttrRowIterator.hpp

@@ -15,7 +15,6 @@ Author(s):
 
 #pragma once
 
-#include "boost/container/small_vector.hpp"
 #include "TextAttribute.hpp"
 #include "TextAttributeRun.hpp"
 

src/buffer/out/CharRow.hpp

@@ -24,7 +24,6 @@ Revision History:
 #include "CharRowCellReference.hpp"
 #include "CharRowCell.hpp"
 #include "UnicodeStorage.hpp"
-#include "boost/container/small_vector.hpp"
 
 class ROW;
 

src/buffer/out/textBuffer.cpp

@@ -1230,9 +1230,15 @@ void TextBuffer::_PruneHyperlinks()
     // If the buffer does not contain the same reference, we can remove that hyperlink from our map
     // This way, obsolete hyperlink references are cleared from our hyperlink map instead of hanging around
     // Get all the hyperlink references in the row we're erasing
-    auto firstRowRefs = _storage.at(_firstRow).GetAttrRow().GetHyperlinks();
-    if (!firstRowRefs.empty())
+    const auto hyperlinks = _storage.at(_firstRow).GetAttrRow().GetHyperlinks();
+
+    if (!hyperlinks.empty())
     {
+        // Move to unordered set so we can use hashed lookup of IDs instead of linear search.
+        // Only make it an unordered set now because set always heap allocates but vector
+        // doesn't when the set is empty (saving an allocation in the common case of no links.)
+        std::unordered_set<uint16_t> firstRowRefs{ hyperlinks.cbegin(), hyperlinks.cend() };
+
         const auto total = TotalRowCount();
         // Loop through all the rows in the buffer except the first row -
         // we have found all hyperlink references in the first row and put them in refs,
@@ -1254,12 +1260,12 @@ void TextBuffer::_PruneHyperlinks()
                 break;
             }
         }
-    }
 
-    // Now delete obsolete references from our map
-    for (auto hyperlinkReference : firstRowRefs)
-    {
-        RemoveHyperlinkFromMap(hyperlinkReference);
+        // Now delete obsolete references from our map
+        for (auto hyperlinkReference : firstRowRefs)
+        {
+            RemoveHyperlinkFromMap(hyperlinkReference);
+        }
     }
 }
 

src/host/precomp.h

@@ -88,8 +88,6 @@ TRACELOGGING_DECLARE_PROVIDER(g_hConhostV2EventTraceProvider);
 #include "../inc/operators.hpp"
 #include "../inc/conattrs.hpp"
 
-#include "boost/container/small_vector.hpp"
-
 // TODO: MSFT 9355094 Find a better way of doing this. http://osgvsowi/9355094
 [[nodiscard]] inline NTSTATUS NTSTATUS_FROM_HRESULT(HRESULT hr)
 {

src/host/tracing.cpp

@@ -210,13 +210,13 @@ void Tracing::s_TraceApi(const CONSOLE_SCREENBUFFERINFO_MSG* const a)
     static_assert(sizeof(UINT32) == sizeof(*a->ColorTable), "a->ColorTable");
 }
 
-void Tracing::s_TraceApi(const CONSOLE_MODE_MSG* const a, const std::wstring& handleType)
+void Tracing::s_TraceApi(const CONSOLE_MODE_MSG* const a, const std::wstring_view handleType)
 {
     TraceLoggingWrite(
         g_hConhostV2EventTraceProvider,
         "API_GetConsoleMode",
         TraceLoggingHexUInt32(a->Mode, "Mode"),
-        TraceLoggingWideString(handleType.c_str(), "Handle type"),
+        TraceLoggingCountedWideString(handleType.data(), gsl::narrow_cast<ULONG>(handleType.size()), "Handle type"),
         TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
         TraceLoggingKeyword(TraceKeywords::API));
 }

src/host/tracing.hpp

@@ -50,7 +50,7 @@ class Tracing
     static void s_TraceApi(_In_ const void* const buffer, const CONSOLE_WRITECONSOLE_MSG* const a);
 
     static void s_TraceApi(const CONSOLE_SCREENBUFFERINFO_MSG* const a);
-    static void s_TraceApi(const CONSOLE_MODE_MSG* const a, const std::wstring& handleType);
+    static void s_TraceApi(const CONSOLE_MODE_MSG* const a, const std::wstring_view handleType);
     static void s_TraceApi(const CONSOLE_SETTEXTATTRIBUTE_MSG* const a);
     static void s_TraceApi(const CONSOLE_WRITECONSOLEOUTPUTSTRING_MSG* const a);
 

src/inc/LibraryIncludes.h

@@ -74,6 +74,9 @@
 #include <base/numerics/safe_math.h>
 #pragma warning(pop)
 
+// Boost
+#include "boost/container/small_vector.hpp"
+
 // IntSafe
 #define ENABLE_INTSAFE_SIGNED_FUNCTIONS
 #include <intsafe.h>

src/server/ApiDispatchers.cpp

@@ -35,7 +35,7 @@
 {
     Telemetry::Instance().LogApiCall(Telemetry::ApiCall::GetConsoleMode);
     CONSOLE_MODE_MSG* const a = &m->u.consoleMsgL1.GetConsoleMode;
-    std::wstring handleType = L"unknown";
+    std::wstring_view handleType = L"unknown";
 
     TraceLoggingWrite(g_hConhostV2EventTraceProvider,
                       "API_GetConsoleMode",
@@ -54,14 +54,14 @@
     RETURN_HR_IF_NULL(E_HANDLE, pObjectHandle);
     if (pObjectHandle->IsInputHandle())
     {
-        handleType = L"input handle";
+        handleType = L"input";
         InputBuffer* pObj;
         RETURN_IF_FAILED(pObjectHandle->GetInputBuffer(GENERIC_READ, &pObj));
         m->_pApiRoutines->GetConsoleInputModeImpl(*pObj, a->Mode);
     }
     else
     {
-        handleType = L"output handle";
+        handleType = L"output";
         SCREEN_INFORMATION* pObj;
         RETURN_IF_FAILED(pObjectHandle->GetScreenBuffer(GENERIC_READ, &pObj));
         m->_pApiRoutines->GetConsoleOutputModeImpl(*pObj, a->Mode);

src/server/ApiMessage.cpp

@@ -9,10 +9,17 @@
 #include "DeviceComm.h"
 
 _CONSOLE_API_MSG::_CONSOLE_API_MSG() :
+    Complete{ 0 },
+    State{ 0 },
     _pDeviceComm(nullptr),
-    _pApiRoutines(nullptr)
+    _pApiRoutines(nullptr),
+    _inputBuffer{},
+    _outputBuffer{},
+    Descriptor{ 0 },
+    CreateObject{ 0 },
+    CreateScreenBuffer{ 0 },
+    msgHeader{ 0 }
 {
-    ZeroMemory(this, sizeof(_CONSOLE_API_MSG));
 }
 
 ConsoleProcessHandle* _CONSOLE_API_MSG::GetProcessHandle() const
@@ -57,6 +64,7 @@ ConsoleHandleData* _CONSOLE_API_MSG::GetObjectHandle() const
 // -  HRESULT indicating if the input buffer was successfully retrieved.
 [[nodiscard]] HRESULT _CONSOLE_API_MSG::GetInputBuffer(_Outptr_result_bytebuffer_(*pcbSize) void** const ppvBuffer,
                                                        _Out_ ULONG* const pcbSize)
+try
 {
     // Initialize the buffer if it hasn't been initialized yet.
     if (State.InputBuffer == nullptr)
@@ -65,12 +73,11 @@ ConsoleHandleData* _CONSOLE_API_MSG::GetObjectHandle() const
 
         ULONG const cbReadSize = Descriptor.InputSize - State.ReadOffset;
 
-        wistd::unique_ptr<BYTE[]> pPayload = wil::make_unique_nothrow<BYTE[]>(cbReadSize);
-        RETURN_IF_NULL_ALLOC(pPayload);
+        _inputBuffer.resize(cbReadSize);
 
-        RETURN_IF_FAILED(ReadMessageInput(0, pPayload.get(), cbReadSize));
+        RETURN_IF_FAILED(ReadMessageInput(0, _inputBuffer.data(), cbReadSize));
 
-        State.InputBuffer = pPayload.release(); // TODO: MSFT: 9565140 - don't release, maintain as smart pointer.
+        State.InputBuffer = _inputBuffer.data();
         State.InputBufferSize = cbReadSize;
     }
 
@@ -80,6 +87,7 @@ ConsoleHandleData* _CONSOLE_API_MSG::GetObjectHandle() const
 
     return S_OK;
 }
+CATCH_RETURN();
 
 // Routine Description:
 // - This routine retrieves the output buffer associated with this message. It will allocate one if needed.
@@ -94,6 +102,7 @@ ConsoleHandleData* _CONSOLE_API_MSG::GetObjectHandle() const
 [[nodiscard]] HRESULT _CONSOLE_API_MSG::GetAugmentedOutputBuffer(const ULONG cbFactor,
                                                                  _Outptr_result_bytebuffer_(*pcbSize) PVOID* const ppvBuffer,
                                                                  _Out_ PULONG pcbSize)
+try
 {
     // Initialize the buffer if it hasn't been initialized yet.
     if (State.OutputBuffer == nullptr)
@@ -103,11 +112,12 @@ ConsoleHandleData* _CONSOLE_API_MSG::GetObjectHandle() const
         ULONG cbWriteSize = Descriptor.OutputSize - State.WriteOffset;
         RETURN_IF_FAILED(ULongMult(cbWriteSize, cbFactor, &cbWriteSize));
 
-        BYTE* pPayload = new (std::nothrow) BYTE[cbWriteSize];
-        RETURN_IF_NULL_ALLOC(pPayload);
-        ZeroMemory(pPayload, sizeof(BYTE) * cbWriteSize);
+        _outputBuffer.resize(cbWriteSize);
 
-        State.OutputBuffer = pPayload; // TODO: MSFT: 9565140 - maintain as smart pointer.
+        // 0 it out.
+        std::fill(_outputBuffer.begin(), _outputBuffer.end(), (BYTE)0);
+
+        State.OutputBuffer = _outputBuffer.data();
         State.OutputBufferSize = cbWriteSize;
     }
 
@@ -117,6 +127,7 @@ ConsoleHandleData* _CONSOLE_API_MSG::GetObjectHandle() const
 
     return S_OK;
 }
+CATCH_RETURN();
 
 // Routine Description:
 // - This routine retrieves the output buffer associated with this message. It will allocate one if needed.
@@ -148,8 +159,9 @@ ConsoleHandleData* _CONSOLE_API_MSG::GetObjectHandle() const
 
     if (State.InputBuffer != nullptr)
     {
-        delete[] static_cast<BYTE*>(State.InputBuffer);
+        _inputBuffer.clear();
         State.InputBuffer = nullptr;
+        State.InputBufferSize = 0;
     }
 
     if (State.OutputBuffer != nullptr)
@@ -165,8 +177,9 @@ ConsoleHandleData* _CONSOLE_API_MSG::GetObjectHandle() const
             LOG_IF_FAILED(_pDeviceComm->WriteOutput(&IoOperation));
         }
 
-        delete[] static_cast<BYTE*>(State.OutputBuffer);
+        _outputBuffer.clear();
         State.OutputBuffer = nullptr;
+        State.OutputBufferSize = 0;
     }
 
     return hr;

src/server/ApiMessage.h

@@ -35,6 +35,11 @@ typedef struct _CONSOLE_API_MSG
     IDeviceComm* _pDeviceComm;
     IApiRoutines* _pApiRoutines;
 
+private:
+    boost::container::small_vector<BYTE, 128> _inputBuffer;
+    boost::container::small_vector<BYTE, 128> _outputBuffer;
+
+public:
     // From here down is the actual packet data sent/received.
     CD_IO_DESCRIPTOR Descriptor;
     union
@@ -58,6 +63,10 @@ typedef struct _CONSOLE_API_MSG
     // End packet data
 
 public:
+    // DO NOT PUT MORE FIELDS DOWN HERE.
+    // The tail end of this structure will have a console driver packet
+    // copied over it and it will overwrite any fields declared here.
+
     ConsoleProcessHandle* GetProcessHandle() const;
     ConsoleHandleData* GetObjectHandle() const;
 
@@ -73,4 +82,8 @@ typedef struct _CONSOLE_API_MSG
     void SetReplyStatus(const NTSTATUS Status);
     void SetReplyInformation(const ULONG_PTR pInformation);
 
+    // DO NOT PUT MORE FIELDS DOWN HERE.
+    // The tail end of this structure will have a console driver packet
+    // copied over it and it will overwrite any fields declared here.
+
 } CONSOLE_API_MSG, *PCONSOLE_API_MSG, * const PCCONSOLE_API_MSG;