Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release unneeded memory more eagerly from conhost #10738

Merged
1 commit merged into from
Jul 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/inc/til/u8u16convert.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
RETURN_HR_IF(E_ABORT, !base::CheckAdd(in.length(), _partialsLen).AssignIfValid(&capacity));

_buffer.clear();

// If we were previously called with a huge buffer we have an equally large _buffer.
// We shouldn't just keep this huge buffer around, if no one needs it anymore.
if (_buffer.capacity() > 16 * 1024 && (_buffer.capacity() >> 1) > capacity)
{
_buffer.shrink_to_fit();
}

_buffer.reserve(capacity);

// copy UTF-8 code units that were remaining from the previous call (if any)
Expand Down
18 changes: 16 additions & 2 deletions src/server/ApiMessage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,14 @@ try
{
RETURN_HR_IF(E_FAIL, State.ReadOffset > Descriptor.InputSize);

ULONG const cbReadSize = Descriptor.InputSize - State.ReadOffset;
const ULONG cbReadSize = Descriptor.InputSize - State.ReadOffset;

// If we were previously called with a huge buffer we have an equally large _inputBuffer.
// We shouldn't just keep this huge buffer around, if no one needs it anymore.
if (_inputBuffer.capacity() > 16 * 1024 && (_inputBuffer.capacity() >> 1) > cbReadSize)
{
_inputBuffer.shrink_to_fit();
}

_inputBuffer.resize(cbReadSize);

Expand Down Expand Up @@ -145,10 +152,17 @@ try
ULONG cbWriteSize = Descriptor.OutputSize - State.WriteOffset;
RETURN_IF_FAILED(ULongMult(cbWriteSize, cbFactor, &cbWriteSize));

// If we were previously called with a huge buffer we have an equally large _outputBuffer.
// We shouldn't just keep this huge buffer around, if no one needs it anymore.
if (_outputBuffer.capacity() > 16 * 1024 && (_outputBuffer.capacity() >> 1) > cbWriteSize)
{
_outputBuffer.shrink_to_fit();
}

_outputBuffer.resize(cbWriteSize);

// 0 it out.
std::fill(_outputBuffer.begin(), _outputBuffer.end(), (BYTE)0);
std::fill_n(_outputBuffer.data(), _outputBuffer.size(), BYTE(0));
Copy link
Member Author

@lhecker lhecker Jul 20, 2021

Choose a reason for hiding this comment

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

This was sort of a programming error that I fixed on the side.
<algorithm> doesn't work properly with non-STL containers, so it's important that we pass it raw pointers if we can. Otherwise it can't figure out what to do with it and sets each byte to 0 one by one in this case for instance.
This one will now properly compile to a single call to memset().

Copy link
Member

Choose a reason for hiding this comment

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

Excellent. Thanks!


State.OutputBuffer = _outputBuffer.data();
State.OutputBufferSize = cbWriteSize;
Expand Down