-
Notifications
You must be signed in to change notification settings - Fork 8.8k
Description
Description of the new feature
Currently the maximum historysize is 32767
, which is really a small number and quite insufficient. There is also another feature request #1410 to set the line limit to infinity.
This is not a duplicate of that issue because increasing the limit only requires a small code change to just increase the buffer limit without losing performance, whereas infinity requires more complicated code changes to make the buffer able to resize/extend (maybe using a rope).
I'm saying without losing performance because #15524 was merged, now it's using virtual memory allocations so that the "real" memory use increases only when you really use that much buffer.
And the code change is quite small, just change these Utils::ClampToShortMax
and gsl::narrow<uint16_t>
.
// src/cascadia/TerminalCore/Terminal.cpp:
// void Terminal::Create(til::size viewportSize, til::CoordType scrollbackLines, Renderer& renderer)
const til::size bufferSize{ viewportSize.width,
Utils::ClampToShortMax(viewportSize.height + scrollbackLines, 1) };
// void Terminal::CreateFromSettings(ICoreSettings settings, Renderer& renderer)
Create(viewportSize, Utils::ClampToShortMax(settings.HistorySize(), 0), renderer);
// src/buffer/out/textBuffer.cpp
// void TextBuffer::_reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes)
const auto h = gsl::narrow<uint16_t>(screenBufferSize.height);
// src/buffer/out/textBuffer.hpp
// class TextBuffer final
uint16_t _height = 0;
Currently these code effectively limits the max buffer size(height) to 32767, we can just increase the limit here.
Proposed technical implementation details
- We can just change the hard limit in
TextBuffer
toint32_t (2^31-1)
becauseCoordType
is currentlyint32_t
and it is large enough.- We can change the code above in
textBuffer.hpp|cpp
to useCoordType
orint32_t
- We can also change type of
_width
for alignment. Alsoint16_t
is slow on modern x86(ia32/x64)/ARM64 CPUs (although it's not the key point here but nice to have)uint16_t _width = 0; uint16_t _height = 0;
- We can change the code above in
- Relax the hard limit when creating the
Terminal
class- maybe clamps to
INT_MAX
- maybe clamps to
- Better to have another layer of soft limit in
settings
- maybe we don't need
INT_MAX,
we can really set it to something like999'999'999
or100'000'000
. The reason here is because I think many people (like me) hates the 32767 limit (but not aware there is a hard limit inside) and fill tons of9
s in the settings. This helps reduce the virtual allocation size because virtual address space is still a resource and wasting it too much may have a side-effect that limits down the maximum tabs a user can open. - After the change we can let users know there was a hard limit and now it's increased, please revise your settings of this value blah blah, so that if a user previously really set it to more than 10
9
s and clamps toINT_MAX
, the user can set it to a more reasonable value to avoid potential issues like stated above.
- maybe we don't need