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.

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

Remove sync I/O from JsonValueUtils async write methods #2707

Merged
merged 3 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
19 changes: 19 additions & 0 deletions src/Microsoft.OData.Core/Json/JsonValueUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,25 @@ private static void WriteSubstringToBuffer(string inputString, ref int currentIn
currentIndex += substrLength;
}

/// <summary>
/// Writes a substring starting at a specified position on the string to the buffer.
/// This method is intended for use in async methods, where you cannot pass ref parameters.
/// </summary>
/// <param name="inputString">Input string value.</param>
/// <param name="currentIndex">The index in the string at which the substring begins.</param>
/// <param name="buffer">Char buffer to use for streaming data.</param>
/// <param name="bufferIndex">Current position in the buffer after the substring has been written.</param>
/// <param name="substrLength">The length of the substring to be copied.</param>
private static void WriteSubstringToBuffer(string inputString, Ref<int> currentIndex, char[] buffer, Ref<int> bufferIndex, int substrLength)
{
Debug.Assert(inputString != null, "inputString != null");
Debug.Assert(buffer != null, "buffer != null");

inputString.CopyTo(currentIndex.Value, buffer, 0, substrLength);
Copy link
Member

Choose a reason for hiding this comment

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

Is it safe to copyto the buffer with less size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The amount to copy is determine by the substrLength. The caller of this method ensures that the substrLength is not greater than the buffer length: https://github.com/OData/odata.net/pull/2707/files#diff-c86855fdf1be1c1e0a820b8852ccc1fef825001c9ef74274af40778f7adfb96dR452.

Should we add an additional Debug.Assert in this method to check whether this precondition is violated?

Note that this method is a duplicate of the following existing method: https://github.com/OData/odata.net/pull/2707/files#diff-c86855fdf1be1c1e0a820b8852ccc1fef825001c9ef74274af40778f7adfb96dR780. The only change is that I replaced the ref parameters with Ref<int> so that we can use it in async methods.

bufferIndex.Value = substrLength;
currentIndex.Value += substrLength;
}

/// <summary>
/// Writes an escaped string to the buffer.
/// </summary>
Expand Down
163 changes: 149 additions & 14 deletions src/Microsoft.OData.Core/Json/JsonValueUtilsAsync.cs
Original file line number Diff line number Diff line change
Expand Up @@ -412,13 +412,13 @@ async Task WriteEscapedJsonStringValueInnerAsync(
{
innerBuffer.Value = BufferUtils.InitializeBufferIfRequired(innerBufferPool, innerBuffer.Value);
int bufferLength = innerBuffer.Value.Length;
int bufferIndex = 0;
int currentIndex = 0;
Ref<int> bufferIndex = new Ref<int>(0);
Ref<int> currentIndex = new Ref<int>(0);

// Let's copy and flush strings up to the first index of the special char
while (currentIndex < innerFirstIndex)
while (currentIndex.Value < innerFirstIndex)
{
int substrLength = innerFirstIndex - currentIndex;
int substrLength = innerFirstIndex - currentIndex.Value;

Debug.Assert(substrLength > 0, "SubStrLength should be greater than 0 always");

Expand All @@ -427,23 +427,24 @@ async Task WriteEscapedJsonStringValueInnerAsync(
// Otherwise copy to the buffer and go on from there.
if (substrLength >= bufferLength)
{
innerInputString.CopyTo(currentIndex, innerBuffer.Value, 0, bufferLength);
innerInputString.CopyTo(currentIndex.Value, innerBuffer.Value, 0, bufferLength);
await innerWriter.WriteAsync(innerBuffer.Value, 0, bufferLength).ConfigureAwait(false);
currentIndex += bufferLength;
currentIndex.Value += bufferLength;
}
else
{
WriteSubstringToBuffer(innerInputString, ref currentIndex, innerBuffer.Value, ref bufferIndex, substrLength);
WriteSubstringToBuffer(innerInputString, currentIndex, innerBuffer.Value, bufferIndex, substrLength);
}
}

// Write escaped string to buffer
WriteEscapedStringToBuffer(innerWriter, innerInputString, ref currentIndex, innerBuffer.Value, ref bufferIndex, innerStringEscapeOption);
await WriteEscapedStringToBufferAsync(innerWriter, innerInputString, currentIndex, innerBuffer.Value, bufferIndex, innerStringEscapeOption)
.ConfigureAwait(false);

// write any remaining chars to the writer
if (bufferIndex > 0)
if (bufferIndex.Value > 0)
{
await innerWriter.WriteAsync(innerBuffer.Value, 0, bufferIndex).ConfigureAwait(false);
await innerWriter.WriteAsync(innerBuffer.Value, 0, bufferIndex.Value).ConfigureAwait(false);
}
}
}
Expand All @@ -468,15 +469,18 @@ internal static async Task WriteEscapedCharArrayAsync(
Ref<char[]> buffer,
ICharArrayPool bufferPool)
{
int bufferIndex = 0;
Ref<int> bufferIndex = new Ref<int>(0);
Ref<int> inputArrayOffsetRef = new Ref<int>(inputArrayOffset);
buffer.Value = BufferUtils.InitializeBufferIfRequired(bufferPool, buffer.Value);

WriteEscapedCharArrayToBuffer(writer, inputArray, ref inputArrayOffset, inputArrayCount, buffer.Value, ref bufferIndex, stringEscapeOption);
await WriteEscapedCharArrayToBufferAsync(writer, inputArray, inputArrayOffsetRef, inputArrayCount, buffer.Value, bufferIndex, stringEscapeOption)
.ConfigureAwait(false);


// write remaining bytes in buffer
if (bufferIndex > 0)
if (bufferIndex.Value > 0)
{
await writer.WriteAsync(buffer.Value, 0, bufferIndex).ConfigureAwait(false);
await writer.WriteAsync(buffer.Value, 0, bufferIndex.Value).ConfigureAwait(false);
}
}

Expand All @@ -490,5 +494,136 @@ private static Task WriteQuotedAsync(this TextWriter writer, string text)
return writer.WriteAsync(
string.Concat(JsonConstants.QuoteCharacter, text, JsonConstants.QuoteCharacter));
}

/// <summary>
/// Writes an escaped string to the buffer.
/// </summary>
/// <param name="writer">The text writer to write the output.</param>

habbes marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="inputString">Input string value.</param>
/// <param name="currentIndex">The index in the string at which copying should begin.</param>
/// <param name="buffer">Char buffer to use for streaming data.</param>
/// <param name="bufferIndex">Current position in the buffer after the string has been written.</param>
/// <param name="stringEscapeOption">The string escape option.</param>
/// <remarks>
/// IMPORTANT: After all characters have been written,
/// caller is responsible for writing the final buffer contents to the writer.
habbes marked this conversation as resolved.
Show resolved Hide resolved
/// </remarks>
private static async Task WriteEscapedStringToBufferAsync(
TextWriter writer,
string inputString,
Ref<int> currentIndex,
char[] buffer,
Ref<int> bufferIndex,
ODataStringEscapeOption stringEscapeOption)
{
Debug.Assert(inputString != null, "inputString != null");
Debug.Assert(buffer != null, "buffer != null");

for (; currentIndex.Value < inputString.Length; currentIndex.Value++)
{
bufferIndex.Value = await EscapeAndWriteCharToBufferAsync(
writer,
inputString[currentIndex.Value],
buffer,
bufferIndex.Value,
stringEscapeOption
)
.ConfigureAwait(false);
}
}

/// <summary>
/// Escapes and writes a character buffer, flushing to the writer as the buffer fills.
/// </summary>
/// <param name="writer">The text writer to write the output to.</param>
/// <param name="character">The character to write to the buffer.</param>
/// <param name="buffer">Char buffer to use for streaming data.</param>
/// <param name="bufferIndex">The index into the buffer in which to write the character.</param>
/// <param name="stringEscapeOption">The string escape option.</param>
/// <returns>Current position in the buffer after the character has been written.</returns>
/// <remarks>
/// IMPORTANT: After all characters have been written,
/// caller is responsible for writing the final buffer contents to the writer.
/// </remarks>
private static async Task<int> EscapeAndWriteCharToBufferAsync(TextWriter writer, char character, char[] buffer, int bufferIndex, ODataStringEscapeOption stringEscapeOption)
{
int bufferLength = buffer.Length;
string escapedString = null;

if (stringEscapeOption == ODataStringEscapeOption.EscapeNonAscii || character <= 0x7F)
{
escapedString = JsonValueUtils.SpecialCharToEscapedStringMap[character];
habbes marked this conversation as resolved.
Show resolved Hide resolved
}

// Append the unhandled characters (that do not require special treatment)
// to the buffer.
if (escapedString == null)
{
buffer[bufferIndex] = character;
bufferIndex++;
}
else
{
// Okay, an unhandled character was detected.
// First lets check if we can fit it in the existing buffer, if not,
// flush the current buffer and reset. Add the escaped string to the buffer
habbes marked this conversation as resolved.
Show resolved Hide resolved
// and continue.
int escapedStringLength = escapedString.Length;
Debug.Assert(escapedStringLength <= bufferLength, "Buffer should be larger than the escaped string");

if ((bufferIndex + escapedStringLength) > bufferLength)
{
await writer.WriteAsync(buffer, 0, bufferIndex).ConfigureAwait(false);
bufferIndex = 0;
}

escapedString.CopyTo(0, buffer, bufferIndex, escapedStringLength);
bufferIndex += escapedStringLength;
}

if (bufferIndex >= bufferLength)
{
Debug.Assert(bufferIndex == bufferLength,
"We should never encounter a situation where the buffer index is greater than the buffer length");
await writer.WriteAsync(buffer, 0, bufferIndex).ConfigureAwait(false);
bufferIndex = 0;
}

return bufferIndex;
}

/// <summary>
/// Writes an escaped char array to the buffer.
/// </summary>
/// <param name="writer">The text writer to write the output to.</param>
/// <param name="inputArray">Character array to write.</param>
/// <param name="inputArrayOffset">How many characters to skip in the input array.</param>
/// <param name="inputArrayCount">How many characters to write from the input array.</param>
/// <param name="buffer">Char buffer to use for streaming data.</param>
/// <param name="bufferIndex">Current position in the buffer after the string has been written.</param>
/// <param name="stringEscapeOption">The string escape option.</param>
/// <remarks>
/// IMPORTANT: After all characters have been written,
/// caller is responsible for writing the final buffer contents to the writer.
/// </remarks>
private static async Task WriteEscapedCharArrayToBufferAsync(
TextWriter writer,
char[] inputArray,
Ref<int> inputArrayOffset,
int inputArrayCount,
char[] buffer,
Ref<int> bufferIndex,
ODataStringEscapeOption stringEscapeOption)
{
Debug.Assert(inputArray != null, "inputArray != null");
Debug.Assert(buffer != null, "buffer != null");

for (; inputArrayOffset.Value < inputArrayCount; inputArrayOffset.Value++)
{
bufferIndex.Value = await EscapeAndWriteCharToBufferAsync(writer, inputArray[inputArrayOffset.Value], buffer, bufferIndex.Value, stringEscapeOption)
.ConfigureAwait(false);
}
}
}
}
12 changes: 12 additions & 0 deletions src/Microsoft.OData.Core/Json/NonIndentedTextWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ public override Task WriteAsync(char value)
return this.writer.WriteAsync(value);
}

/// <inheritdoc/>
public override void Write(char[] buffer, int index, int count)
{
this.writer.Write(buffer, index, count);
}

/// <inheritdoc/>
public override Task WriteAsync(char[] buffer, int index, int count)
{
return this.writer.WriteAsync(buffer, index, count);
}

/// <summary>
/// Writes a new line.
/// </summary>
Expand Down
Loading