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

Jump to bottom

Optimize progress reporting by reusing StringBuilder #4360

Merged
merged 1 commit into from
May 1, 2023
Merged

Optimize progress reporting by reusing StringBuilder #4360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
120 changes: 4 additions & 116 deletions src/NUnitFramework/framework/Interfaces/TNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Runtime.CompilerServices;
using System.Text;
using System.Xml;

namespace NUnit.Framework.Interfaces
Expand All @@ -23,11 +21,6 @@ namespace NUnit.Framework.Interfaces
[DebuggerDisplay("{OuterXml}")]
public sealed class TNode
{
private static readonly XmlWriterSettings _xmlWriterSettings = new()
{
ConformanceLevel = ConformanceLevel.Fragment
};

internal List<TNode>? _childNodes;
internal Dictionary<string, string>? _attributes;

Expand Down Expand Up @@ -58,7 +51,7 @@ public TNode(string name)
public TNode(string name, string? value, bool valueIsCDATA)
: this(name)
{
Value = EscapeInvalidXmlCharacters(value);
Value = XmlExtensions.EscapeInvalidXmlCharacters(value);
ValueIsCDATA = valueIsCDATA;
}

Expand Down Expand Up @@ -104,7 +97,7 @@ public string OuterXml
get
{
using var stringWriter = new StringWriter();
using (var xmlWriter = XmlWriter.Create(stringWriter, _xmlWriterSettings))
using (var xmlWriter = XmlWriter.Create(stringWriter, XmlExtensions.FragmentWriterSettings))
{
WriteTo(xmlWriter);
}
Expand Down Expand Up @@ -267,7 +260,7 @@ public void InsertChildNode(int index, TNode node)
public void AddAttribute(string name, string value)
{
_attributes ??= new Dictionary<string, string>();
_attributes.Add(name, EscapeInvalidXmlCharacters(value));
_attributes.Add(name, XmlExtensions.EscapeInvalidXmlCharacters(value));
}

/// <summary>
Expand Down Expand Up @@ -312,7 +305,7 @@ public void WriteTo(XmlWriter writer)

if (Value != null)
if (ValueIsCDATA)
WriteCDataTo(writer);
writer.WriteCDataSafe(Value);
else
writer.WriteString(Value);

Expand Down Expand Up @@ -385,111 +378,6 @@ private static List<TNode> ApplySelection(List<TNode> nodeList, string xpath)
: resultNodes;
}

[return: NotNullIfNotNull("str")]
private static string? EscapeInvalidXmlCharacters(string? str)
{
if (str == null) return null;

// quick check when we expect valid input
foreach (var c in str)
{
if (c < 0x20 || c > 0x7F)
{
return EscapeInvalidXmlCharactersUnlikely(str);
}
}

return str;
}

private static string EscapeInvalidXmlCharactersUnlikely(string str)
{
StringBuilder? builder = null;
for (int i = 0; i < str.Length; i++)
{
char c = str[i];
if(c > 0x20 && c < 0x7F)
{
// ASCII characters - break quickly for these
builder?.Append(c);
}
// From the XML specification: https://www.w3.org/TR/xml/#charsets
// Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]
// Any Unicode character, excluding the surrogate blocks, FFFE, and FFFF.
else if (!(0x0 <= c && c <= 0x8) &&
c != 0xB &&
c != 0xC &&
!(0xE <= c && c <= 0x1F) &&
!(0x7F <= c && c <= 0x84) &&
!(0x86 <= c && c <= 0x9F) &&
!(0xD800 <= c && c <= 0xDFFF) &&
c != 0xFFFE &&
c != 0xFFFF)
{
builder?.Append(c);
}
// Also check if the char is actually a high/low surrogate pair of two characters.
// If it is, then it is a valid XML character (from above based on the surrogate blocks).
else if (char.IsHighSurrogate(c) &&
i + 1 != str.Length &&
char.IsLowSurrogate(str[i + 1]))
{
if (builder != null)
{
builder.Append(c);
builder.Append(str[i + 1]);
}
i++;
}
else
{
// We keep the builder null so that we don't allocate a string
// when doing this conversion until we encounter a unicode character.
// Then, we allocate the rest of the string and escape the invalid
// character.
if (builder == null)
{
builder = new StringBuilder();
for (int index = 0; index < i; index++)
builder.Append(str[index]);
}
builder.Append(CharToUnicodeSequence(c));
}
}

if (builder != null)
return builder.ToString();
else
return str;
}

private static string CharToUnicodeSequence(char symbol)
{
return $"\\u{(int)symbol:x4}";
}

private void WriteCDataTo(XmlWriter writer)
{
int start = 0;
string text = Value ?? throw new InvalidOperationException();

while (true)
{
int illegal = text.IndexOf("]]>", start, StringComparison.Ordinal);
if (illegal < 0)
break;
writer.WriteCData(text.Substring(start, illegal - start + 2));
start = illegal + 2;
if (start >= text.Length)
return;
}

if (start > 0)
writer.WriteCData(text.Substring(start));
else
writer.WriteCData(text);
}

#endregion

#region Nested NodeFilter class
Expand Down
39 changes: 22 additions & 17 deletions src/NUnitFramework/framework/Interfaces/TestMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt

using System;
using System.Diagnostics;
using System.IO;
using System.Xml;

namespace NUnit.Framework.Interfaces
{
Expand All @@ -18,17 +19,10 @@ public sealed class TestMessage
/// <param name="destination">Destination of the message</param>
/// <param name="text">Text to be sent</param>
/// <param name="testId">ID of the test that produced the message</param>
public TestMessage(string destination, string text, string testId)
public TestMessage(string destination, string text, string? testId)
{
if (destination == null)
{
throw new ArgumentNullException(nameof(destination));
}

if (text == null)
{
throw new ArgumentNullException(nameof(text));
}
Guard.ArgumentNotNull(destination, nameof(destination));
Guard.ArgumentNotNull(text, nameof(text));

Destination = destination;
Message = text;
Expand Down Expand Up @@ -56,22 +50,33 @@ public override string ToString()
/// <summary>
/// The ID of the test that sent the message
/// </summary>
public string TestId { get; }
public string? TestId { get; }

/// <summary>
/// Returns the XML representation of the <see cref="TestMessage"/> object.
/// </summary>
public string ToXml()
{
TNode tnode = new TNode("test-message", Message, true);
using var stringWriter = new StringWriter();
using (var writer = XmlWriter.Create(stringWriter, XmlExtensions.FragmentWriterSettings))
{
ToXml(writer);
}
return stringWriter.ToString();
}

if (Destination != null)
tnode.AddAttribute("destination", Destination);
internal void ToXml(XmlWriter writer)
{
writer.WriteStartElement("test-message");

writer.WriteAttributeString("destination", Destination);

if (TestId != null)
tnode.AddAttribute("testid", TestId);
writer.WriteAttributeString("testid", TestId);

writer.WriteCDataSafe(Message);

return tnode.OuterXml;
writer.WriteEndElement();
}
}
}
32 changes: 25 additions & 7 deletions src/NUnitFramework/framework/Interfaces/TestOutput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt

using System.IO;
using System.Xml;

namespace NUnit.Framework.Interfaces
{
/// <summary>
/// The TestOutput class holds a unit of output from
/// a test to a specific output stream
/// </summary>
public class TestOutput
{
public class TestOutput
{
/// <summary>
/// Construct with text, output destination type and
/// the name of the test that produced the output.
Expand All @@ -18,6 +21,9 @@ public class TestOutput
/// <param name="testName">FullName of test that produced the output</param>
public TestOutput(string text, string stream, string? testId, string? testName)
{
Guard.ArgumentNotNull(text, nameof(text));
Guard.ArgumentNotNull(stream, nameof(stream));

Text = text;
Stream = stream;
TestId = testId;
Expand Down Expand Up @@ -58,16 +64,28 @@ public override string ToString()
/// </summary>
public string ToXml()
{
TNode tnode = new TNode("test-output", Text, true);
using var stringWriter = new StringWriter();
using (var writer = XmlWriter.Create(stringWriter, XmlExtensions.FragmentWriterSettings))
{
ToXml(writer);
}
return stringWriter.ToString();
}

internal void ToXml(XmlWriter writer)
{
writer.WriteStartElement("test-output");
writer.WriteAttributeString("stream", Stream);

tnode.AddAttribute("stream", Stream);
if (TestId != null)
tnode.AddAttribute("testid", TestId);
writer.WriteAttributeString("testid", TestId);

if (TestName != null)
tnode.AddAttribute("testname", TestName);
writer.WriteAttributeStringSafe("testname", TestName);

writer.WriteCDataSafe(Text);

return tnode.OuterXml;
writer.WriteEndElement();
}
}
}
7 changes: 5 additions & 2 deletions src/NUnitFramework/framework/Internal/PropertyBag.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,16 @@ public TNode AddToXml(TNode parentNode, bool recursive)
// enumerating dictionary directly with struct enumerator which is fastest
foreach (var pair in inner)
{
foreach (var value in pair.Value)
// Use for-loop to avoid allocating the enumerator
var list = pair.Value;
var propertyCount = list.Count;
for (var i = 0; i < propertyCount; i++)
{
TNode prop = properties.AddElement("property");

// TODO: Format as string
prop.AddAttribute("name", pair.Key);
prop.AddAttribute("value", value.ToString()!);
prop.AddAttribute("value", list[i]!.ToString()!);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
prop.AddAttribute("value", list[i]!.ToString()!);
prop.AddAttribute("value", list[i].ToString()!);

list elements are not-null. Null suppression operator not needed.

}
}

Expand Down
Loading
Loading