Skip to content

Commit 406f10b

Browse files
SeeminglyScienceTylerLeonhardt
authored andcommitted
Fix performance issue with ReadKey changes (#621)
* Fix performance issue with ReadKey changes - Separated the ReadKey changes to be platform specific. This way we don't need a busy loop to wait until a key is available on Windows. - Changed the Unix implementation of ReadKey to wait for a longer period of time between Console.KeyAvailable checks if the user has not pressed a key within the last five seconds. This change was made to ensure the CPU is able to enter low power mode. * Addressing feedback - Added final new lines to new files - Added VSCode workspace setting to automatically add final new lines - Changed unix readkey long delay to 300ms from 400ms - Added a short wait timer to unix readkey short delay to reduce CPU consumption
1 parent 5eb586e commit 406f10b

File tree

6 files changed

+166
-24
lines changed

6 files changed

+166
-24
lines changed

.vscode/settings.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@
22
{
33
"editor.tabSize": 4,
44
"editor.insertSpaces": true,
5-
"files.trimTrailingWhitespace": true
6-
}
5+
"files.trimTrailingWhitespace": true,
6+
"files.insertFinalNewline": true
7+
}

src/PowerShellEditorServices/Console/ConsoleReadLine.cs

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
using System.Collections.ObjectModel;
77
using System.Linq;
88
using System.Text;
9+
using System.Runtime.InteropServices;
910
using System.Threading;
1011
using System.Threading.Tasks;
11-
using UnixConsoleEcho;
1212

1313
namespace Microsoft.PowerShell.EditorServices.Console
1414
{
@@ -20,12 +20,28 @@ namespace Microsoft.PowerShell.EditorServices.Console
2020
internal class ConsoleReadLine
2121
{
2222
#region Private Field
23+
private static IConsoleOperations s_consoleProxy;
2324

2425
private PowerShellContext powerShellContext;
2526

2627
#endregion
2728

2829
#region Constructors
30+
static ConsoleReadLine()
31+
{
32+
// Maybe we should just include the RuntimeInformation package for FullCLR?
33+
#if CoreCLR
34+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
35+
{
36+
s_consoleProxy = new WindowsConsoleOperations();
37+
return;
38+
}
39+
40+
s_consoleProxy = new UnixConsoleOperations();
41+
#else
42+
s_consoleProxy = new WindowsConsoleOperations();
43+
#endif
44+
}
2945

3046
public ConsoleReadLine(PowerShellContext powerShellContext)
3147
{
@@ -130,24 +146,7 @@ public async Task<SecureString> ReadSecureLine(CancellationToken cancellationTok
130146

131147
private static async Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken cancellationToken)
132148
{
133-
await WaitForKeyAvailableAsync(cancellationToken);
134-
return Console.ReadKey(true);
135-
}
136-
137-
private static async Task WaitForKeyAvailableAsync(CancellationToken cancellationToken)
138-
{
139-
InputEcho.Disable();
140-
try
141-
{
142-
while (!Console.KeyAvailable)
143-
{
144-
await Task.Delay(50, cancellationToken);
145-
}
146-
}
147-
finally
148-
{
149-
InputEcho.Enable();
150-
}
149+
return await s_consoleProxy.ReadKeyAsync(cancellationToken);
151150
}
152151

153152
private async Task<string> ReadLine(bool isCommandLine, CancellationToken cancellationToken)
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
using System;
2+
using System.Threading;
3+
using System.Threading.Tasks;
4+
5+
namespace Microsoft.PowerShell.EditorServices.Console
6+
{
7+
/// <summary>
8+
/// Provides platform specific console utilities.
9+
/// </summary>
10+
public interface IConsoleOperations
11+
{
12+
/// <summary>
13+
/// Obtains the next character or function key pressed by the user asynchronously.
14+
/// Does not block when other console API's are called.
15+
/// </summary>
16+
/// <param name="cancellationToken">The CancellationToken to observe.</param>
17+
/// <returns>
18+
/// A task that will complete with a result of the key pressed by the user.
19+
/// </returns>
20+
Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken cancellationToken);
21+
}
22+
}
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
using System;
2+
using System.Threading;
3+
using System.Threading.Tasks;
4+
using UnixConsoleEcho;
5+
6+
namespace Microsoft.PowerShell.EditorServices.Console
7+
{
8+
internal class UnixConsoleOperations : IConsoleOperations
9+
{
10+
private const int LONG_READ_DELAY = 300;
11+
12+
private const int SHORT_READ_TIMEOUT = 5000;
13+
14+
private static readonly ManualResetEventSlim _waitHandle = new ManualResetEventSlim();
15+
16+
private SemaphoreSlim _readKeyHandle = new SemaphoreSlim(1, 1);
17+
18+
internal UnixConsoleOperations()
19+
{
20+
// Switch between long and short wait periods depending on if the
21+
// user has recently (last 5 seconds) pressed a key to avoid preventing
22+
// the CPU from entering low power mode.
23+
WaitForKeyAvailable = LongWaitForKey;
24+
}
25+
26+
public async Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken cancellationToken)
27+
{
28+
await _readKeyHandle.WaitAsync(cancellationToken);
29+
30+
// I tried to replace this library with a call to `stty -echo`, but unfortunately
31+
// the library also sets up allowing backspace to trigger `Console.KeyAvailable`.
32+
InputEcho.Disable();
33+
try
34+
{
35+
while (!await WaitForKeyAvailable(cancellationToken));
36+
}
37+
finally
38+
{
39+
InputEcho.Enable();
40+
_readKeyHandle.Release();
41+
}
42+
43+
return System.Console.ReadKey(intercept: true);
44+
}
45+
46+
private Func<CancellationToken, Task<bool>> WaitForKeyAvailable;
47+
48+
private async Task<bool> LongWaitForKey(CancellationToken cancellationToken)
49+
{
50+
while (!System.Console.KeyAvailable)
51+
{
52+
await Task.Delay(LONG_READ_DELAY, cancellationToken);
53+
}
54+
55+
WaitForKeyAvailable = ShortWaitForKey;
56+
return true;
57+
}
58+
59+
private async Task<bool> ShortWaitForKey(CancellationToken cancellationToken)
60+
{
61+
if (await SpinUntilKeyAvailable(SHORT_READ_TIMEOUT, cancellationToken))
62+
{
63+
cancellationToken.ThrowIfCancellationRequested();
64+
return true;
65+
}
66+
67+
cancellationToken.ThrowIfCancellationRequested();
68+
WaitForKeyAvailable = LongWaitForKey;
69+
return false;
70+
}
71+
72+
private async Task<bool> SpinUntilKeyAvailable(int millisecondsTimeout, CancellationToken cancellationToken)
73+
{
74+
return await Task<bool>.Factory.StartNew(
75+
() => SpinWait.SpinUntil(
76+
() =>
77+
{
78+
// The wait handle is never set, it's just used to enable cancelling the wait.
79+
_waitHandle.Wait(30, cancellationToken);
80+
return System.Console.KeyAvailable || cancellationToken.IsCancellationRequested;
81+
},
82+
millisecondsTimeout));
83+
}
84+
}
85+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
using System;
2+
using System.Threading;
3+
using System.Threading.Tasks;
4+
5+
namespace Microsoft.PowerShell.EditorServices.Console
6+
{
7+
internal class WindowsConsoleOperations : IConsoleOperations
8+
{
9+
private ConsoleKeyInfo? _bufferedKey;
10+
11+
private SemaphoreSlim _readKeyHandle = new SemaphoreSlim(1, 1);
12+
13+
public async Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken cancellationToken)
14+
{
15+
await _readKeyHandle.WaitAsync(cancellationToken);
16+
try
17+
{
18+
return
19+
_bufferedKey.HasValue
20+
? _bufferedKey.Value
21+
: await Task.Factory.StartNew(
22+
() => (_bufferedKey = System.Console.ReadKey(intercept: true)).Value);
23+
}
24+
finally
25+
{
26+
_readKeyHandle.Release();
27+
28+
// Throw if we're cancelled so the buffered key isn't cleared.
29+
cancellationToken.ThrowIfCancellationRequested();
30+
_bufferedKey = null;
31+
}
32+
}
33+
}
34+
}

src/PowerShellEditorServices/Session/Host/EditorServicesPSHostUserInterface.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -704,10 +704,11 @@ private async Task StartReplLoop(CancellationToken cancellationToken)
704704
OutputType.Normal,
705705
foregroundColor: ConsoleColor.Red);
706706
}
707+
// Do nothing here, the while loop condition will exit.
707708
catch (TaskCanceledException)
708-
{
709-
// Do nothing here, the while loop condition will exit.
710-
}
709+
{ }
710+
catch (OperationCanceledException)
711+
{ }
711712
catch (Exception e) // Narrow this if possible
712713
{
713714
this.WriteOutput(

0 commit comments

Comments
 (0)