Release unneeded memory more eagerly from conhost #10738
Conversation
ghost
added
Area-Server
| _outputBuffer.resize(cbWriteSize); | ||
|
|
||
| // 0 it out. | ||
| std::fill(_outputBuffer.begin(), _outputBuffer.end(), (BYTE)0); | ||
| std::fill_n(_outputBuffer.data(), _outputBuffer.size(), BYTE(0)); |
There was a problem hiding this comment.
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().
lhecker
force-pushed
the
dev/lhecker/server-memory-limit
branch
from
524effd
to
e023dfa
Compare
|
@msftbot make sure @miniksa signs off |
ghost
added
the
AutoMerge
|
Hello @DHowett! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
ghost
merged commit 
|
@lhecker Once that I worked on the code I was facing a huge performance drop caused by I'm absolutely aware that your latest update is not the same as what I originally did. I called |
|
@german-one Yeah in this case specifically we're only dropping the buffer after calling |
The `_CONSOLE_API_MSG` buffer is resized to cover an entire message. Later on any UTF-8 data is cached in a separate temporary buffer inside `til::u8state` to prevent lone surrogate pairs. Both cases are problematic as neither buffer is freed after the read has finished. Passing a 100MB buffer to conhost once will thus cause it to continue using ~220MB of physical memory until the conhost process exits. This change releases unneeded memory as soon as the requested buffer size has halved. In practice this means that once a command has returned all buffers will shrink, as the shell commonly sends very small messages. ## PR Checklist * [x] Closes microsoft#10731 * [x] I work here * [x] Tests added/passed ## Validation Steps Performed * Buffers aren't reallocated during printing ✔️ * Buffers shrink after printing finished ✔️
The `_CONSOLE_API_MSG` buffer is resized to cover an entire message. Later on any UTF-8 data is cached in a separate temporary buffer inside `til::u8state` to prevent lone surrogate pairs. Both cases are problematic as neither buffer is freed after the read has finished. Passing a 100MB buffer to conhost once will thus cause it to continue using ~220MB of physical memory until the conhost process exits. This change releases unneeded memory as soon as the requested buffer size has halved. In practice this means that once a command has returned all buffers will shrink, as the shell commonly sends very small messages. ## PR Checklist * [x] Closes #10731 * [x] I work here * [x] Tests added/passed ## Validation Steps Performed * Buffers aren't reallocated during printing ✔️ * Buffers shrink after printing finished ✔️
|
🎉 Handy links: |
ghost
mentioned this pull request
|
🎉 Handy links: |
The
_CONSOLE_API_MSGbuffer is resized to cover an entire message.Later on any UTF-8 data is cached in a separate temporary
buffer inside
til::u8stateto prevent lone surrogate pairs.Both cases are problematic as neither buffer is freed after the read
has finished. Passing a 100MB buffer to conhost once will thus cause it
to continue using ~220MB of physical memory until the conhost process exits.
This change releases unneeded memory as soon as the requested buffer
size has halved. In practice this means that once a command has returned
all buffers will shrink, as the shell commonly sends very small messages.
PR Checklist
Validation Steps Performed