Throttle cursor redrawing in outputStream.cpp #10394
Conversation
|
I know conhost code are in maintenance mode so I'm marking this as a draft. This should help both conhost & WT (OpenConsole). |
| @@ -76,8 +76,13 @@ void WriteBuffer::_DefaultStringCase(const std::wstring_view string) | |||
| { | |||
| size_t dwNumBytes = string.size() * sizeof(wchar_t); | |||
|
|
|||
| _io.GetActiveOutputBuffer().GetTextBuffer().GetCursor().SetIsOn(true); | |||
| Cursor& cursor = _io.GetActiveOutputBuffer().GetTextBuffer().GetCursor(); | |||
| if (!cursor.IsOn()) | |||
There was a problem hiding this comment.
Here SetIsOn() calls RedrawCursorAlways() and causes unnecessary overhead.
I feel this one is safe, since we deliberately want the cursor to be on and visible.
src/host/outputStream.cpp
Outdated
|
|
||
| //cursor.StartDeferDrawing(); |
There was a problem hiding this comment.
I'm no sure if this is a safe change, though.
This reduces the unnecessary cursor redrawing caused by WriteCharsLegacy where a lot of cursor position adjustment happens.
There was a problem hiding this comment.
Why wouldn't this be a safe change?
There was a problem hiding this comment.
TBH it's because I'm afraid of WriteCharsLegacy, one of the most easy-to-break piece of code in conhost.
There was a problem hiding this comment.
No, do it. Please. It honestly was deferred like 7 years ago and I think I screwed it up and it's how I've been grinding out "cursor turds" bugs ever since. And if I'm wrong.... well it won't be the first or last time.
skyline75489
changed the title
|
I see no obvious cursor rendering issue after this. So I'm marking this ready for review. |
skyline75489
changed the title
DHowett
added
the
AutoMerge
|
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
| Cursor& cursor = _io.GetActiveOutputBuffer().GetTextBuffer().GetCursor(); | ||
| if (!cursor.IsOn()) | ||
| { | ||
| cursor.SetIsOn(true); |
There was a problem hiding this comment.
I am surprised that Cursor does not ignore an "on->on" transition itself
|
🎉 Handy links: |
Try to throttle the cursor redrawing in the conhost world.
The motivation of this is the high CPU usage of
TriggerRedrawCursor(#10393).This can be seen as the conhost version of #2960.
This saves 5%~8% of the CPU time.
Supports #10462.