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

Improve Console for iOS and Android #37078

Closed
wants to merge 7 commits into from
Closed

Improve Console for iOS and Android #37078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
87622d0
Improve Console for iOS and Android
EgorBo May 27, 2020
ad41075
Update src/libraries/System.Console/src/System/ConsolePal.Android.cs
EgorBo May 27, 2020
eb867ae
Update src/libraries/System.Console/src/System/ConsolePal.iOS.cs
EgorBo May 27, 2020
f4f6613
Address feedback
EgorBo May 27, 2020
835cdc0
Address feedback
EgorBo May 28, 2020
8eb4d08
Introduce AccumulateNewLines
EgorBo May 28, 2020
bd8b9f6
Fix compilation errors
EgorBo May 28, 2020
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
21 changes: 19 additions & 2 deletions src/libraries/System.Console/src/System/ConsolePal.Android.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,37 @@

using System.IO;
using System.Text;
using System.Diagnostics;
using System.Runtime.InteropServices;

namespace System
{
internal sealed unsafe class LogcatStream : ConsoleStream
{
private readonly StringBuilder _buffer = new StringBuilder();

public LogcatStream() : base(FileAccess.Write) {}

public override int Read(byte[] buffer, int offset, int count) => throw Error.GetReadNotSupported();

public override unsafe void Write(byte[] buffer, int offset, int count)
{
string log = ConsolePal.OutputEncoding.GetString(buffer, offset, count);
Interop.Logcat.AndroidLogPrint(Interop.Logcat.LogLevel.Info, "DOTNET", log);
ValidateWrite(buffer, offset, count);

ReadOnlySpan<byte> bufferSpan = new ReadOnlySpan<byte>(buffer, offset, count);

Debug.Assert(ConsolePal.OutputEncoding == Encoding.Unicode);
Debug.Assert(bufferSpan.Length % 2 == 0);

ReadOnlySpan<char> charSpan = MemoryMarshal.Cast<byte, char>(bufferSpan);
Copy link
Member

Choose a reason for hiding this comment

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

This won't necessarily be two-byte aligned. Is that ok?


lock (_buffer)
{
AccumulateNewLines(_buffer, charSpan, line =>
{
Interop.Logcat.AndroidLogPrint(Interop.Logcat.LogLevel.Info, "DOTNET", line);
});
}
}
}

Expand Down
23 changes: 20 additions & 3 deletions src/libraries/System.Console/src/System/ConsolePal.iOS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@

using System.IO;
using System.Text;
using System.Diagnostics;
using System.Runtime.InteropServices;

namespace System
{
internal sealed class NSLogStream : ConsoleStream
{
private readonly StringBuilder _buffer = new StringBuilder();

public NSLogStream() : base(FileAccess.Write) {}

public override int Read(byte[] buffer, int offset, int count) => throw Error.GetReadNotSupported();
Expand All @@ -17,9 +21,22 @@ public override unsafe void Write(byte[] buffer, int offset, int count)
{
ValidateWrite(buffer, offset, count);

fixed (byte* ptr = buffer)
ReadOnlySpan<byte> bufferSpan = new ReadOnlySpan<byte>(buffer, offset, count);

Debug.Assert(ConsolePal.OutputEncoding == Encoding.Unicode);
Debug.Assert(bufferSpan.Length % 2 == 0);

ReadOnlySpan<char> charSpan = MemoryMarshal.Cast<byte, char>(bufferSpan);

lock (_buffer)
{
Interop.Sys.Log(ptr + offset, count);
AccumulateNewLines(_buffer, charSpan, line =>
{
fixed (char* ptr = line)
{
Interop.Sys.Log((byte*)ptr, line.Length * 2);
}
});
}
}
}
Expand Down Expand Up @@ -170,4 +187,4 @@ internal sealed class ControlCHandlerRegistrar
internal void Unregister() => throw new PlatformNotSupportedException();
}
}
}
}
32 changes: 32 additions & 0 deletions src/libraries/System.Console/src/System/IO/ConsoleStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,37 @@ protected void ValidateWrite(byte[] buffer, int offset, int count)

if (!_canWrite) throw Error.GetWriteNotSupported();
}

protected static void AccumulateNewLines(StringBuilder accumulator, ReadOnlySpan<char> buffer, Action<string> printer)
{
int lineStartIndex = 0;
for (int i = 0; i < buffer.Length; i++)
Copy link
Member

Choose a reason for hiding this comment

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

This will always walk the entire buffer.

I think a more performant algorithm would be to walk the buffer backwards, and if you find a newline character, you can print everything up to that character (prepending anything in the buffer), and then put the rest in the buffer. This would be much faster for Console.WriteLine statements, since it would only check the last character in the buffer.

for (int i = buffer.Length - 1; i >= 0; i--) {
    if (buffer[i] == '\n') {
        if (accumulator.Length > 0) {
            printer (accumulator + buffer.Slice(0, i + 1));
            accumulator.Clear();
        } else {
            printer (buffer.Slice(0, i + 1));
        }
        if (i < buffer.Length - 1) {
            // there's text after the last newline, add it to the accumulator
            accumulator.Append(buffer.Slice(i + 1));
        }
        return;
    }
}
// no newlines found, add the entire buffer to the accumulator
accumulator.Append(buffer);

{
if (buffer[i] == '\n')
{
ReadOnlySpan<char> sliceWithNl = buffer.Slice(lineStartIndex, i - lineStartIndex);
if (accumulator.Length > 0)
{
// we found a new line, append content from accumulator if it's not empty
accumulator.Append(sliceWithNl);
printer(accumulator.ToString());
Copy link
Member

Choose a reason for hiding this comment

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

There are lots of strings created. Would it be possible to pass ReadOnlySpans around instead?

accumulator.Clear();
}
else
{
// accumulator is empty - print the line as it is
printer(sliceWithNl.ToString());
}
lineStartIndex = i + 1;
}
}

if (buffer.Length > 0 && buffer[^1] != '\n')
{
// add a line without '\n' to accumulator
ReadOnlySpan<char> appendix = buffer.Slice(lineStartIndex, buffer.Length - lineStartIndex);
accumulator.Append(appendix);
}
}
}
}
6 changes: 3 additions & 3 deletions src/mono/netcore/sample/iOS/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ all: runtimepack run

TOOLS_DIR=../../../../../tools-local/tasks/mobile.tasks
appbuilder:
$(DOTNET) build -c Release $(TOOLS_DIR)/AotCompilerTask/MonoAOTCompiler.csproj
$(DOTNET) build -c Release $(TOOLS_DIR)/AppleAppBuilder/AppleAppBuilder.csproj
$(DOTNET) build -c Debug $(TOOLS_DIR)/AotCompilerTask/MonoAOTCompiler.csproj
$(DOTNET) build -c Debug $(TOOLS_DIR)/AppleAppBuilder/AppleAppBuilder.csproj

runtimepack:
../../../../.././build.sh Mono+Libs -os iOS -arch $(MONO_ARCH) -c $(MONO_CONFIG)

run: clean appbuilder
$(DOTNET) publish -c $(MONO_CONFIG) /p:TargetArchitecture=$(MONO_ARCH) \
/p:UseLLVM=$(USE_LLVM) /p:UseAotForSimulator=true
/p:UseLLVM=$(USE_LLVM) /p:UseAotForSimulator=false /p:TargetOS=iOS

clean:
rm -rf bin
10 changes: 3 additions & 7 deletions src/mono/netcore/sample/iOS/Program.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
<OutputPath>bin</OutputPath>
<DebugType>Portable</DebugType>
<TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
<TargetOS>iOS</TargetOS>
<RuntimePackDir>$(ArtifactsBinDir)lib-runtime-packs\$(NetCoreAppCurrent)-iOS-$(Configuration)-$(TargetArchitecture)\runtimes\ios-$(TargetArchitecture)</RuntimePackDir>
<EnableTargetingPackDownload>false</EnableTargetingPackDownload>
<RuntimeIdentifier>ios-$(TargetArchitecture)</RuntimeIdentifier>
Expand All @@ -23,12 +22,9 @@
<Message Text="Packaged ID: %(RuntimePack.PackageDirectory)" Importance="high" />
</Target>

<Import Project="/Users/egorbo/prj/runtime/tools-local/tasks/mobile.tasks/AotCompilerTask/MonoAOTCompiler.props" />
<UsingTask TaskName="AppleAppBuilderTask"
AssemblyFile="$(ArtifactsBinDir)AppleAppBuilder\$(Configuration)\$(NetCoreAppCurrent)\AppleAppBuilder.dll" />

<UsingTask TaskName="MonoAOTCompiler"
AssemblyFile="$(ArtifactsBinDir)MonoAOTCompiler\$(Configuration)\$(NetCoreAppCurrent)\MonoAOTCompiler.dll" />
<Import Project="$(RepoRoot)tools-local\tasks\mobile.tasks\AotCompilerTask\MonoAOTCompiler.props" />
<UsingTask TaskName="AppleAppBuilderTask" AssemblyFile="$(AppleAppBuilderTasksAssemblyPath)" />
<UsingTask TaskName="MonoAOTCompiler" AssemblyFile="$(MonoAOTCompilerTasksAssemblyPath)" />

<Target Name="BuildAppBundle" AfterTargets="CopyFilesToPublishDirectory">
<PropertyGroup>
Expand Down