Add support for renderer backoff, don't FAIL_FAST on 3x failures, add UI by DHowett-MSFT · Pull Request #5353 · microsoft/terminal

DHowett-MSFT · 2020-04-14T19:33:19Z

Summary of the Pull Request

Renderer: Add support for backoff and auto-disable on failed retry

This commit introduces a backoff (150ms * number of tries) to the
renderer's retry logic (introduced in #2830). It also changes the
FAIL_FAST to a less globally-harmful render thread disable, so that we
stop blowing up any application hosting a terminal when the graphics
driver goes away.

In addition, it adds a callback that a Renderer consumer can use to
determine when the renderer has failed, and a public method to kick it
back into life.

Fixes #5340.

This PR also wires up TermControl so that it shows some UI when the renderer tastes clay.

PR Checklist

Closes Crash in render engine due to directx device hanging up #5340
cla
Tests added/passed
Requires documentation to be updated
I've discussed this with core contributors already.

Validation Steps Performed

I tested this by dropping the number of retries to 1 and forcing a TDR while doing wsl cmatrix -u0. It picked up exactly where it left off.

As a bonus, you can actually still type into the terminal when it's graphically suspended (and exit still works.). The block is entirely graphical.

This commit introduces a backoff (150ms * number of tries) to the renderer's retry logic (introduced in #2830). It also changes the FAIL_FAST to a less globally-harmful render thread disable, so that we stop blowing up any application hosting a terminal when the graphics driver goes away. In addition, it adds a callback that a Renderer consumer can use to determine when the renderer _has_ failed, and a public method to kick it back into life. Fixes #5340.

zadjii-msft

Only minor questions tbh

src/cascadia/TerminalControl/TermControl.xaml

zadjii-msft · 2020-04-14T19:40:26Z

src/renderer/base/renderer.cpp

                }
+                // Add a bit of backoff.
+                // Sleep 150ms, 300ms, 450ms before failing out and disabling the renderer.
+                Sleep(150 * (maxRetriesForRenderEngine - tries));


Any particular reason for the backoff like this? just to give it another chance?

Under the idea that the graphics driver probably went down for a real reason, and that restarting the card might take some time if that's what it needs to do.

zadjii-msft · 2020-04-14T19:40:54Z

src/renderer/base/renderer.cpp

                }
+                // Add a bit of backoff.
+                // Sleep 150ms, 300ms, 450ms before failing out and disabling the renderer.
+                Sleep(150 * (maxRetriesForRenderEngine - tries));


Should 150 be a constant somewhere? 🤷‍♂️

src/renderer/base/renderer.cpp

zadjii-msft · 2020-04-14T19:43:34Z

@msftbot make sure @miniksa signs off on this

ghost · 2020-04-14T19:43:38Z

Hello @zadjii-msft!

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:

I'll only merge this pull request if it's approved by @miniksa

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".

miniksa · 2020-04-14T19:46:33Z

Looks alright, but I'd like to see @zadjii-msft's questions answered and @cinnamon-msft give this a pass for how it looks before we commit it.

zadjii-msft

alright I'm cool with this

miniksa

Sure ok.

DHowett-MSFT · 2020-04-14T19:54:11Z

@msftbot merge this in 2 minutes

ghost · 2020-04-14T19:54:15Z

Hello @DHowett-MSFT!

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:

I'll only merge this pull request if it's approved by @miniksa
I won't merge this pull request until after the UTC date Tue, 14 Apr 2020 19:56:12 GMT, which is in 2 minutes

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".

… UI (#5353) Renderer: Add support for backoff and auto-disable on failed retry This commit introduces a backoff (150ms * number of tries) to the renderer's retry logic (introduced in #2830). It also changes the FAIL_FAST to a less globally-harmful render thread disable, so that we stop blowing up any application hosting a terminal when the graphics driver goes away. In addition, it adds a callback that a Renderer consumer can use to determine when the renderer _has_ failed, and a public method to kick it back into life. Fixes #5340. This PR also wires up TermControl so that it shows some UI when the renderer tastes clay. ![image](https://user-images.githubusercontent.com/14316954/79266118-f073f680-7e4b-11ea-8b96-5588a13aff3b.png) ![image](https://user-images.githubusercontent.com/14316954/79266125-f36ee700-7e4b-11ea-9314-4280e9149461.png) * [x] Closes #5340 * [x] cla * [ ] Tests added/passed * [ ] Requires documentation to be updated * [x] I've discussed this with core contributors already. I tested this by dropping the number of retries to 1 and forcing a TDR while doing `wsl cmatrix -u0`. It picked up exactly where it left off. As a bonus, you can actually still type into the terminal when it's graphically suspended (and `exit` still works.). The block is _entirely graphical_.

Improve wide glyph support in UIA (GH-4946) Add enhanced key support for ConPty (GH-5021) Set DxRenderer non-text alias mode (GH-5149) Reduce CursorChanged Events for Accessibility (GH-5196) Add more object ID tracing for Accessibility (GH-5215) Add SS3 cursor key encoding to ConPty (GH-5383) UIA: Prevent crash from invalid UTR endpoint comparison (GH-5399) Make CodepointWidthDetector::GetWidth faster (CC-3727) add til::math, use it for float conversions to point, size (GH-5150) Add support for renderer backoff, don't FAIL_FAST on 3x failures, add UI (GH-5353) Fix a deadlock and a bounding rects issue in UIA (GH-5385) Don't duplicate spaces from potentially-wrapped EOL-deferred lines (GH-5398) Reimplement the VT tab stop functionality (CC-5173) Clamp parameter values to a maximum of 32767. (CC-5200) Prevent the cursor type being reset when changing the visibility (CC-5251) Make RIS switch back to the main buffer (CC-5248) Add support for the DSR-OS operating status report (CC-5300) Update the virtual bottom location if the cursor moves below it (CC-5317) ci: run spell check in CI, fix remaining issues (CC-4799) (CC-5352) Set Cascadia Code as default font (GH-5121) Show a double width cursor for double width characters (GH-5319) Delegate all character input to the character event handler (CC-4192) Update til::bitmap to use dynamic_bitset<> + libpopcnt (GH-5092) Merged PR 4465022: [Git2Git] Merged PR 4464559: Console: Ingest OSS changes up to e055079 Correct scrolling invalidation region for tmux in pty w/ bitmap (GH-5122) Render row-by-row instead of invalidating entire screen (GH-5185) Make conechokey use ReadConsoleInputW by default (GH-5148) Manually pass mouse wheel messages to TermControls (GH-5131) This fixes C-M-space for WSL but not for Win32, but I'm not sure there's a problem in Win32 quite yet. (GH-5208) Fix copying wrapped lines by implementing better scrolling (GH-5181) Emit lines wrapped due to spaces at the end correctly (GH-5294) Remove unneeded whitespace (CC-5162)

ghost · 2020-04-22T17:11:57Z

🎉Windows Terminal Preview v0.11.1121.0 has been released which incorporates this pull request.:tada:

Handy links:

DHowett added 2 commits April 14, 2020 12:29

Wire TermControl's Render Failed handler up to show some auxiliary UI

57ead2d

DHowett-MSFT requested review from miniksa and zadjii-msft April 14, 2020 19:33

ghost approved these changes Apr 14, 2020

View reviewed changes

zadjii-msft reviewed Apr 14, 2020

View reviewed changes

ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 14, 2020

zadjii-msft added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. labels Apr 14, 2020

zadjii-msft approved these changes Apr 14, 2020

View reviewed changes

miniksa approved these changes Apr 14, 2020

View reviewed changes

DHowett added 2 commits April 14, 2020 12:52

Constantize

eb934e4

Commentize

621e595

ghost merged commit e61968c into master Apr 14, 2020

ghost deleted the dev/duhowett/eyes_off_the_prize branch April 14, 2020 20:11

ghost mentioned this pull request Apr 22, 2020

Crash in render engine due to directx device hanging up #5340

Closed

DHowett-MSFT mentioned this pull request May 12, 2020

When the renderer requires too many retries, try gracefully surviving #3985

Closed

This pull request was closed.

Conversation

DHowett-MSFT commented Apr 14, 2020

Summary of the Pull Request

PR Checklist

Validation Steps Performed

Uh oh!

zadjii-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zadjii-msft Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

DHowett-MSFT Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

zadjii-msft Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zadjii-msft commented Apr 14, 2020

Uh oh!

ghost commented Apr 14, 2020

Uh oh!

miniksa commented Apr 14, 2020

Uh oh!

zadjii-msft left a comment

Choose a reason for hiding this comment

Uh oh!

miniksa left a comment

Choose a reason for hiding this comment

Uh oh!

DHowett-MSFT commented Apr 14, 2020

Uh oh!

ghost commented Apr 14, 2020

Uh oh!

ghost commented Apr 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants