Skip to content

Commit 673c354

Browse files
authored
Reduce ThicknessConverter allocs to minimum and improve conversion performance (#9363)
* use DefaultInterpolatedStringHandler over StringBuilder and remove large chunk of allocs * simplify and remove alloc on FromString method as well * further optimize codegen based on manual review/benchmark * removal additional type casts, simplify code logic * remove unsafe code, move FormatDoubleAsString back to LengthConverter
1 parent cae5cef commit 673c354

File tree

2 files changed

+76
-68
lines changed

2 files changed

+76
-68
lines changed

src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/LengthConverter.cs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111

1212
using System;
1313
using System.ComponentModel;
14-
1514
using System.ComponentModel.Design.Serialization;
15+
using System.Runtime.CompilerServices;
1616
using System.Diagnostics;
1717
using System.Globalization;
1818
using System.Reflection;
@@ -162,7 +162,7 @@ public override object ConvertTo(ITypeDescriptorContext typeDescriptorContext,
162162
}
163163
throw GetConvertToException(value, destinationType);
164164
}
165-
#endregion
165+
#endregion
166166

167167
//-------------------------------------------------------------------
168168
//
@@ -172,6 +172,18 @@ public override object ConvertTo(ITypeDescriptorContext typeDescriptorContext,
172172

173173
#region Internal Methods
174174

175+
/// <summary> Format <see cref="double"/> into <see cref="string"/> using specified <see cref="CultureInfo"/>
176+
/// in <paramref name="handler"/>. <br /> <br />
177+
/// Special representation applies for <see cref="double.NaN"/> values, emitted as "Auto" string instead. </summary>
178+
/// <param name="value">The value to format as string.</param>
179+
/// <param name="handler">The handler specifying culture used for conversion.</param>
180+
static internal void FormatLengthAsString(double value, ref DefaultInterpolatedStringHandler handler)
181+
{
182+
if (double.IsNaN(value))
183+
handler.AppendLiteral("Auto");
184+
else
185+
handler.AppendFormatted(value);
186+
}
175187

176188
// Parse a Length from a string given the CultureInfo.
177189
// Formats:
@@ -221,11 +233,6 @@ private static double ParseDouble(ReadOnlySpan<char> span, CultureInfo cultureIn
221233
}
222234
}
223235

224-
static internal string ToString(double l, CultureInfo cultureInfo)
225-
{
226-
if(double.IsNaN(l)) return "Auto";
227-
return Convert.ToString(l, cultureInfo);
228-
}
229236

230237
#endregion
231238

src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ThicknessConverter.cs

Lines changed: 62 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@
55
//
66
//
77
//
8-
// Description: Contains the ThicknessConverter: TypeConverter for the Thicknessclass.
8+
// Description: Contains the ThicknessConverter: TypeConverter for the Thickness struct.
99
//
1010
//
1111

1212
using System;
1313
using System.ComponentModel;
1414
using System.ComponentModel.Design.Serialization;
15+
using System.Runtime.CompilerServices;
1516
using System.Globalization;
1617
using System.Reflection;
1718
using System.Text;
@@ -73,15 +74,7 @@ public override bool CanConvertFrom(ITypeDescriptorContext typeDescriptorContext
7374
public override bool CanConvertTo(ITypeDescriptorContext typeDescriptorContext, Type destinationType)
7475
{
7576
// We can convert to an InstanceDescriptor or to a string.
76-
if ( destinationType == typeof(InstanceDescriptor)
77-
|| destinationType == typeof(string))
78-
{
79-
return true;
80-
}
81-
else
82-
{
83-
return false;
84-
}
77+
return destinationType == typeof(InstanceDescriptor) || destinationType == typeof(string);
8578
}
8679

8780
/// <summary>
@@ -102,20 +95,22 @@ public override bool CanConvertTo(ITypeDescriptorContext typeDescriptorContext,
10295
/// <param name="source"> The object to convert to a Thickness. </param>
10396
public override object ConvertFrom(ITypeDescriptorContext typeDescriptorContext, CultureInfo cultureInfo, object source)
10497
{
105-
if (source != null)
106-
{
107-
if (source is string) { return FromString((string)source, cultureInfo); }
108-
else if (source is double) { return new Thickness((double)source); }
109-
else { return new Thickness(Convert.ToDouble(source, cultureInfo)); }
110-
}
111-
throw GetConvertFromException(source);
98+
if (source is null)
99+
throw GetConvertFromException(source);
100+
101+
if (source is string sourceString)
102+
return FromString(sourceString, cultureInfo);
103+
else if (source is double sourceValue)
104+
return new Thickness(sourceValue);
105+
else
106+
return new Thickness(Convert.ToDouble(source, cultureInfo));
112107
}
113108

114109
/// <summary>
115110
/// ConvertTo - Attempt to convert a Thickness to the given type
116111
/// </summary>
117112
/// <returns>
118-
/// The object which was constructoed.
113+
/// The object which was constructed.
119114
/// </returns>
120115
/// <exception cref="ArgumentNullException">
121116
/// An ArgumentNullException is thrown if the example object is null.
@@ -133,22 +128,18 @@ public override object ConvertTo(ITypeDescriptorContext typeDescriptorContext, C
133128
ArgumentNullException.ThrowIfNull(value);
134129
ArgumentNullException.ThrowIfNull(destinationType);
135130

136-
if (!(value is Thickness))
137-
{
138-
#pragma warning suppress 6506 // value is obviously not null
139-
throw new ArgumentException(SR.Format(SR.UnexpectedParameterType, value.GetType(), typeof(Thickness)), "value");
140-
}
131+
if (value is not Thickness thickness)
132+
throw new ArgumentException(SR.Format(SR.UnexpectedParameterType, value.GetType(), typeof(Thickness)), nameof(value));
141133

142-
Thickness th = (Thickness)value;
143-
if (destinationType == typeof(string)) { return ToString(th, cultureInfo); }
144-
if (destinationType == typeof(InstanceDescriptor))
134+
if (destinationType == typeof(string))
135+
return ToString(thickness, cultureInfo);
136+
else if (destinationType == typeof(InstanceDescriptor))
145137
{
146138
ConstructorInfo ci = typeof(Thickness).GetConstructor(new Type[] { typeof(double), typeof(double), typeof(double), typeof(double) });
147-
return new InstanceDescriptor(ci, new object[] { th.Left, th.Top, th.Right, th.Bottom });
139+
return new InstanceDescriptor(ci, new object[] { thickness.Left, thickness.Top, thickness.Right, thickness.Bottom });
148140
}
149141

150142
throw new ArgumentException(SR.Format(SR.CannotConvertType, typeof(Thickness), destinationType.FullName));
151-
152143
}
153144

154145

@@ -162,60 +153,70 @@ public override object ConvertTo(ITypeDescriptorContext typeDescriptorContext, C
162153

163154
#region Internal Methods
164155

165-
static internal string ToString(Thickness th, CultureInfo cultureInfo)
156+
/// <summary>
157+
/// Converts <paramref name="th"/> to its string representation using the specified <paramref name="cultureInfo"/>.
158+
/// </summary>
159+
/// <param name="th">The <see cref="Thickness"/> to convert to string.</param>
160+
/// <param name="cultureInfo">Culture to use when formatting doubles and choosing separator.</param>
161+
/// <returns>The formatted <paramref name="th"/> as string using the specified <paramref name="cultureInfo"/>.</returns>
162+
internal static string ToString(Thickness th, CultureInfo cultureInfo)
166163
{
167164
char listSeparator = TokenizerHelper.GetNumericListSeparator(cultureInfo);
168165

169166
// Initial capacity [64] is an estimate based on a sum of:
170167
// 48 = 4x double (twelve digits is generous for the range of values likely)
171168
// 8 = 4x Unit Type string (approx two characters)
172-
// 4 = 4x separator characters
173-
StringBuilder sb = new StringBuilder(64);
174-
175-
sb.Append(LengthConverter.ToString(th.Left, cultureInfo));
176-
sb.Append(listSeparator);
177-
sb.Append(LengthConverter.ToString(th.Top, cultureInfo));
178-
sb.Append(listSeparator);
179-
sb.Append(LengthConverter.ToString(th.Right, cultureInfo));
180-
sb.Append(listSeparator);
181-
sb.Append(LengthConverter.ToString(th.Bottom, cultureInfo));
182-
return sb.ToString();
169+
// 3 = 3x separator characters
170+
// 1 = 1x scratch space for alignment
171+
172+
DefaultInterpolatedStringHandler handler = new(0, 7, cultureInfo, stackalloc char[64]);
173+
LengthConverter.FormatLengthAsString(th.Left, ref handler);
174+
handler.AppendFormatted(listSeparator);
175+
176+
LengthConverter.FormatLengthAsString(th.Top, ref handler);
177+
handler.AppendFormatted(listSeparator);
178+
179+
LengthConverter.FormatLengthAsString(th.Right, ref handler);
180+
handler.AppendFormatted(listSeparator);
181+
182+
LengthConverter.FormatLengthAsString(th.Bottom, ref handler);
183+
184+
return handler.ToStringAndClear();
183185
}
184186

185-
static internal Thickness FromString(string s, CultureInfo cultureInfo)
187+
/// <summary>
188+
/// Constructs a <see cref="Thickness"/> struct out of string representation supplied by <paramref name="s"/> and the specified <paramref name="cultureInfo"/>.
189+
/// </summary>
190+
/// <param name="s">The string representation of a <see cref="Thickness"/> struct.</param>
191+
/// <param name="cultureInfo">The <see cref="CultureInfo"/> which was used to format this string.</param>
192+
/// <returns>A new instance of <see cref="Thickness"/> struct representing the data contained in <paramref name="s"/>.</returns>
193+
/// <exception cref="FormatException">Thrown when <paramref name="s"/> contains invalid string representation.</exception>
194+
internal static Thickness FromString(string s, CultureInfo cultureInfo)
186195
{
187-
TokenizerHelper th = new TokenizerHelper(s, cultureInfo);
188-
double[] lengths = new double[4];
196+
TokenizerHelper th = new(s, cultureInfo);
197+
Span<double> lengths = stackalloc double[4];
189198
int i = 0;
190199

191200
// Peel off each double in the delimited list.
192201
while (th.NextToken())
193202
{
194-
if (i >= 4)
195-
{
196-
i = 5; // Set i to a bad value.
197-
break;
198-
}
203+
if (i >= 4) // In case we've got more than 4 doubles, we throw
204+
throw new FormatException(SR.Format(SR.InvalidStringThickness, s));
199205

200206
lengths[i] = LengthConverter.FromString(th.GetCurrentToken(), cultureInfo);
201207
i++;
202208
}
203209

204-
// We have a reasonable interpreation for one value (all four edges), two values (horizontal, vertical),
210+
// We have a reasonable interpretation for one value (all four edges),
211+
// two values (horizontal, vertical),
205212
// and four values (left, top, right, bottom).
206-
switch (i)
213+
return i switch
207214
{
208-
case 1:
209-
return new Thickness(lengths[0]);
210-
211-
case 2:
212-
return new Thickness(lengths[0], lengths[1], lengths[0], lengths[1]);
213-
214-
case 4:
215-
return new Thickness(lengths[0], lengths[1], lengths[2], lengths[3]);
216-
}
217-
218-
throw new FormatException(SR.Format(SR.InvalidStringThickness, s));
215+
1 => new Thickness(lengths[0]),
216+
2 => new Thickness(lengths[0], lengths[1], lengths[0], lengths[1]),
217+
4 => new Thickness(lengths[0], lengths[1], lengths[2], lengths[3]),
218+
_ => throw new FormatException(SR.Format(SR.InvalidStringThickness, s)),
219+
};
219220
}
220221

221222
#endregion

0 commit comments

Comments
 (0)