Skip to content

Commit

Permalink
Avoid unnecessary string allocations in IdnMapping (dotnet#17399)
Browse files Browse the repository at this point in the history 
If the output matches the input string, we can just use the input string as the result.
  • Loading branch information
@stephentoub
stephentoub authored Apr 4, 2018
1 parent 872095a commit fff9f71
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 21 deletions.
21 changes: 12 additions & 9 deletions src/mscorlib/shared/System/Globalization/IdnMapping.Unix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ namespace System.Globalization
{
sealed partial class IdnMapping
{
private unsafe string GetAsciiCore(char* unicode, int count)
private unsafe string GetAsciiCore(string unicodeString, char* unicode, int count)
{
Debug.Assert(!GlobalizationMode.Invariant);
Debug.Assert(unicodeString != null && unicodeString.Length >= count);

uint flags = Flags;
CheckInvalidIdnCharacters(unicode, count, flags, nameof(unicode));
Expand All @@ -26,7 +27,7 @@ private unsafe string GetAsciiCore(char* unicode, int count)
actualLength = Interop.Globalization.ToAscii(flags, unicode, count, outputStack, estimatedLength);
if (actualLength > 0 && actualLength <= estimatedLength)
{
return new string(outputStack, 0, actualLength);
return GetStringForOutput(unicodeString, unicode, count, outputStack, actualLength);
}
}
else
Expand All @@ -46,13 +47,14 @@ private unsafe string GetAsciiCore(char* unicode, int count)
{
throw new ArgumentException(SR.Argument_IdnIllegalName, nameof(unicode));
}
return new string(pOutputHeap, 0, actualLength);
return GetStringForOutput(unicodeString, unicode, count, pOutputHeap, actualLength);
}
}

private unsafe string GetUnicodeCore(char* ascii, int count)
private unsafe string GetUnicodeCore(string asciiString, char* ascii, int count)
{
Debug.Assert(!GlobalizationMode.Invariant);
Debug.Assert(asciiString != null && asciiString.Length >= count);

uint flags = Flags;
CheckInvalidIdnCharacters(ascii, count, flags, nameof(ascii));
Expand All @@ -61,21 +63,22 @@ private unsafe string GetUnicodeCore(char* ascii, int count)
if (count < StackAllocThreshold)
{
char* output = stackalloc char[count];
return GetUnicodeCore(ascii, count, flags, output, count, reattempt: true);
return GetUnicodeCore(asciiString, ascii, count, flags, output, count, reattempt: true);
}
else
{
char[] output = new char[count];
fixed (char* pOutput = &output[0])
{
return GetUnicodeCore(ascii, count, flags, pOutput, count, reattempt: true);
return GetUnicodeCore(asciiString, ascii, count, flags, pOutput, count, reattempt: true);
}
}
}

private unsafe string GetUnicodeCore(char* ascii, int count, uint flags, char* output, int outputLength, bool reattempt)
private unsafe string GetUnicodeCore(string asciiString, char* ascii, int count, uint flags, char* output, int outputLength, bool reattempt)
{
Debug.Assert(!GlobalizationMode.Invariant);
Debug.Assert(asciiString != null && asciiString.Length >= count);

int realLen = Interop.Globalization.ToUnicode(flags, ascii, count, output, outputLength);

Expand All @@ -85,14 +88,14 @@ private unsafe string GetUnicodeCore(char* ascii, int count, uint flags, char* o
}
else if (realLen <= outputLength)
{
return new string(output, 0, realLen);
return GetStringForOutput(asciiString, ascii, count, output, realLen);
}
else if (reattempt)
{
char[] newOutput = new char[realLen];
fixed (char* pNewOutput = newOutput)
{
return GetUnicodeCore(ascii, count, flags, pNewOutput, realLen, reattempt: false);
return GetUnicodeCore(asciiString, ascii, count, flags, pNewOutput, realLen, reattempt: false);
}
}

Expand Down
24 changes: 14 additions & 10 deletions src/mscorlib/shared/System/Globalization/IdnMapping.Windows.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ namespace System.Globalization
{
public sealed partial class IdnMapping
{
private unsafe string GetAsciiCore(char* unicode, int count)
private unsafe string GetAsciiCore(string unicodeString, char* unicode, int count)
{
Debug.Assert(!GlobalizationMode.Invariant);
Debug.Assert(unicodeString != null && unicodeString.Length >= count);

uint flags = Flags;

Expand All @@ -27,34 +28,36 @@ private unsafe string GetAsciiCore(char* unicode, int count)
if (length < StackAllocThreshold)
{
char* output = stackalloc char[length];
return GetAsciiCore(unicode, count, flags, output, length);
return GetAsciiCore(unicodeString, unicode, count, flags, output, length);
}
else
{
char[] output = new char[length];
fixed (char* pOutput = &output[0])
{
return GetAsciiCore(unicode, count, flags, pOutput, length);
return GetAsciiCore(unicodeString, unicode, count, flags, pOutput, length);
}
}
}

private unsafe string GetAsciiCore(char* unicode, int count, uint flags, char* output, int outputLength)
private unsafe string GetAsciiCore(string unicodeString, char* unicode, int count, uint flags, char* output, int outputLength)
{
Debug.Assert(!GlobalizationMode.Invariant);
Debug.Assert(unicodeString != null && unicodeString.Length >= count);

int length = Interop.Normaliz.IdnToAscii(flags, unicode, count, output, outputLength);
if (length == 0)
{
ThrowForZeroLength(unicode: true);
}
Debug.Assert(length == outputLength);
return new string(output, 0, length);
return GetStringForOutput(unicodeString, unicode, count, output, length);
}

private unsafe string GetUnicodeCore(char* ascii, int count)
private unsafe string GetUnicodeCore(string asciiString, char* ascii, int count)
{
Debug.Assert(!GlobalizationMode.Invariant);
Debug.Assert(asciiString != null && asciiString.Length >= count);

uint flags = Flags;

Expand All @@ -70,29 +73,30 @@ private unsafe string GetUnicodeCore(char* ascii, int count)
if (length < StackAllocThreshold)
{
char* output = stackalloc char[length];
return GetUnicodeCore(ascii, count, flags, output, length);
return GetUnicodeCore(asciiString, ascii, count, flags, output, length);
}
else
{
char[] output = new char[length];
fixed (char* pOutput = &output[0])
{
return GetUnicodeCore(ascii, count, flags, pOutput, length);
return GetUnicodeCore(asciiString, ascii, count, flags, pOutput, length);
}
}
}

private unsafe string GetUnicodeCore(char* ascii, int count, uint flags, char* output, int outputLength)
private unsafe string GetUnicodeCore(string asciiString, char* ascii, int count, uint flags, char* output, int outputLength)
{
Debug.Assert(!GlobalizationMode.Invariant);
Debug.Assert(asciiString != null && asciiString.Length >= count);

int length = Interop.Normaliz.IdnToUnicode(flags, ascii, count, output, outputLength);
if (length == 0)
{
ThrowForZeroLength(unicode: false);
}
Debug.Assert(length == outputLength);
return new string(output, 0, length);
return GetStringForOutput(asciiString, ascii, count, output, length);
}

// -----------------------------
Expand Down
13 changes: 11 additions & 2 deletions src/mscorlib/shared/System/Globalization/IdnMapping.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
// RFC 3492 - Punycode: A Bootstring encoding of Unicode for Internationalized Domain Names in Applications (IDNA)

using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Text;

namespace System.Globalization
Expand Down Expand Up @@ -93,7 +94,7 @@ public string GetAscii(string unicode, int index, int count)
{
fixed (char* pUnicode = unicode)
{
return GetAsciiCore(pUnicode + index, count);
return GetAsciiCore(unicode, pUnicode + index, count);
}
}
}
Expand Down Expand Up @@ -137,7 +138,7 @@ public string GetUnicode(string ascii, int index, int count)
{
fixed (char* pAscii = ascii)
{
return GetUnicodeCore(pAscii + index, count);
return GetUnicodeCore(ascii, pAscii + index, count);
}
}
}
Expand All @@ -156,6 +157,14 @@ public override int GetHashCode()
return (_allowUnassigned ? 100 : 200) + (_useStd3AsciiRules ? 1000 : 2000);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static unsafe string GetStringForOutput(string originalString, char* input, int inputLength, char* output, int outputLength)
{
return originalString.Length == inputLength && new ReadOnlySpan<char>(input, inputLength).SequenceEqual(new ReadOnlySpan<char>(output, outputLength)) ?
originalString :
new string(output, 0, outputLength);
}

//
// Invariant implementation
//
Expand Down

0 comments on commit fff9f71

Please sign in to comment.