-
Notifications
You must be signed in to change notification settings - Fork 53
Do some extra work to avoid making a full size copy of the buffer when base64 encoding #1372
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
Draft
lewing
wants to merge
4
commits into
dotnet:main
Choose a base branch
from
lewing:overkill
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
178 changes: 154 additions & 24 deletions
178
src/Microsoft.DotNet.XHarness.TestRunners.Xunit/WasmXmlResultWriter.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,154 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System; | ||
using System.IO; | ||
using System.Xml.Linq; | ||
|
||
#nullable enable | ||
|
||
namespace Microsoft.DotNet.XHarness.TestRunners.Xunit; | ||
|
||
internal class WasmXmlResultWriter | ||
{ | ||
public static void WriteOnSingleLine(XElement assembliesElement) | ||
{ | ||
using var ms = new MemoryStream(); | ||
assembliesElement.Save(ms); | ||
ms.TryGetBuffer(out var bytes); | ||
var base64 = Convert.ToBase64String(bytes, Base64FormattingOptions.None); | ||
Console.WriteLine($"STARTRESULTXML {bytes.Count} {base64} ENDRESULTXML"); | ||
Console.WriteLine($"Finished writing {bytes.Count} bytes of RESULTXML"); | ||
} | ||
} | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System; | ||
using System.IO; | ||
using System.Runtime.InteropServices; | ||
using System.Xml.Linq; | ||
using System.Text; | ||
using System.Security.Cryptography; | ||
|
||
#nullable enable | ||
|
||
namespace Microsoft.DotNet.XHarness.TestRunners.Xunit; | ||
|
||
internal class WasmXmlResultWriter | ||
{ | ||
#if !NET || DEBUG | ||
public static void WriteOnSingleLine(XElement assembliesElement) | ||
{ | ||
using var ms = new MemoryStream(); | ||
assembliesElement.Save(ms); | ||
ms.TryGetBuffer(out var bytes); | ||
var base64 = Convert.ToBase64String(bytes, Base64FormattingOptions.None); | ||
Console.WriteLine($"STARTRESULTXML {bytes.Count} {base64} ENDRESULTXML"); | ||
Console.WriteLine($"Finished writing {bytes.Count} bytes of RESULTXML"); | ||
} | ||
#else | ||
private class ToBase64CharTransform : ICryptoTransform | ||
{ | ||
private readonly ToBase64Transform _base64Transform = new ToBase64Transform(); | ||
|
||
public int InputBlockSize => _base64Transform.InputBlockSize; // 3 bytes of input | ||
public int OutputBlockSize => _base64Transform.OutputBlockSize * 2; // 4 bytes of base64 output * 2 for UTF-16 encoding | ||
|
||
public bool CanTransformMultipleBlocks => _base64Transform.CanTransformMultipleBlocks; | ||
public bool CanReuseTransform => _base64Transform.CanReuseTransform; | ||
|
||
public void Dispose() | ||
{ | ||
_base64Transform.Dispose(); | ||
} | ||
|
||
public int TransformBlock( | ||
byte[] inputBuffer, int inputOffset, int inputCount, | ||
byte[] outputBuffer, int outputOffset) | ||
{ | ||
int totalBytesWritten = 0; | ||
int inputProcessed = 0; | ||
|
||
while (inputProcessed < inputCount) | ||
{ | ||
int bytesToProcess = Math.Min(InputBlockSize, inputCount - inputProcessed); | ||
|
||
/* | ||
Input Buffer ("hi mom"): | ||
+-----+-----+-----+-----+-----+-----+ | ||
| 'h' | 'i' | ' ' | 'm' | 'o' | 'm' | | ||
+-----+-----+-----+-----+-----+-----+ | ||
|104 |105 | 32 |109 |111 |109 | | ||
+-----+-----+-----+-----+-----+-----+ | ||
|
||
Base64 Encoding Process: | ||
- 'hi ' -> 'aGkg' | ||
- 'mom' -> 'bW9t' | ||
|
||
Base64 Encoded Output: | ||
| |base64Written | | base64Written | | ||
+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+ | ||
| \0 | \0 | \0 | \0 |'a' |'G' |'k' |'g' | \0 | \0 | \0 | \0 |'b' |'W' |'9' |'t' | | ||
+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+ | ||
| 0 | 0 | 0 | 0 | 97 | 71 |107 |103 | 0 | 0 | 0 | 0 | 98 | 87 | 57 |116 | | ||
+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+ | ||
|
||
Expanded Output Buffer (UTF-16 Encoding): | ||
| outputChars | outputChars | | ||
+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+ | ||
| \0 |'a' | \0 |'G' | \0 |'k' | \0 |'g' | \0 |'b' | \0 |'W' | \0 |'9' | \0 |'t' | | ||
+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+ | ||
| 0 | 97 | 0 | 71 | 0 |107 | 0 |103 | 0 | 98 | 0 | 87 | 0 | 57 | 0 |116 | | ||
+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+----+ | ||
|
||
*/ | ||
|
||
// Calculate positions in the output buffer | ||
int outputStart = outputOffset + totalBytesWritten; | ||
int base64OutputStart = outputStart + OutputBlockSize / 2; | ||
|
||
// write Base64 transformation directly to the second half of the output buffer | ||
int base64BytesWritten = _base64Transform.TransformBlock( | ||
inputBuffer, inputOffset + inputProcessed, bytesToProcess, | ||
outputBuffer, base64OutputStart); | ||
|
||
var base64Written = outputBuffer.AsSpan(base64OutputStart, base64BytesWritten); | ||
var outputChars = outputBuffer.AsSpan(outputStart, OutputBlockSize); | ||
for (int i = 0; i < base64BytesWritten; i++) | ||
{ | ||
// Expand each ascii byte to a char write it in the same logical position | ||
// as a char in outputChars eventually filling the output buffer | ||
BitConverter.TryWriteBytes(outputChars.Slice(i * 2), (char)base64Written[i]); | ||
} | ||
|
||
inputProcessed += bytesToProcess; | ||
totalBytesWritten += base64BytesWritten * 2; | ||
} | ||
|
||
return totalBytesWritten; | ||
} | ||
|
||
public byte[] TransformFinalBlock(byte[] inputBuffer, int inputOffset, int inputCount) | ||
{ | ||
// Apply Base64 transformation to the final block | ||
byte[] base64Buffer = _base64Transform.TransformFinalBlock(inputBuffer, inputOffset, inputCount); | ||
|
||
// Expand each Base64 byte to two bytes in the output buffer | ||
byte[] outputBuffer = new byte[base64Buffer.Length * 2]; | ||
for (int i = 0; i < base64Buffer.Length; i++) | ||
{ | ||
// Convert each ascii byte to a char | ||
BitConverter.TryWriteBytes(outputBuffer.AsSpan(i * 2), (char)base64Buffer[i]); | ||
} | ||
|
||
return outputBuffer; | ||
} | ||
} | ||
|
||
public static void WriteOnSingleLine(XElement assembliesElement) | ||
{ | ||
using var ms = new MemoryStream(); | ||
using var transform = new ToBase64CharTransform(); | ||
using var cryptoStream = new CryptoStream(ms, transform, CryptoStreamMode.Write); | ||
|
||
// Create a StreamWriter to write the XML content to the CryptoStream | ||
using var xmlWriter = new StreamWriter(cryptoStream, Encoding.UTF8); | ||
|
||
assembliesElement.Save(xmlWriter); | ||
|
||
// Ensure all data is flushed through the CryptoStream | ||
xmlWriter.Flush(); | ||
cryptoStream.FlushFinalBlock(); | ||
|
||
// guaranteed to succeed with the MemoryStream() constructor | ||
ms.TryGetBuffer(out var bytes); | ||
// we went to a lot of trouble to put characters in the final buffer | ||
// so that we can avoid a copy here and pass the span directly to the | ||
// string interpolation logic. | ||
Span<char> charData = MemoryMarshal.Cast<byte,char>(bytes); | ||
|
||
// Output the result | ||
lewing marked this conversation as resolved.
Show resolved
Hide resolved
lewing marked this conversation as resolved.
Show resolved
Hide resolved
lewing marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Console.WriteLine($"STARTRESULTXML {charData.Length} {charData} ENDRESULTXML"); | ||
Console.WriteLine($"Finished writing {charData.Length} bytes of RESULTXML"); | ||
lewing marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
lewing marked this conversation as resolved.
Show resolved
Hide resolved
lewing marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#endif | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that base64 could contain
\n
in the middle right ? If so, you can insert some which would make the buffer in emscripten smaller. Becausestdout
emulator would build buffer until newline and only then write it into browserconsole.log
.@maraf noted that maybe we had some issues with message order WRT
WASM EXIT
... so I don't know if multi-line is good idea or not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could in theory write each block as we transform it rather than create a single buffer. We should really review the emscripten line buffering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, this could be
fetch('POST', xml)
in browser and nodejs.Remaining is
v8
shell which doesn't have file write. So we may need to keep this contraption.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really think we should implement it as POST. Do we run V8 in CI at all ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we noticed it in dotnet/runtime#102069 (comment). it'd be nice to fix that though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#1388