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.

Sign up for GitHub

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

Jump to bottom

Should we have a ThrottledFunc that works without a dispatcher? #10393

Closed
skyline75489 opened this issue Jun 10, 2021 · 3 comments · Fixed by #10403
Closed

Should we have a ThrottledFunc that works without a dispatcher? #10393

skyline75489 opened this issue Jun 10, 2021 · 3 comments · Fixed by #10403
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@skyline75489
Copy link
Collaborator

I found that I need something like ThrottledFunc, but works in DxRenderer. There's something that's such a pain point, is that we're wasting valuable CPU times redrawing cursors when we actually don't need to . I know this long ago, just hit this again when investigating #10362 . This is an example WPR trace:

image

To update the cursor, the underlying buffer is frequently checked for IsCursorDoubleWidth, which consumes valuable CPU times and totally not necessary.

A simple solution would be remember the timestamp of the last update and refuse to update it until a certain period of time passes.
@skyline75489 skyline75489 added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. labels Jun 10, 2021
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jun 10, 2021
@skyline75489 skyline75489 mentioned this issue Jun 10, 2021
Merged
@skyline75489
Copy link
Collaborator Author

CC @lhecker for thoughts

@lhecker
Copy link
Member

lhecker commented Jun 10, 2021

@skyline75489 I wrote a til::throttled_func without a dispatcher just yesterday. hahahahaha
I can contribute it separately today if you want to. 👍

@lhecker lhecker mentioned this issue Jun 10, 2021
Merged
@ghost ghost added the In-PR This issue has a related PR label Jun 10, 2021
@skyline75489 skyline75489 mentioned this issue Jun 21, 2021
Open
@ghost ghost closed this as completed in #10403 Jun 22, 2021
@ghost ghost removed the In-PR This issue has a related PR label Jun 22, 2021
ghost pushed a commit that referenced this issue Jun 22, 2021
@lhecker
Introduce til/latch.h, til/mutex.h and til/throttled_func.h (#10403)
0d9a357 
This commit introduce three new `til` features:
* "til/latch.h": A std::latch clone, until we're on C++20.
* "til/mutex.h": A safe mutex wrapper, which only allows you access to the protected data after locking it. No more forgetting to lock mutexes!
* "til/throttled_func.h": Function invocation throttling used to be available as the `ThrottledFunc` class already. But this class is vastly more efficient and doesn't rely on any WinRT types.

This PR also adds a `til::ends_with` string helper which is `til::starts_with` counterpart.

## Validation Steps Performed

* Scrollbar throttling still works as it used to ✔️
* No performance regressions when printing big.txt ✔️

Closes #10393
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Jun 22, 2021
ghost pushed a commit that referenced this issue Jun 28, 2021
@skyline75489
Throttle cursor redrawing in outputStream.cpp (#10394)
4fc283f 
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.
DHowett pushed a commit that referenced this issue Jul 7, 2021
@lhecker @DHowett
Introduce til/latch.h, til/mutex.h and til/throttled_func.h (#10403)
1b996e7 
This commit introduce three new `til` features:
* "til/latch.h": A std::latch clone, until we're on C++20.
* "til/mutex.h": A safe mutex wrapper, which only allows you access to the protected data after locking it. No more forgetting to lock mutexes!
* "til/throttled_func.h": Function invocation throttling used to be available as the `ThrottledFunc` class already. But this class is vastly more efficient and doesn't rely on any WinRT types.

This PR also adds a `til::ends_with` string helper which is `til::starts_with` counterpart.

* Scrollbar throttling still works as it used to ✔️
* No performance regressions when printing big.txt ✔️

Closes #10393

(cherry picked from commit 0d9a357)
@ghost
Copy link

ghost commented Jul 14, 2021

🎉This issue was addressed in #10403, which has now been successfully released as Windows Terminal Preview v1.10.1933.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Introduce til/latch.h, til/mutex.h and til/throttled_func.h microsoft/terminal
2 participants
@lhecker @skyline75489