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

Remove unnecessary string and string[] allocations from MS.Internal.ContentType #6268

Merged
merged 3 commits into from May 5, 2022
Merged

Remove unnecessary string and string[] allocations from MS.Internal.ContentType #6268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6442932
Remove Split from ContentType.ParseTypeAndSubType
stephentoub Mar 17, 2022
31dd8de
Remove Substrings from ContentType
stephentoub Mar 17, 2022
67c0a5d
Merge branch 'main' into contenttypeparsing
dipeshmsft May 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
97 changes: 34 additions & 63 deletions src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/ContentType.cs
View file
Open in desktop
Expand Up @@ -110,7 +110,7 @@ internal ContentType(string contentType)
else
{
// Parse content type similar to - type/subtype ; param1=value1 ; param2=value2 ; param3="value3"
ParseTypeAndSubType(contentType.Substring(0, semiColonIndex));
ParseTypeAndSubType(contentType.AsSpan(0, semiColonIndex));
ParseParameterAndValue(contentType.Substring(semiColonIndex));
}
}
Expand Down Expand Up @@ -282,7 +282,7 @@ public override string ToString()
|| String.CompareOrdinal(_subType, String.Empty) != 0);

StringBuilder stringBuilder = new StringBuilder(_type);
stringBuilder.Append(_forwardSlashSeparator[0]);
stringBuilder.Append('/');
stringBuilder.Append(_subType);

if (_parameterDictionary != null && _parameterDictionary.Count > 0)
Expand Down Expand Up @@ -420,18 +420,20 @@ private static void ValidateCarriageReturns(string contentType)
/// </summary>
/// <param name="typeAndSubType">substring that has the type and subType of the content type</param>
/// <exception cref="ArgumentException">If the typeAndSubType parameter does not have the "/" character</exception>
private void ParseTypeAndSubType(string typeAndSubType)
private void ParseTypeAndSubType(ReadOnlySpan<char> typeAndSubType)
{
//okay to trim at this point the end of the string as Linear White Spaces(LWS) chars are allowed here.
typeAndSubType = typeAndSubType.TrimEnd(_LinearWhiteSpaceChars);

string[] splitBasedOnForwardSlash = typeAndSubType.Split(_forwardSlashSeparator);

if (splitBasedOnForwardSlash.Length != 2)
int forwardSlashPos = typeAndSubType.IndexOf('/');
if (forwardSlashPos < 0 || // no slashes
typeAndSubType.Slice(forwardSlashPos + 1).IndexOf('/') >= 0) // more than one slash
{
throw new ArgumentException(SR.InvalidTypeSubType);
}

_type = ValidateToken(splitBasedOnForwardSlash[0]);
_subType = ValidateToken(splitBasedOnForwardSlash[1]);
_type = ValidateToken(typeAndSubType.Slice(0, forwardSlashPos).ToString());
_subType = ValidateToken(typeAndSubType.Slice(forwardSlashPos + 1).ToString());
}

/// <summary>
Expand All @@ -440,9 +442,9 @@ private void ParseTypeAndSubType(string typeAndSubType)
/// <param name="parameterAndValue">This string has the parameter and value pair of the form
/// parameter=value</param>
/// <exception cref="ArgumentException">If the string does not have the required "="</exception>
private void ParseParameterAndValue(string parameterAndValue)
private void ParseParameterAndValue(ReadOnlySpan<char> parameterAndValue)
{
while (String.CompareOrdinal(parameterAndValue, String.Empty) != 0)
while (!parameterAndValue.IsEmpty)
{
//At this point the first character MUST be a semi-colon
//First time through this test is serving more as an assert.
Expand All @@ -456,7 +458,7 @@ private void ParseParameterAndValue(string parameterAndValue)
throw new ArgumentException(SR.ExpectingParameterValuePairs);

//Removing the leading ; from the string
parameterAndValue = parameterAndValue.Substring(1);
parameterAndValue = parameterAndValue.Slice(1);

//okay to trim start as there can be spaces before the begining
//of the parameter name.
Expand All @@ -475,10 +477,10 @@ private void ParseParameterAndValue(string parameterAndValue)
EnsureParameterDictionary();

_parameterDictionary.Add(
ValidateToken(parameterAndValue.Substring(0, equalSignIndex)),
ValidateQuotedStringOrToken(parameterAndValue.Substring(parameterStartIndex, parameterValueLength)));
ValidateToken(parameterAndValue.Slice(0, equalSignIndex).ToString()),
ValidateQuotedStringOrToken(parameterAndValue.Slice(parameterStartIndex, parameterValueLength).ToString()));

parameterAndValue = parameterAndValue.Substring(parameterStartIndex + parameterValueLength).TrimStart(_LinearWhiteSpaceChars);
parameterAndValue = parameterAndValue.Slice(parameterStartIndex + parameterValueLength).TrimStart(_LinearWhiteSpaceChars);
}
}

Expand All @@ -488,34 +490,31 @@ private void ParseParameterAndValue(string parameterAndValue)
/// <param name="s"></param>
/// <param name="startIndex">Starting index for parsing</param>
/// <returns></returns>
private static int GetLengthOfParameterValue(string s, int startIndex)
private static int GetLengthOfParameterValue(ReadOnlySpan<char> s, int startIndex)
{
Debug.Assert(s != null);

int length = 0;
int length;

//if the parameter value does not start with a '"' then,
//we expect a valid token. So we look for Linear White Spaces or
//a ';' as the terminator for the token value.
if (s[startIndex] != '"')
{
int semicolonIndex = s.IndexOf(_semicolonSeparator, startIndex);
int semicolonIndex = s.Slice(startIndex).IndexOf(_semicolonSeparator);

if (semicolonIndex != -1)
{
int lwsIndex = s.IndexOfAny(_LinearWhiteSpaceChars, startIndex);
if (lwsIndex != -1 && lwsIndex < semicolonIndex)
length = lwsIndex;
else
length = semicolonIndex;
int lwsIndex = s.Slice(startIndex).IndexOfAny(_LinearWhiteSpaceChars);
length = lwsIndex != -1 && lwsIndex < semicolonIndex ? lwsIndex : semicolonIndex;
length += startIndex; // the indexes from IndexOf{Any} are based on slicing from startIndex
}
else
length = semicolonIndex;

//If there is no linear white space found we treat the entire remaining string as
//parameter value.
if (length == -1)
{
//If there is no linear white space found we treat the entire remaining string as
//parameter value.
length = s.Length;
}
}
else
{
Expand All @@ -526,10 +525,12 @@ private static int GetLengthOfParameterValue(string s, int startIndex)

while (!found)
{
length = s.IndexOf('"', ++length);
int startingLength = ++length;
length = s.Slice(startingLength).IndexOf('"');
Comment on lines +528 to +529
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
int startingLength = ++length;
length = s.Slice(startingLength).IndexOf('"');
int startingLength = length + 1;
length = s.Slice(startingLength).IndexOf('"') + 1;

Copy link
Member Author

Choose a reason for hiding this comment

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

That won't work correctly. We can change the ++length to be length + 1, but not the second line.


if (length == -1)
throw new ArgumentException(SR.InvalidParameterValue);
length += startingLength; // IndexOf result is based on slicing from startingLength

if (s[length - 1] != '\\')
{
Expand Down Expand Up @@ -584,7 +585,7 @@ private static string ValidateQuotedStringOrToken(string parameterValue)
if (parameterValue.Length >= 2 &&
parameterValue.StartsWith(_quote, StringComparison.Ordinal) &&
parameterValue.EndsWith(_quote, StringComparison.Ordinal))
ValidateQuotedText(parameterValue.Substring(1, parameterValue.Length-2));
ValidateQuotedText(parameterValue.AsSpan(1, parameterValue.Length-2));
else
ValidateToken(parameterValue);

Expand All @@ -595,7 +596,7 @@ private static string ValidateQuotedStringOrToken(string parameterValue)
/// This method validates if the text in the quoted string
/// </summary>
/// <param name="quotedText"></param>
private static void ValidateQuotedText(string quotedText)
private static void ValidateQuotedText(ReadOnlySpan<char> quotedText)
{
//empty is okay

Expand Down Expand Up @@ -637,37 +638,9 @@ private static bool IsAllowedCharacter(char character)
/// </summary>
/// <param name="character">input character</param>
/// <returns></returns>
private static bool IsAsciiLetterOrDigit(char character)
{
if (IsAsciiLetter(character))
{
return true;
}
if (character >= '0')
{
return (character <= '9');
}
return false;
}

/// <summary>
/// Returns true if the input character is an ASCII letter
/// Returns false if the input character is not an ASCII letter
/// </summary>
/// <param name="character">input character</param>
/// <returns></returns>
private static bool IsAsciiLetter(char character)
{
if ((character >= 'a') && (character <= 'z'))
{
return true;
}
if (character >= 'A')
{
return (character <= 'Z');
}
return false;
}
private static bool IsAsciiLetterOrDigit(char character) =>
((((uint)character - 'A') & ~0x20) < 26) ||
(((uint)character - '0') < 10);

/// <summary>
/// Returns true if the input character is one of the Linear White Space characters -
Expand Down Expand Up @@ -731,8 +704,6 @@ private void EnsureParameterDictionary()
'.' /*46*/, '^' /*94*/ , '_' /*95*/,
'`' /*96*/, '|' /*124*/, '~' /*126*/,
};

private static readonly char[] _forwardSlashSeparator = { '/' };

//Linear White Space characters
private static readonly char[] _LinearWhiteSpaceChars =
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.DotNet.Wpf/src/WindowsBase/WindowsBase.csproj
View file
Open in desktop
Expand Up @@ -397,6 +397,7 @@
<NetCoreReference Include="System.Linq" />
<NetCoreReference Include="System.ObjectModel" />
<NetCoreReference Include="System.Resources.ResourceManager" />
<NetCoreReference Include="System.Memory" />
<NetCoreReference Include="System.Runtime" />
<NetCoreReference Include="System.Runtime.Extensions" />
<NetCoreReference Include="System.Runtime.InteropServices" />
Expand Down