Skip to content

Commit

Permalink
Address regressions from previous formatting changes
Browse files Browse the repository at this point in the history 
- Use an internal interface implemented by char and byte to have dedicated CastFrom methods that are always inlineable due to very small size.
- Use pointers in some core formatting routines to avoid needing bulky IL for manipulating refs with spans, making various members more inlineable.
- Avoid Encoding.UTF8.GetBytes in various code paths by caching more UTF8 sequences on DateTimeFormatInfo and NumberFormatInfo
- Change FormatCustomizedTimeZone to special-case 2 vs 3+ tokens in order to avoid extra AppendSpan calls
- Fix growth logic in ValueListBuilder to not forcibly grow more than is needed
- Inline ValueListBuilder.AppendSpan and remove some bounds checks (at least on 64-bit)
- Change FormatDigits to special-case lengths of 1/2/4 and to use existing formatting routines rather than a custom one
- Remove the FormatDigits wrapper overload and just have all calls go to the main workhorse method.
- Remove the use of "..."u8 in R/O formatting that leads to needing to use additional span-based helpers.  The minimal gain on coreclr isn't worth the extra complication
- Changed some switches to include half the cases based on lowercasing the ASCII input char
- Moved Date/TimeOnly charsWritten into Try method to be closer to the source of truth rather than having the value far aware (this isn't for perf and could possibly even be a microregression, so I included it here to ensure it's not measurable).
  • Loading branch information
@stephentoub
stephentoub committed Apr 12, 2023
1 parent 1a3c5d2 commit b593d86
Show file tree
Hide file tree
Showing 16 changed files with 644 additions and 569 deletions.
1 change: 1 addition & 0 deletions src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@
<Compile Include="$(MSBuildThisFileDirectory)System\IObserver.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\IProgress.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\ISpanFormattable.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\IUtfChar.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\IUtf8SpanFormattable.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Lazy.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\LazyOfTTMetadata.cs" />
Expand Down
75 changes: 37 additions & 38 deletions ...ries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Formatter/Utf8Formatter.Date.G.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;

namespace System.Buffers.Text
{
public static partial class Utf8Formatter
Expand All @@ -18,7 +20,7 @@ public static partial class Utf8Formatter
// --------------------------
// 05/25/2017 10:30:15 -08:00
//
private static bool TryFormatDateTimeG(DateTime value, TimeSpan offset, Span<byte> destination, out int bytesWritten)
private static unsafe bool TryFormatDateTimeG(DateTime value, TimeSpan offset, Span<byte> destination, out int bytesWritten)
{
const int MinimumBytesNeeded = 19;

Expand All @@ -37,54 +39,51 @@ private static bool TryFormatDateTimeG(DateTime value, TimeSpan offset, Span<byt

bytesWritten = bytesRequired;

// Hoist most of the bounds checks on buffer.
{ var unused = destination[MinimumBytesNeeded - 1]; }

value.GetDate(out int year, out int month, out int day);
value.GetTime(out int hour, out int minute, out int second);

Number.WriteTwoDigits((uint)month, destination, 0);
destination[2] = Utf8Constants.Slash;

Number.WriteTwoDigits((uint)day, destination, 3);
destination[5] = Utf8Constants.Slash;
fixed (byte* dest = &MemoryMarshal.GetReference(destination))
{
Number.WriteTwoDigits((uint)month, dest);
dest[2] = Utf8Constants.Slash;

Number.WriteFourDigits((uint)year, destination, 6);
destination[10] = Utf8Constants.Space;
Number.WriteTwoDigits((uint)day, dest + 3);
dest[5] = Utf8Constants.Slash;

Number.WriteTwoDigits((uint)hour, destination, 11);
destination[13] = Utf8Constants.Colon;
Number.WriteFourDigits((uint)year, dest + 6);
dest[10] = Utf8Constants.Space;

Number.WriteTwoDigits((uint)minute, destination, 14);
destination[16] = Utf8Constants.Colon;
Number.WriteTwoDigits((uint)hour, dest + 11);
dest[13] = Utf8Constants.Colon;

Number.WriteTwoDigits((uint)second, destination, 17);
Number.WriteTwoDigits((uint)minute, dest + 14);
dest[16] = Utf8Constants.Colon;

if (offset != Utf8Constants.NullUtcOffset)
{
int offsetTotalMinutes = (int)(offset.Ticks / TimeSpan.TicksPerMinute);
byte sign;
Number.WriteTwoDigits((uint)second, dest + 17);

if (offsetTotalMinutes < 0)
if (offset != Utf8Constants.NullUtcOffset)
{
sign = Utf8Constants.Minus;
offsetTotalMinutes = -offsetTotalMinutes;
int offsetTotalMinutes = (int)(offset.Ticks / TimeSpan.TicksPerMinute);
byte sign;

if (offsetTotalMinutes < 0)
{
sign = Utf8Constants.Minus;
offsetTotalMinutes = -offsetTotalMinutes;
}
else
{
sign = Utf8Constants.Plus;
}

int offsetHours = Math.DivRem(offsetTotalMinutes, 60, out int offsetMinutes);

dest[19] = Utf8Constants.Space;
dest[20] = sign;
Number.WriteTwoDigits((uint)offsetHours, dest + 21);
dest[23] = Utf8Constants.Colon;
Number.WriteTwoDigits((uint)offsetMinutes, dest + 24);
}
else
{
sign = Utf8Constants.Plus;
}

int offsetHours = Math.DivRem(offsetTotalMinutes, 60, out int offsetMinutes);

// Writing the value backward allows the JIT to optimize by
// performing a single bounds check against buffer.

Number.WriteTwoDigits((uint)offsetMinutes, destination, 24);
destination[23] = Utf8Constants.Colon;
Number.WriteTwoDigits((uint)offsetHours, destination, 21);
destination[20] = sign;
destination[19] = Utf8Constants.Space;
}

return true;
Expand Down
4 changes: 2 additions & 2 deletions ...es/System.Private.CoreLib/src/System/Buffers/Text/Utf8Formatter/Utf8Formatter.TimeSpan.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ public static partial class Utf8Formatter
public static bool TryFormat(TimeSpan value, Span<byte> destination, out int bytesWritten, StandardFormat format = default)
{
TimeSpanFormat.StandardFormat sf = TimeSpanFormat.StandardFormat.C;
string? decimalSeparator = null;
ReadOnlySpan<byte> decimalSeparator = default;

char symbol = FormattingHelpers.GetSymbolOrDefault(format, 'c');
if (symbol != 'c' && (symbol | 0x20) != 't')
{
decimalSeparator = DateTimeFormatInfo.InvariantInfo.DecimalSeparator;
decimalSeparator = DateTimeFormatInfo.InvariantInfo.DecimalSeparatorTChar<byte>();
if (symbol == 'g')
{
sf = TimeSpanFormat.StandardFormat.g;
Expand Down
14 changes: 12 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Byte.cs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Buffers.Binary;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Numerics;
Expand All @@ -23,7 +22,8 @@ public readonly struct Byte
IBinaryInteger<byte>,
IMinMaxValue<byte>,
IUnsignedNumber<byte>,
IUtf8SpanFormattable
IUtf8SpanFormattable,
IUtfChar<byte>
{
private readonly byte m_value; // Do not rename (binary serialization)

Expand Down Expand Up @@ -1203,5 +1203,15 @@ static bool INumberBase<byte>.TryConvertToTruncating<TOther>(byte value, [MaybeN

/// <inheritdoc cref="IUnaryPlusOperators{TSelf, TResult}.op_UnaryPlus(TSelf)" />
static byte IUnaryPlusOperators<byte, byte>.operator +(byte value) => (byte)(+value);

//
// IUtfChar
//

static byte IUtfChar<byte>.CastFrom(byte value) => value;
static byte IUtfChar<byte>.CastFrom(char value) => (byte)value;
static byte IUtfChar<byte>.CastFrom(int value) => (byte)value;
static byte IUtfChar<byte>.CastFrom(uint value) => (byte)value;
static byte IUtfChar<byte>.CastFrom(ulong value) => (byte)value;
}
}
13 changes: 12 additions & 1 deletion src/libraries/System.Private.CoreLib/src/System/Char.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ public readonly struct Char
IBinaryInteger<char>,
IMinMaxValue<char>,
IUnsignedNumber<char>,
IUtf8SpanFormattable
IUtf8SpanFormattable,
IUtfChar<char>
{
//
// Member Variables
Expand Down Expand Up @@ -2010,5 +2011,15 @@ static bool ISpanParsable<char>.TryParse(ReadOnlySpan<char> s, IFormatProvider?

/// <inheritdoc cref="IUnaryPlusOperators{TSelf, TResult}.op_UnaryPlus(TSelf)" />
static char IUnaryPlusOperators<char, char>.operator +(char value) => (char)(+value);

//
// IUtfChar
//

static char IUtfChar<char>.CastFrom(byte value) => (char)value;
static char IUtfChar<char>.CastFrom(char value) => value;
static char IUtfChar<char>.CastFrom(int value) => (char)value;
static char IUtfChar<char>.CastFrom(uint value) => (char)value;
static char IUtfChar<char>.CastFrom(ulong value) => (char)value;
}
}
19 changes: 16 additions & 3 deletions src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ValueListBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private void AppendMultiChar(scoped ReadOnlySpan<T> source)
{
if ((uint)(_pos + source.Length) > (uint)_span.Length)
{
Grow(source.Length);
Grow(_span.Length - _pos + source.Length);
}

source.CopyTo(_span.Slice(_pos));
Expand All @@ -100,16 +100,29 @@ public void Insert(int index, scoped ReadOnlySpan<T> source)
_pos += source.Length;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public Span<T> AppendSpan(int length)
{
Debug.Assert(length >= 0);

int pos = _pos;
if ((uint)(pos + length) > (uint)_span.Length)
Span<T> span = _span;
if ((ulong)(uint)pos + (ulong)(uint)length <= (ulong)(uint)span.Length) // same guard condition as in Span<T>.Slice on 64-bit
{
_pos = pos + length;
return span.Slice(pos, length);
}
else
{
Grow(length);
return AppendSpanWithGrow(length);
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
private Span<T> AppendSpanWithGrow(int length)
{
int pos = _pos;
Grow(_span.Length - pos + length);
_pos += length;
return _span.Slice(pos, length);
}
Expand Down
52 changes: 13 additions & 39 deletions src/libraries/System.Private.CoreLib/src/System/DateOnly.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,16 +491,14 @@ private static ParseFailureKind TryParseExactInternal(ReadOnlySpan<char> s, Read

if (format.Length == 1)
{
switch (format[0])
switch (format[0] | 0x20)
{
case 'o':
case 'O':
format = OFormat;
provider = CultureInfo.InvariantCulture.DateTimeFormat;
break;

case 'r':
case 'R':
format = RFormat;
provider = CultureInfo.InvariantCulture.DateTimeFormat;
break;
Expand Down Expand Up @@ -570,16 +568,14 @@ private static ParseFailureKind TryParseExactInternal(ReadOnlySpan<char> s, stri

if (format.Length == 1)
{
switch (format[0])
switch (format[0] | 0x20)
{
case 'o':
case 'O':
format = OFormat;
dtfiToUse = CultureInfo.InvariantCulture.DateTimeFormat;
break;

case 'r':
case 'R':
format = RFormat;
dtfiToUse = CultureInfo.InvariantCulture.DateTimeFormat;
break;
Expand Down Expand Up @@ -750,30 +746,25 @@ public string ToString([StringSyntax(StringSyntaxAttribute.DateOnlyFormat)] stri

if (format.Length == 1)
{
switch (format[0])
switch (format[0] | 0x20)
{
case 'o':
case 'O':
return string.Create(10, this, (destination, value) =>
{
bool b = DateTimeFormat.TryFormatDateOnlyO(value.Year, value.Month, value.Day, destination);
Debug.Assert(b);
DateTimeFormat.TryFormatDateOnlyO(value.Year, value.Month, value.Day, destination, out int charsWritten);
Debug.Assert(charsWritten == destination.Length);
});

case 'r':
case 'R':
return string.Create(16, this, (destination, value) =>
{
bool b = DateTimeFormat.TryFormatDateOnlyR(value.DayOfWeek, value.Year, value.Month, value.Day, destination);
Debug.Assert(b);
DateTimeFormat.TryFormatDateOnlyR(value.DayOfWeek, value.Year, value.Month, value.Day, destination, out int charsWritten);
Debug.Assert(charsWritten == destination.Length);
});

case 'm':
case 'M':
case 'd':
case 'D':
case 'y':
case 'Y':
return DateTimeFormat.Format(GetEquivalentDateTime(), format, provider);

default:
Expand All @@ -800,7 +791,7 @@ bool IUtf8SpanFormattable.TryFormat(Span<byte> utf8Destination, out int bytesWri
TryFormatCore(utf8Destination, out bytesWritten, format, provider);

private bool TryFormatCore<TChar>(Span<TChar> destination, out int charsWritten, [StringSyntax(StringSyntaxAttribute.DateOnlyFormat)] ReadOnlySpan<char> format, IFormatProvider? provider = null)
where TChar : unmanaged, IBinaryInteger<TChar>
where TChar : unmanaged, IUtfChar<TChar>
{
if (format.Length == 0)
{
Expand All @@ -809,39 +800,22 @@ private bool TryFormatCore<TChar>(Span<TChar> destination, out int charsWritten,

if (format.Length == 1)
{
switch (format[0])
switch (format[0] | 0x20)
{
case 'o':
case 'O':
if (!DateTimeFormat.TryFormatDateOnlyO(Year, Month, Day, destination))
{
charsWritten = 0;
return false;
}
charsWritten = 10;
return true;
return DateTimeFormat.TryFormatDateOnlyO(Year, Month, Day, destination, out charsWritten);

case 'r':
case 'R':

if (!DateTimeFormat.TryFormatDateOnlyR(DayOfWeek, Year, Month, Day, destination))
{
charsWritten = 0;
return false;
}
charsWritten = 16;
return true;
return DateTimeFormat.TryFormatDateOnlyR(DayOfWeek, Year, Month, Day, destination, out charsWritten);

case 'm':
case 'M':
case 'd':
case 'D':
case 'y':
case 'Y':
return DateTimeFormat.TryFormat(GetEquivalentDateTime(), destination, out charsWritten, format, provider);

default:
throw new FormatException(SR.Argument_BadFormatSpecifier);
ThrowHelper.ThrowFormatException_BadFormatSpecifier();
break;
}
}

Expand Down
Loading

0 comments on commit b593d86

Please sign in to comment.