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

Extremely slow performance when processing virtual terminal sequences #10462

Open
CoreyDotCom opened this issue Jun 19, 2021 · 48 comments
Open
Labels
Area-Performance Performance-related issue Impact-Correctness It be wrong. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Conpty For console issues specifically related to conpty Product-Meta The product is the management of the products.
Milestone

Comments

@CoreyDotCom
Copy link

Windows Terminal version (or Windows build number)

1.8.1521.0

Other Software

No response

Steps to reproduce

Using any command line utility that produces virtual terminal sequences for setting the colors of individual characters, the performance of the terminal drops by a factor of around 40.

To measure this effect precisely, you can use the F2 key in termbench and observe the performance difference between color-per-character output and single-color output:

https://github.com/cmuratori/termbench/releases/tag/V1

Expected Behavior

Despite the increased parsing load, modern CPUs should not have a problem parsing per-character color escape codes quickly. I would expect the performance of the terminal to be able to sustain roughly the same frame rate with per-character color codes as without, and if there was a performance drop, I wouldn't expect it to be anything close to 40x.

Actual Behavior

The speed of per-character color output is 40x slower than the speed of single-color output.

@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 19, 2021
@CoreyDotCom
Copy link
Author

CoreyDotCom commented Jun 19, 2021

Note that this is a clone of #10362 which was closed by original submitter. As the original bug report was valid it should have been remained open, especially after quite a few suggestions were provided as to how to improve matters. Feel free to reopen original tracking issue and close this duplicate if appropriate. Thank you for the work that you guys do on this project.

@CoreyDotCom CoreyDotCom changed the title https://github.com/microsoft/terminal/issues/10362 Extremely slow performance when processing virtual terminal sequences Jun 19, 2021
@DHowett
Copy link
Member

DHowett commented Jun 19, 2021

Thanks for re-opening this! It’s definitely valuable for us to be tracking it, and the ideas suggested in the original thread are good ones that are worth considering. We also have #10461 tracking specific requests from #10362.

@lhecker
Copy link
Member

lhecker commented Jun 19, 2021

Personally I'd like to also welcome @cmuratori and @mmozeiko back to give any comments if they wish to do so. Of course it'd be entirely understandable if they don't. I believe I can speak for the terminal team, when I say that we want to keep the door to our community open and welcome any civil discourse.


I see the comment regarding slow VT parsing again, but I hope you consider that performance of OpenConsole (aka future conhost) has already improved about 4-5x in the last year. With upcoming conhost versions, or the current OpenConsole, you can expect to see a throughput improvement from 4MB/s to about 16-20MB/s for termbench. I know this isn't 270MB/s, but it's a significant improvement regardless, despite keeping all the backwards-compatibility, etc.

For instance - referring to the mentioned offenders:

We're now working on improving _SetGraphicsRenditionRGBColor in #10426, which will improve performance for termbench by another 20-25%. FYI: @skyline75489 is working on this entirely in their spare time! (...and the PR is in fact not finished yet. It'll likely feature more improvements once it's done.)

Here's a flamegraph of OpenConsole for termbench:
openconsole_termbench_flamegraph

The render output thread is highlighted in blue. _SetGraphicsRenditionRGBColor is being hovered.

If you have concrete ideas what to improve (and optionally how) to speed up OpenConsole's VT performance further, please let us know and we'll get to it. Promise! Contributions are especially welcome at any time!

@lhecker lhecker added Area-Performance Performance-related issue Product-Conpty For console issues specifically related to conpty labels Jun 19, 2021
@mmozeiko
Copy link

Is there a possibility we (outside of Microsoft) can get some documentation or specification on console driver interface -\\Device\\ConDrv\\Server, structures in condrv.h and conmsgl*.h files ?

@DHowett
Copy link
Member

DHowett commented Jun 19, 2021

Fair question! We should definitely have some docs on the driver interface, since we're now an open source project that is using an underdocumented OS fixture...

Right now the best we have is in the ApiDispatcher and some of the "load bearing code" comments across the codebase. That's not great. I'll see what we can do about that!

@lhecker
Copy link
Member

lhecker commented Jun 19, 2021

@mmozeiko Sure! Let's continue that discussion here: #10463

@skyline75489

This comment has been minimized.

@skyline75489

This comment has been minimized.

@mmozeiko

This comment has been minimized.

@skyline75489

This comment has been minimized.

@lhecker

This comment has been minimized.

@nico-abram
Copy link
Contributor

This is sadly not the truth at the moment. I've tested a bunch of terminals, including GNOME Terminal, rxvt, tilix and Alacritty (perhaps the fastest terminal emulator for the moment being), they both show significant performance drop with heavy VT load. See alacritty/alacritty#5278 for details. The maintainer of Alacritty seems to concur with this conclusion: the performance drop with massive VT load is reasonable & expected.

To be clear: You are saying this can be the case for just the rendering part, but that it can be slower because of VT processing (Which as I understand it, happens mostly in conhost)?

Also, is it possible Alacritty (And all the other terminals you may have tried on windows) are being slowed down by an older version of conhost and slower processing of VT sequences in it than the current OpenConsole? (Possibly making it an unfair comparison)

@skyline75489
Copy link
Collaborator

skyline75489 commented Jun 26, 2021

@Nicholas-Baron I don’t quite get it what you are asking. I was testing those Linux terminals on barebone Linux machine.

Which as I understand it, happens mostly in conhost

This is not correct. I think you don’t quite know the full picture of ConPTY. VT processing happens in both conhost & Windows Terminal. This is also true with other terminals on Windows that uses ConPTY. If I have to guess the ratio, I’d probably say 60% in conhost and 40% in the terminal? Just a rough guess. Not precise numbers.

@Nicholas-Baron
Copy link
Contributor

@Nicholas-Baron I don’t quite get it what you are asking. I was testing those Linux terminals on barebone Linux machine.

@skyline75489, I think you have the wrong person.

@skyline75489
Copy link
Collaborator

Yep sorry. On my phone 😅. @nico-abram

ghost pushed a commit that referenced this issue Jun 28, 2021
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 DHowett added Impact-Correctness It be wrong. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Meta The product is the management of the products. labels Jul 2, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 2, 2021
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jul 2, 2021
@DHowett DHowett added this to the Terminal v2.0 milestone Jul 2, 2021
@DHowett DHowett added the Priority-1 A description (P1) label Jul 2, 2021
@dmitriid

This comment has been minimized.

@christianparpart

This comment has been minimized.

@dmitriid

This comment has been minimized.

@dmitriid

This comment has been minimized.

@christianparpart

This comment has been minimized.

@uspasojevic96

This comment has been minimized.

@lhecker

This comment has been minimized.

@ndwork

This comment was marked as off-topic.

@microsoft microsoft locked as too heated and limited conversation to collaborators Nov 2, 2021
@DHowett
Copy link
Member

DHowett commented Nov 2, 2021

We certainly handled the original issue poorly, and we're trying to do better.

Casey showed us, with his own particular brand of "showing," that our terminal emulator could be more performant.

Valuable feedback has been provided, with proof, and summarily dismissed.

We may not have taken the feedback seriously to begin with, but if it appears to have been completely dismissed then I am sorry.

Since July, we've begun work on a new renderer implementing the improvements Casey suggested and removed a number of performance bottlenecks along the way (resulting in a ~30% speed improvement for bulk text output and a ~15% speed improvement for VT parsing.) Yes, we move slowly. Whether that is a reasonable cause for community ire is not something I intend to address.

To @ndwork's point: the question of "evolution versus revolution" is a worthwhile one to ask. Why do we care so much about keeping what we have and not stopping to rewrite it? I can only answer from my perspective as the engineering lead here.

  • It was very effective for us to base Terminal off of some code we shared with the Windows Console Host, in a literal The Mythical Man-Month sense. We had the code already and it worked well enough for us to ship something usable by the vast majority of people.
  • The improvements we make to the code shared with the console host--assuming that we keep roughly the same underpinnings--go back into Windows to make the console better for everyone, not just Terminal users. A rewrite would (in my opinion) jeopardize our ability to do that unless we put that in-scope for the rewrite. The performance issue absolutely pervades both of these components, so it should be in scope.
  • However, on rewriting the console bits: I feel like the console is a pretty big burden. It holds us back not because we're afraid of it, but because invariably some OEM will come back to us when Windows is about to ship with some issue that brought down their production lines. This happens even when we're not rewriting things, honestly. Maybe that's just a fancy explanation for what is essentially just fear.
  • Given those, though... I just can't afford to take half my engineering team offline to reimplement both VT parsing + rendering (classically, Terminal things) as well as the Console subsystem/Win32 APIs (all the things that our OEMs would notice and file bugs on).

Unlike other folks, I'm just not in it to kill the old and replace it with the new. That's never been a winning strategy, has it? Everybody else seems like they're rolling out a new rewrite every year, or every two years, that has 20% of the features of the original and the team pretty much shuts down when they hear that users are (quelle surprise!) dissatisfied with their great new thing.

I feel like our approach, while slow, is at least measured.


Now.
This is not really the appropriate venue to discuss:

  • Our company's valuation
  • My team's staffing

It is the appropriate venue to discuss:

  • What we are doing to improve the "extremely slow performance when processing ..."
  • How we can match or exceed the capabilities of other terminals

I'm leaving this issue unlocked. Let's work together to keep it on topic!

Edit log
  • On 11/4, I reworded the section about killing the old/replacing it with the new.
  • At 01:24 CDT 11/2, I removed a comment about our competitors.

@microsoft microsoft unlocked this conversation Nov 2, 2021
@ndwork
Copy link

ndwork commented Nov 2, 2021

Thank you for your response.

Why were so many comments deleted? It's hard to accept your communal statements when you discard so many of the comments. Indeed, it seems that my comment which you are addressing has been deleted. (Or, I don't really know how to use this messaging board, the comment is there, and I just don't see it.)

Perhaps the reason that the code is so difficult to improve is because of the dependencies. Is it the role of the Terminal to improve the console host? Or should Terminal just focus on being an excellent terminal. Which would serve Microsoft better? In my mind, retaining a manageable scope (making a good terminal) has much more valuable than minimally improving Windows by contributing to some massive underlying library.

I'm just not in it to kill the old and replace it with the new. That's never been a winning strategy, has it?

Based on this idea, Apple would still be using slow Motorolla chips. Instead, they moved to Intel (which Microsoft still uses) and have just recently exceeded Intel's capabilities (for laptops) with their own chips. In my own work, I routinely destroy old codes and replace them with new. By doing so regularly, I never build up too much technical debt. Instead, it's paid off daily, and the codes naturally adapt. Having said that, I'm not a lead of a product at Microsoft used by gazillions; I cannot comment intelligently on your situation. There may be great reasons not to do so; based on the above, I don't understand those reasons. And when one programmer can exceed the performance of a Microsoft product by factors of hundreds with less than two days of programming, I would stare that fear of change in the face, ask exactly what you're afraid of, and is there a way to mitigate that consequence while still achieving. Perhaps limiting the scope of the terminal and eliminating its dependency on the console host is a reasonable approach.

I offer the following statement by Isaacson in the biography on Jobs:

“One of Job's business rules was to never be afraid of cannibalizing yourself. " If you don't cannibalize yourself, someone else will," he said. So even though an Iphone might cannibalize sales of an IPod, or an IPad might cannibalize sales of a laptop, that did not deter him.”

Again, thank you for your response.

@DHowett
Copy link
Member

DHowett commented Nov 2, 2021

(Or, I don't really know how to use this messaging board, the comment is there, and I just don't see it.)

Ah, I minimized the comments above before I locked the thread. They can be expanded with the "Show comment" link at the righthand side for now, but I'll also go through and unminimize some of them. Sorry!

"...never be afraid of cannibalizing yourself..."

I like that. You're right, of course. These are all really good points for us to think about. 😄

@plasticalligator

This comment was marked as spam.

@christianparpart

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Performance Performance-related issue Impact-Correctness It be wrong. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Conpty For console issues specifically related to conpty Product-Meta The product is the management of the products.
Projects
None yet
Development

No branches or pull requests