Can we have infinite scrollback?

.github/actions/spelling/expect/expect.txt

@@ -392,6 +392,7 @@ DECNKM
 DECNRCM
 DECOM
 decommit
+decommitting
 DECPCCM
 DECPCTERM
 DECPS
@@ -1808,6 +1809,7 @@ uwa
 uwp
 uwu
 uxtheme
+VADs
 Vanara
 vararg
 vclib

doc/cascadia/profiles.schema.json

@@ -3035,7 +3035,7 @@
           "type": "boolean"
         },
         "historySize": {
-          "default": 9001,
+          "default": 16384,
           "description": "The number of lines above the ones displayed in the window you can scroll back to.",
           "minimum": -1,
           "type": "integer"

src/buffer/out/OutputCellRect.cpp

@@ -25,7 +25,7 @@ OutputCellRect::OutputCellRect(const til::CoordType rows, const til::CoordType c
     _rows(rows),
     _cols(cols)
 {
-    _storage.resize(gsl::narrow<size_t>(rows * cols));
+    _storage.resize(gsl::narrow<size_t>(static_cast<til::HugeCoordType>(rows) * cols));
 }
 
 // Routine Description:
@@ -61,7 +61,7 @@ OutputCellIterator OutputCellRect::GetRowIter(const til::CoordType row) const
 // - Pointer to the location in the rectangle that represents the start of the requested row.
 OutputCell* OutputCellRect::_FindRowOffset(const til::CoordType row)
 {
-    return &_storage.at(gsl::narrow_cast<size_t>(row * _cols));
+    return &_storage.at(gsl::narrow<size_t>(static_cast<til::HugeCoordType>(row) * _cols));
 }
 
 // Routine Description:
@@ -73,7 +73,7 @@ OutputCell* OutputCellRect::_FindRowOffset(const til::CoordType row)
 // - Pointer to the location in the rectangle that represents the start of the requested row.
 const OutputCell* OutputCellRect::_FindRowOffset(const til::CoordType row) const
 {
-    return &_storage.at(gsl::narrow_cast<size_t>(row * _cols));
+    return &_storage.at(gsl::narrow<size_t>(static_cast<til::HugeCoordType>(row) * _cols));
 }
 
 // Routine Description:

src/buffer/out/textBuffer.cpp

@@ -60,17 +60,14 @@ TextBuffer::TextBuffer(til::size screenBufferSize,
     _cursor{ cursorSize, *this },
     _isActiveBuffer{ isActiveBuffer }
 {
-    // Guard against resizing the text buffer to 0 columns/rows, which would break being able to insert text.
-    screenBufferSize.width = std::max(screenBufferSize.width, 1);
-    screenBufferSize.height = std::max(screenBufferSize.height, 1);
     _reserve(screenBufferSize, defaultAttributes);
 }
 
 TextBuffer::~TextBuffer()
 {
     if (_buffer)
     {
-        _destroy();
+        _destroy(_buffer.get());
     }
 }
 
@@ -91,8 +88,9 @@ TextBuffer::~TextBuffer()
 // memory usage from ~7MB down to just ~2MB at startup in the general case.
 void TextBuffer::_reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes)
 {
-    const auto w = gsl::narrow<uint16_t>(screenBufferSize.width);
-    const auto h = gsl::narrow<uint16_t>(screenBufferSize.height);
+    // Guard against resizing the text buffer to 0 columns/rows, which would break being able to insert text.
+    const auto w = std::clamp(screenBufferSize.width, 1, 0xffff);
+    const auto h = std::clamp(screenBufferSize.height, 1, til::CoordTypeMax / 2);
 
     constexpr auto rowSize = ROW::CalculateRowSize();
     const auto charsBufferSize = ROW::CalculateCharsBufferSize(w);
@@ -103,7 +101,7 @@ void TextBuffer::_reserve(til::size screenBufferSize, const TextAttribute& defau
     // 65535*65535 cells would result in a allocSize of 8GiB.
     // --> Use uint64_t so that we can safely do our calculations even on x86.
     // We allocate 1 additional row, which will be used for GetScratchpadRow().
-    const auto rowCount = ::base::strict_cast<uint64_t>(h) + 1;
+    const auto rowCount = gsl::narrow_cast<uint64_t>(h) + 1;
     const auto allocSize = gsl::narrow<size_t>(rowCount * rowStride);
 
     // NOTE: Modifications to this block of code might have to be mirrored over to ResizeTraditional().
@@ -142,31 +140,71 @@ __declspec(noinline) void TextBuffer::_commit(const std::byte* row)
     _construct(_commitWatermark + size);
 }
 
-// Destructs and MEM_DECOMMITs all previously constructed ROWs.
+// Destructs and MEM_DECOMMITs all rows between [rowsToKeep,_commitWatermark).
 // You can use this (or rather the Reset() method) to fully clear the TextBuffer.
-void TextBuffer::_decommit() noexcept
+void TextBuffer::_decommit(til::CoordType rowsToKeep) noexcept
 {
-    _destroy();
-    VirtualFree(_buffer.get(), 0, MEM_DECOMMIT);
-    _commitWatermark = _buffer.get();
+    // Amount of bytes that have been allocated with MEM_COMMIT so far.
+    const auto commitBytes = gsl::narrow_cast<size_t>(_commitWatermark - _buffer.get());
+    auto newWatermark = _buffer.get();
+    size_t pageOffset = 0;
+
+    if (rowsToKeep > 0)
+    {
+        SYSTEM_INFO si;
+        GetSystemInfo(&si);
+
+        rowsToKeep = std::min(rowsToKeep, _height);
+
+        // +1 for the scratchpad row at offset 0.
+        const auto rows = gsl::narrow_cast<size_t>(rowsToKeep) + 1;
+
+        // Offset in bytes to the first row that we were asked to destroy.
+        // We must ensure that the offset is not past the end of the current _commitWatermark,
+        // since we don't want to finish with a watermark that's somehow larger than what we started with.
+        const auto byteOffset = std::min(commitBytes, rows * _bufferRowStride);
+        newWatermark = _buffer.get() + byteOffset;
+
+        // Since the last row we were asked to keep may reside in the middle
+        // of a page, we must round the offset up to the next page boundary.
+        // That offset will tell us the offset at which we will MEM_DECOMMIT memory.
+        const auto pageMask = gsl::narrow_cast<size_t>(si.dwPageSize) - 1;
+        pageOffset = (byteOffset + pageMask) & ~pageMask;
+    }
+
+    // _destroy() takes care to check that the given pointer is valid.
+    _destroy(newWatermark);
+
+    // MEM_DECOMMIT the memory that we don't need anymore.
+    if (pageOffset < commitBytes)
+    {
+        // The analyzer can't know that we're intentionally decommitting memory here.
+#pragma warning(suppress : 6250) // Calling 'VirtualFree' without the MEM_RELEASE flag may free memory but not address descriptors (VADs). This causes address space leaks.
+        VirtualFree(_buffer.get() + pageOffset, commitBytes - pageOffset, MEM_DECOMMIT);
+    }
+
+    _commitWatermark = newWatermark;
 }
 
 // Constructs ROWs between [_commitWatermark,until).
 void TextBuffer::_construct(const std::byte* until) noexcept
 {
+    // _width has been validated to fit into uint16_t during reserve().
+    const auto width = gsl::narrow_cast<uint16_t>(_width);
+
     for (; _commitWatermark < until; _commitWatermark += _bufferRowStride)
     {
         const auto row = reinterpret_cast<ROW*>(_commitWatermark);
         const auto chars = reinterpret_cast<wchar_t*>(_commitWatermark + _bufferOffsetChars);
         const auto indices = reinterpret_cast<uint16_t*>(_commitWatermark + _bufferOffsetCharOffsets);
-        std::construct_at(row, chars, indices, _width, _initialAttributes);
+        std::construct_at(row, chars, indices, width, _initialAttributes);
     }
 }
 
-// Destructs ROWs between [_buffer,_commitWatermark).
-void TextBuffer::_destroy() const noexcept
+// Destructs ROWs between [it,_commitWatermark).
+void TextBuffer::_destroy(std::byte* it) const noexcept
 {
-    for (auto it = _buffer.get(); it < _commitWatermark; it += _bufferRowStride)
+    for (; it < _commitWatermark; it += _bufferRowStride)
     {
         std::destroy_at(reinterpret_cast<ROW*>(it));
     }
@@ -974,7 +1012,7 @@ til::point TextBuffer::BufferToScreenPosition(const til::point position) const
 //   and the default current color attributes
 void TextBuffer::Reset() noexcept
 {
-    _decommit();
+    _decommit(0);
     _initialAttributes = _currentAttributes;
 }
 
@@ -988,30 +1026,22 @@ void TextBuffer::ClearScrollback(const til::CoordType newFirstRow, const til::Co
     {
         return;
     }
-    // The new viewport should keep 0 rows? Then just reset everything.
-    if (rowsToKeep <= 0)
-    {
-        _decommit();
-        return;
-    }
 
-    ClearMarksInRange(til::point{ 0, 0 }, til::point{ _width, std::max(0, newFirstRow - 1) });
-
-    // Our goal is to move the viewport to the absolute start of the underlying memory buffer so that we can
-    // MEM_DECOMMIT the remaining memory. _firstRow is used to make the TextBuffer behave like a circular buffer.
-    // The newFirstRow parameter is relative to the _firstRow. The trick to get the content to the absolute start
-    // is to simply add _firstRow ourselves and then reset it to 0. This causes ScrollRows() to write into
-    // the absolute start while reading from relative coordinates. This works because GetRowByOffset()
-    // operates modulo the buffer height and so the possibly-too-large startAbsolute won't be an issue.
-    const auto startAbsolute = _firstRow + newFirstRow;
-    _firstRow = 0;
-    ScrollRows(startAbsolute, rowsToKeep, -startAbsolute);
-
-    const auto end = _estimateOffsetOfLastCommittedRow();
-    for (auto y = rowsToKeep; y <= end; ++y)
+    if (rowsToKeep > 0)
     {
-        GetMutableRowByOffset(y).Reset(_initialAttributes);
+        // Our goal is to move the viewport to the absolute start of the underlying memory buffer so that we can
+        // MEM_DECOMMIT the remaining memory. _firstRow is used to make the TextBuffer behave like a circular buffer.
+        // The newFirstRow parameter is relative to the _firstRow. The trick to get the content to the absolute start
+        // is to simply add _firstRow ourselves and then reset it to 0. This causes ScrollRows() to write into
+        // the absolute start while reading from relative coordinates. This works because GetRowByOffset()
+        // operates modulo the buffer height and so the possibly-too-large startAbsolute won't be an issue.
+        const auto startAbsolute = _firstRow + newFirstRow;
+        _firstRow = 0;
+        ScrollRows(startAbsolute, rowsToKeep, -startAbsolute);
     }
+
+    // NOTE: If rowsToKeep is 0 we'll fall through to here and just decommit everything.
+    _decommit(rowsToKeep);
 }
 
 // Routine Description:
@@ -1993,7 +2023,7 @@ std::vector<til::point_span> TextBuffer::GetTextSpans(til::point start, til::poi
         const auto rects = GetTextRects(start, end, /*blockSelection*/ true, bufferCoordinates);
         textSpans.reserve(rects.size());
 
-        for (auto rect : rects)
+        for (const auto& rect : rects)
         {
             const til::point first = { rect.left, rect.top };
             const til::point second = { rect.right, rect.bottom };
@@ -2260,7 +2290,7 @@ std::string TextBuffer::GenHTML(const CopyRequest& req,
             const auto [rowBeg, rowEnd, addLineBreak] = _RowCopyHelper(req, iRow, row);
             const auto rowBegU16 = gsl::narrow_cast<uint16_t>(rowBeg);
             const auto rowEndU16 = gsl::narrow_cast<uint16_t>(rowEnd);
-            const auto runs = row.Attributes().slice(rowBegU16, rowEndU16).runs();
+            const auto& runs = row.Attributes().slice(rowBegU16, rowEndU16).runs();
 
             auto x = rowBegU16;
             for (const auto& [attr, length] : runs)
@@ -2510,7 +2540,7 @@ std::string TextBuffer::GenRTF(const CopyRequest& req,
             const auto [rowBeg, rowEnd, addLineBreak] = _RowCopyHelper(req, iRow, row);
             const auto rowBegU16 = gsl::narrow_cast<uint16_t>(rowBeg);
             const auto rowEndU16 = gsl::narrow_cast<uint16_t>(rowEnd);
-            const auto runs = row.Attributes().slice(rowBegU16, rowEndU16).runs();
+            const auto& runs = row.Attributes().slice(rowBegU16, rowEndU16).runs();
 
             auto x = rowBegU16;
             for (auto& [attr, length] : runs)
@@ -2702,7 +2732,7 @@ void TextBuffer::_SerializeRow(const ROW& row, const til::CoordType startX, cons
 
     const auto startXU16 = gsl::narrow_cast<uint16_t>(startX);
     const auto endXU16 = gsl::narrow_cast<uint16_t>(endX);
-    const auto runs = row.Attributes().slice(startXU16, endXU16).runs();
+    const auto& runs = row.Attributes().slice(startXU16, endXU16).runs();
 
     const auto beg = runs.begin();
     const auto end = runs.end();
@@ -3295,7 +3325,7 @@ void TextBuffer::RemoveHyperlinkFromMap(uint16_t id) noexcept
 // - The custom ID if there was one, empty string otherwise
 std::wstring TextBuffer::GetCustomIdFromId(uint16_t id) const
 {
-    for (auto customIdPair : _hyperlinkCustomIdMap)
+    for (const auto& customIdPair : _hyperlinkCustomIdMap)
     {
         if (customIdPair.second == id)
         {
@@ -3491,7 +3521,6 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset,
     bool startedPrompt = false;
     bool startedCommand = false;
     bool startedOutput = false;
-    MarkKind lastMarkKind = MarkKind::Output;
 
     const auto endThisMark = [&](auto x, auto y) {
         if (startedOutput)
@@ -3518,7 +3547,7 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset,
         // Output attribute.
 
         const auto& row = GetRowByOffset(y);
-        const auto runs = row.Attributes().runs();
+        const auto& runs = row.Attributes().runs();
         x = 0;
         for (const auto& [attr, length] : runs)
         {
@@ -3561,8 +3590,6 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset,
 
                     endThisMark(lastMarkedText.x, lastMarkedText.y);
                 }
-                // Otherwise, we've changed from any state -> any state, and it doesn't really matter.
-                lastMarkKind = markKind;
             }
             // advance to next run of text
             x = nextX;
@@ -3595,7 +3622,7 @@ std::wstring TextBuffer::_commandForRow(const til::CoordType rowOffset,
         // Command attributes. Collect up all of those, till we get to the next
         // Output attribute.
         const auto& row = GetRowByOffset(y);
-        const auto runs = row.Attributes().runs();
+        const auto& runs = row.Attributes().runs();
         auto x = 0;
         for (const auto& [attr, length] : runs)
         {

src/buffer/out/textBuffer.hpp

@@ -318,9 +318,9 @@ class TextBuffer final
 private:
     void _reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes);
     void _commit(const std::byte* row);
-    void _decommit() noexcept;
+    void _decommit(til::CoordType keep) noexcept;
     void _construct(const std::byte* until) noexcept;
-    void _destroy() const noexcept;
+    void _destroy(std::byte* it) const noexcept;
     ROW& _getRowByOffsetDirect(size_t offset);
     ROW& _getRow(til::CoordType y) const;
     til::CoordType _estimateOffsetOfLastCommittedRow() const noexcept;
@@ -384,14 +384,14 @@ class TextBuffer final
     // In other words, _commitWatermark itself will either point exactly onto the next ROW
     // that should be committed or be equal to _bufferEnd when all ROWs are committed.
     std::byte* _commitWatermark = nullptr;
-    // This will MEM_COMMIT 128 rows more than we need, to avoid us from having to call VirtualAlloc too often.
+    // This will MEM_COMMIT 256 rows more than we need, to avoid us from having to call VirtualAlloc too often.
     // This equates to roughly the following commit chunk sizes at these column counts:
-    // *  80 columns (the usual minimum) =  60KB chunks,  4.1MB buffer at 9001 rows
-    // * 120 columns (the most common)   =  80KB chunks,  5.6MB buffer at 9001 rows
-    // * 400 columns (the usual maximum) = 220KB chunks, 15.5MB buffer at 9001 rows
+    // *  80 columns (the usual minimum) = 131KB chunks,  4.6MB buffer at 9001 rows
+    // * 120 columns (the most common)   = 172KB chunks,  6.0MB buffer at 9001 rows
+    // * 400 columns (the usual maximum) = 459KB chunks, 16.1MB buffer at 9001 rows
     // There's probably a better metric than this. (This comment was written when ROW had both,
     // a _chars array containing text and a _charOffsets array contain column-to-text indices.)
-    static constexpr size_t _commitReadAheadRowCount = 128;
+    static constexpr size_t _commitReadAheadRowCount = 256;
     // Before TextBuffer was made to use virtual memory it initialized the entire memory arena with the initial
     // attributes right away. To ensure it continues to work the way it used to, this stores these initial attributes.
     TextAttribute _initialAttributes;
@@ -405,9 +405,9 @@ class TextBuffer final
     size_t _bufferOffsetChars = 0;
     size_t _bufferOffsetCharOffsets = 0;
     // The width of the buffer in columns.
-    uint16_t _width = 0;
+    til::CoordType _width = 0;
     // The height of the buffer in rows, excluding the scratchpad row.
-    uint16_t _height = 0;
+    til::CoordType _height = 0;
 
     TextAttribute _currentAttributes;
     til::CoordType _firstRow = 0; // indexes top row (not necessarily 0)

src/cascadia/TerminalCore/Terminal.cpp

@@ -44,8 +44,7 @@ void Terminal::Create(til::size viewportSize, til::CoordType scrollbackLines, Re
 {
     _mutableViewport = Viewport::FromDimensions({ 0, 0 }, viewportSize);
     _scrollbackLines = scrollbackLines;
-    const til::size bufferSize{ viewportSize.width,
-                                Utils::ClampToShortMax(viewportSize.height + scrollbackLines, 1) };
+    const til::size bufferSize{ viewportSize.width, viewportSize.height + scrollbackLines };
     const TextAttribute attr{};
     const UINT cursorSize = 12;
     _mainBuffer = std::make_unique<TextBuffer>(bufferSize, attr, cursorSize, true, &renderer);
@@ -67,7 +66,7 @@ void Terminal::CreateFromSettings(ICoreSettings settings,
                                   Utils::ClampToShortMax(settings.InitialRows(), 1) };
 
     // TODO:MSFT:20642297 - Support infinite scrollback here, if HistorySize is -1
-    Create(viewportSize, Utils::ClampToShortMax(settings.HistorySize(), 0), renderer);
+    Create(viewportSize, std::clamp(settings.HistorySize(), 0, til::CoordTypeMax / 2), renderer);
 
     UpdateSettings(settings);
 }

src/cascadia/TerminalSettingsEditor/Profiles_Advanced.cpp

@@ -22,6 +22,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         InitializeComponent();
 
         Automation::AutomationProperties::SetName(AddBellSoundButton(), RS_(L"Profile_AddBellSound/Text"));
+
+        // The XAML -> C++ converter seems to use floats instead of doubles, which means it
+        // can't represent large numbers accurately (lol!). So, we set the property manually.
+        HistorySizeBox().Maximum(static_cast<double>(til::CoordTypeMax / 2));
     }
 
     void Profiles_Advanced::OnNavigatedTo(const NavigationEventArgs& e)

src/cascadia/TerminalSettingsEditor/Profiles_Advanced.xaml

@@ -71,7 +71,8 @@
                                     ClearSettingValue="{x:Bind Profile.ClearHistorySize}"
                                     HasSettingValue="{x:Bind Profile.HasHistorySize, Mode=OneWay}"
                                     SettingOverrideSource="{x:Bind Profile.HistorySizeOverrideSource, Mode=OneWay}">
-                <muxc:NumberBox x:Uid="Profile_HistorySizeBox"
+                <muxc:NumberBox x:Name="HistorySizeBox"
+                                x:Uid="Profile_HistorySizeBox"
                                 LargeChange="100"
                                 Minimum="0"
                                 SmallChange="10"