Skip to content

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Dec 6, 2024

Mom: We have infinite scrollback at home.
Infinite scrollback at home:

...very much finite. How do you define "infinite" scrollback anyway?
Does it entail downloading RAM? It's a mystery. So, this PR adds support
for 1B rows of scrollback, because that's >600GB of memory, which makes
it effectively unusable. But it's enough to have a lot of finity.

Thanks to the raised limit, this PR sets the default history size from
to 16Ki which is roughly 10MB of memory. The can be cut in half once we
remove the charOffsets cache on each ROW.

Related to #1410

Validation Steps Performed

  • Set scrollback to 1M lines
  • Dump 100MB of text and watch the RSS grow
  • Reflow the window (it lags a little) ✅
  • Clear the buffer (it's fast) ✅

This comment has been minimized.

void _decommit() noexcept;
void _construct(const std::byte* until) noexcept;
void _destroy() const noexcept;
void _commit(const uint8_t* row);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm why these?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so that cls releases the memory again. I figured that this would become much more important once the scrollback can actually become quite large.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you meant std::byte vs. uint8_t. We only use the former in a teeny tiny subset of the codebase, of which this usage here is half of it. By using the more traditional uint8_t we make our code simpler and need less specialization. It's also more predictable, because primitive integers have received compiler optimizations over decades whereas std::byte hasn't.
I'm reminded of that one blog post that showed how poorly std::byte was optimized by compilers at the time, but I was unable to find it while writing this comment.

@lhecker lhecker marked this pull request as draft December 6, 2024 23:33
@lhecker lhecker force-pushed the dev/lhecker/1410-large-scrollback branch from 5500606 to e6acccf Compare December 11, 2024 20:34
const auto wroteWholeBuffer = lengthToWrite == (currentBufferDimensions.area<size_t>());
const auto startedAtOrigin = startingCoordinate == til::point{ 0, 0 };
const auto wroteSpaces = attribute == (FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED);
const auto currentBufferDimensions{ OutContext.GetBufferSize().Dimensions() };
Copy link
Member Author

@lhecker lhecker Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Suppress whitespace differences. My goal here is to make cls work properly in regular conhost.)

@lhecker lhecker force-pushed the dev/lhecker/1410-large-scrollback branch from e6acccf to 3567827 Compare December 11, 2024 20:45
@DHowett
Copy link
Member

DHowett commented Feb 20, 2025

fwiw this is still draft

@zadjii-msft zadjii-msft linked an issue Aug 13, 2025 that may be closed by this pull request
@lhecker lhecker force-pushed the dev/lhecker/1410-large-scrollback branch from 3567827 to 26120be Compare September 1, 2025 15:44
@lhecker lhecker force-pushed the dev/lhecker/1410-large-scrollback branch from 26120be to 971dba7 Compare September 1, 2025 21:42
@lhecker lhecker marked this pull request as ready for review September 1, 2025 21:42

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Finite (but large) scrollback (not infinite)
2 participants