Skip to content

Optimize console output in ConsoleHost #6882

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

Merged
daxian-dbw merged 18 commits into from
Apr 17, 2019
Merged

Optimize console output in ConsoleHost #6882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
0043f30
[Feature] Exclude extra allocations in ConsoleControl
iSazonov May 15, 2018
fdd4e8d
Fix param section
iSazonov Jun 6, 2018
c8e106c
Minor style corrections
iSazonov Jun 6, 2018
ded4dd7
Fix doc comments
iSazonov Jan 10, 2019
a6be5b1
Address feedback
iSazonov Jan 10, 2019
9250356
Style fix 1
iSazonov Jan 10, 2019
afea4df
Style fix 2
iSazonov Jan 10, 2019
0354f72
Style fix 3
iSazonov Jan 10, 2019
b66f03e
Address feedback
iSazonov Apr 3, 2019
42823c6
Add AggressiveInlining
iSazonov Apr 3, 2019
273b1b7
Fix broken rebase
iSazonov Apr 4, 2019
5b7bb77
Put new line only after last position
iSazonov Apr 4, 2019
e57001d
Fix init
iSazonov Apr 8, 2019
d16d779
Update ConsoleTextWriter.cs
iSazonov Apr 8, 2019
c97670d
Reorder methods
iSazonov Apr 8, 2019
232e303
Fix Serialize
iSazonov Apr 9, 2019
59b6cd0
Address my own comments
daxian-dbw Apr 15, 2019
f6a72af
Rename 'WriteHelper' to 'WriteImpl'; refactor 'WriteLine' and 'WriteL…
daxian-dbw Apr 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 75 additions & 30 deletions src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleControl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2542,61 +2542,91 @@ internal static void SetConsoleWindowTitle(string consoleTitle)
/// Wrap Win32 WriteConsole.
/// </summary>
/// <param name="consoleHandle">
/// handle for the console where the string is written
/// Handle for the console where the string is written.
/// </param>
/// <param name="output">
/// string that is written
/// String that is written.
/// </param>
/// <param name="newLine">
/// New line is written.
/// </param>
/// <exception cref="HostException">
/// if the Win32's WriteConsole fails
/// If the Win32's WriteConsole fails.
/// </exception>

internal static void WriteConsole(ConsoleHandle consoleHandle, string output)
internal static void WriteConsole(ConsoleHandle consoleHandle, ReadOnlySpan<char> output, bool newLine)
{
Dbg.Assert(!consoleHandle.IsInvalid, "ConsoleHandle is not valid");
Dbg.Assert(!consoleHandle.IsClosed, "ConsoleHandle is closed");

if (string.IsNullOrEmpty(output))
if (output.Length == 0)
{
if (newLine)
{
WriteConsole(consoleHandle, ConsoleHostUserInterface.Crlf);
}

return;
}

// Native WriteConsole doesn't support output buffer longer than 64K.
// We need to chop the output string if it is too long.

int cursor = 0; // This records the chopping position in output string
const int maxBufferSize = 16383; // this is 64K/4 - 1 to account for possible width of each character.
const int MaxBufferSize = 16383; // this is 64K/4 - 1 to account for possible width of each character.

while (cursor < output.Length)
{
string outBuffer;
ReadOnlySpan<char> outBuffer;

if (cursor + maxBufferSize < output.Length)
if (cursor + MaxBufferSize < output.Length)
{
outBuffer = output.Substring(cursor, maxBufferSize);
cursor += maxBufferSize;
outBuffer = output.Slice(cursor, MaxBufferSize);
cursor += MaxBufferSize;

WriteConsole(consoleHandle, outBuffer);
}
else
{
outBuffer = output.Substring(cursor);
outBuffer = output.Slice(cursor);
cursor = output.Length;

if (newLine)
{
var endOfLine = ConsoleHostUserInterface.Crlf.AsSpan();
var endOfLineLength = endOfLine.Length;
Span<char> outBufferLine = stackalloc char[outBuffer.Length + endOfLineLength];
outBuffer.CopyTo(outBufferLine);
endOfLine.CopyTo(outBufferLine.Slice(outBufferLine.Length - endOfLineLength));
WriteConsole(consoleHandle, outBufferLine);
}
else
{
WriteConsole(consoleHandle, outBuffer);
}
}
}
}

DWORD charsWritten;
bool result =
NativeMethods.WriteConsole(
consoleHandle.DangerousGetHandle(),
outBuffer,
(DWORD)outBuffer.Length,
out charsWritten,
IntPtr.Zero);
private static void WriteConsole(ConsoleHandle consoleHandle, ReadOnlySpan<char> buffer)
{
DWORD charsWritten;
bool result =
NativeMethods.WriteConsole(
consoleHandle.DangerousGetHandle(),
buffer,
(DWORD)buffer.Length,
out charsWritten,
IntPtr.Zero);

if (result == false)
{
int err = Marshal.GetLastWin32Error();
if (result == false)
{
int err = Marshal.GetLastWin32Error();

HostException e = CreateHostException(err, "WriteConsole",
ErrorCategory.WriteError, ConsoleControlStrings.WriteConsoleExceptionTemplate);
throw e;
}
HostException e = CreateHostException(
err,
"WriteConsole",
ErrorCategory.WriteError,
ConsoleControlStrings.WriteConsoleExceptionTemplate);
throw e;
}
}

Expand Down Expand Up @@ -3009,15 +3039,30 @@ out DWORD numberOfCharsWritten

[DllImport(PinvokeDllNames.WriteConsoleDllName, SetLastError = true, CharSet = CharSet.Unicode)]
[return: MarshalAs(UnmanagedType.Bool)]
internal static extern bool WriteConsole
private static extern unsafe bool WriteConsole
(
NakedWin32Handle consoleOutput,
string buffer,
char* buffer,
DWORD numberOfCharsToWrite,
out DWORD numberOfCharsWritten,
IntPtr reserved
);

internal static unsafe bool WriteConsole
(
NakedWin32Handle consoleOutput,
ReadOnlySpan<char> buffer,
DWORD numberOfCharsToWrite,
out DWORD numberOfCharsWritten,
IntPtr reserved
)
{
fixed (char* bufferPtr = &MemoryMarshal.GetReference(buffer))
{
return WriteConsole(consoleOutput, bufferPtr, numberOfCharsToWrite, out numberOfCharsWritten, reserved);
}
Copy link
Member

@daxian-dbw daxian-dbw Jun 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This obviously complexes the code. Actually, all changes here make code more complex. Do we have measurable perf improvement to justify the changes? I don't see a large amount of string manipulation here (please point it out if I miss them). Note that many string APIs and StringBuilder already received perf improvements in 2.1.

Copy link
Collaborator Author

@iSazonov iSazonov Jun 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like complement - I'm not sure I can create a complex code 😄

Exactly the pattern I found in CoreFX. This is due to C# language limitations.

Really the code isn't getting any more complicated. Rather on the contrary it becomes more complete.
By analyzing the code to clean up psl I found that in this place we make a lot of kernel calls and extra locks because of the multitude of:

WriteToConsole(text, true);
WriteToConsole(Crlf, true);

I was very surprised that we don't have a WriteLine() method to write to a host and use many allocations like value + Crlf - so I just implemented the method.
The only complicity of the code is that I had to modify three levels of interfaces. So really the PR is very simple.

Note that many string APIs and StringBuilder already received perf improvements in 2.1.

Yes, I saw up to 1100000 allocations in VS Profiler at pwsh.exe 6.0 startup and up to 850000 allocation at 6.1 (already on .Net 2.1) startup. Nevertheless, I still see a lot of allocations. The profile show that up to 36% allocations is chars and strings.
This shows that CoreCLR does not solve all allocation problems and we should continue to consume resources cautiously and to optimize our code.

Copy link
Member

@daxian-dbw daxian-dbw Jun 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced whether the startup is the right scenario to measure for changes in this PR, but still, for the startup scenario, what's the improvement in object allocation with your changes?

Copy link
Collaborator Author

@iSazonov iSazonov Jun 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it was only sample that 2.1 reduce allocations but not remove its at all. I don't know how measure wins in the PR - VS Profiler is crashed after first run. I look https://benchmarkdotnet.org/ but it takes a lot of effort. At least in the first steps.

Copy link
Collaborator Author

@iSazonov iSazonov Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I attached a file with simple perf tests. I saw win ~2 second on 100000 outputs.
measure_conhost.txt

Copy link
Collaborator

@BrucePay BrucePay Jul 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need any of this code anymore? It was written back in V1 when the Console API was very limited. I suspect that we could remove most of the code with just a thin wrapper around the .NET APIs. (Which, in fact, were somewhat inspired by this code.)

Copy link
Collaborator Author

@iSazonov iSazonov Jul 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this code would have to be cleaned more thoroughly. The problem is that we do not have tests for the console code. Therefore, I would prefer to make small changes step by step so as not to break something. I intend to continue working in this direction (if only you do not chop it in one your PR).

}

[DllImport(PinvokeDllNames.GetConsoleTitleDllName, SetLastError = true, CharSet = CharSet.Unicode)]
internal static extern DWORD GetConsoleTitle(StringBuilder consoleTitle, DWORD size);

Expand Down
4 changes: 1 addition & 3 deletions src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHostRawUserInterface.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,9 +705,7 @@ public override

if ((options & ReadKeyOptions.NoEcho) == 0)
{
parent.WriteToConsole(
keyInfo.Character.ToString(),
true);
parent.WriteToConsole(keyInfo.Character, transcribeResult: true);
}

return keyInfo;
Expand Down
37 changes: 22 additions & 15 deletions src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHostTranscript.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,9 @@

namespace Microsoft.PowerShell
{
internal sealed partial
class ConsoleHost
:
PSHost,
IDisposable
internal sealed partial class ConsoleHost : PSHost, IDisposable
{
internal
bool
IsTranscribing
internal bool IsTranscribing
{
get
{
Expand Down Expand Up @@ -69,9 +63,7 @@ internal void StartTranscribing(string transcriptFilename, bool shouldAppend)
*/
private string _transcriptFileName = string.Empty;

internal
string
StopTranscribing()
internal string StopTranscribing()
{
lock (_transcriptionStateLock)
{
Expand Down Expand Up @@ -106,15 +98,30 @@ internal void StartTranscribing(string transcriptFilename, bool shouldAppend)
}
}

internal
void
WriteToTranscript(string text)
internal void WriteToTranscript(ReadOnlySpan<char> text)
{
WriteToTranscript(text, newLine: false);
}

internal void WriteLineToTranscript(ReadOnlySpan<char> text)
{
WriteToTranscript(text, newLine: true);
}

internal void WriteToTranscript(ReadOnlySpan<char> text, bool newLine)
{
lock (_transcriptionStateLock)
{
if (_isTranscribing && _transcriptionWriter != null)
{
_transcriptionWriter.Write(text);
if (newLine)
{
_transcriptionWriter.WriteLine(text);
}
else
{
_transcriptionWriter.Write(text);
}
}
}
}
Expand Down
Loading