Skip to content

Commit 4e18bca

Browse files
benaadamsbenaadams
authored
Remove unsafe use ASCII.GetBytes for WriteAscii (#18404)
* Use ParallelBitExtract for EncodeAsciiCharsToBytes * Use ASCII.GetBytes * Use span overloads * Betterize * Remove old comment
1 parent a29bacc commit 4e18bca

File tree

5 files changed

+36
-135
lines changed

5 files changed

+36
-135
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11538,7 +11538,7 @@ internal unsafe void CopyToFast(ref BufferWriter<PipeWriter> output)
1153811538
if (value != null)
1153911539
{
1154011540
output.Write(headerKey);
11541-
output.WriteAsciiNoValidation(value);
11541+
output.WriteAscii(value);
1154211542
}
1154311543
}
1154411544
}

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpResponseHeaders.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ static void CopyExtraHeaders(ref BufferWriter<PipeWriter> buffer, Dictionary<str
4848
if (value != null)
4949
{
5050
buffer.Write(CrLf);
51-
buffer.WriteAsciiNoValidation(kv.Key);
51+
buffer.WriteAscii(kv.Key);
5252
buffer.Write(ColonSpace);
53-
buffer.WriteAsciiNoValidation(value);
53+
buffer.WriteAscii(value);
5454
}
5555
}
5656
}

‎src/Servers/Kestrel/Core/test/PipelineExtensionTests.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public void EncodesAsAscii(string input, byte[] expected)
8787
{
8888
var pipeWriter = _pipe.Writer;
8989
var writer = new BufferWriter<PipeWriter>(pipeWriter);
90-
writer.WriteAsciiNoValidation(input);
90+
writer.WriteAscii(input);
9191
writer.Commit();
9292
pipeWriter.FlushAsync().GetAwaiter().GetResult();
9393
pipeWriter.Complete();
@@ -111,13 +111,13 @@ public void EncodesAsAscii(string input, byte[] expected)
111111
[InlineData("𤭢𐐝")]
112112
// non-ascii characters stored in 16 bits
113113
[InlineData("ñ٢⛄⛵")]
114-
public void WriteAsciiNoValidationWritesOnlyOneBytePerChar(string input)
114+
public void WriteAsciiWritesOnlyOneBytePerChar(string input)
115115
{
116116
// WriteAscii doesn't validate if characters are in the ASCII range
117117
// but it shouldn't produce more than one byte per character
118118
var writerBuffer = _pipe.Writer;
119119
var writer = new BufferWriter<PipeWriter>(writerBuffer);
120-
writer.WriteAsciiNoValidation(input);
120+
writer.WriteAscii(input);
121121
writer.Commit();
122122
writerBuffer.FlushAsync().GetAwaiter().GetResult();
123123
var reader = _pipe.Reader.ReadAsync().GetAwaiter().GetResult();
@@ -126,14 +126,14 @@ public void WriteAsciiNoValidationWritesOnlyOneBytePerChar(string input)
126126
}
127127

128128
[Fact]
129-
public void WriteAsciiNoValidation()
129+
public void WriteAscii()
130130
{
131131
const byte maxAscii = 0x7f;
132132
var writerBuffer = _pipe.Writer;
133133
var writer = new BufferWriter<PipeWriter>(writerBuffer);
134134
for (var i = 0; i < maxAscii; i++)
135135
{
136-
writer.WriteAsciiNoValidation(new string((char)i, 1));
136+
writer.WriteAscii(new string((char)i, 1));
137137
}
138138
writer.Commit();
139139
writerBuffer.FlushAsync().GetAwaiter().GetResult();
@@ -167,7 +167,7 @@ public void WritesAsciiAcrossBlockBoundaries(int stringLength, int gapSize)
167167
Assert.Equal(gapSize, writer.Span.Length);
168168

169169
var bufferLength = writer.Span.Length;
170-
writer.WriteAsciiNoValidation(testString);
170+
writer.WriteAscii(testString);
171171
Assert.NotEqual(bufferLength, writer.Span.Length);
172172
writer.Commit();
173173
writerBuffer.FlushAsync().GetAwaiter().GetResult();

‎src/Servers/Kestrel/shared/KnownHeaders.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -945,7 +945,7 @@ internal unsafe void CopyToFast(ref BufferWriter<PipeWriter> output)
945945
if (value != null)
946946
{{
947947
output.Write(headerKey);
948-
output.WriteAsciiNoValidation(value);
948+
output.WriteAscii(value);
949949
}}
950950
}}
951951
}}

‎src/Shared/ServerInfrastructure/BufferExtensions.cs

Lines changed: 26 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System;
5-
using System.Buffers;
4+
using System.Diagnostics;
65
using System.IO.Pipelines;
76
using System.Runtime.CompilerServices;
87
using System.Runtime.InteropServices;
8+
using System.Text;
99

1010
namespace System.Buffers
1111
{
@@ -40,26 +40,19 @@ public static ArraySegment<byte> GetArray(this ReadOnlyMemory<byte> memory)
4040
return result;
4141
}
4242

43-
internal static unsafe void WriteAsciiNoValidation(ref this BufferWriter<PipeWriter> buffer, string data)
43+
internal static void WriteAscii(ref this BufferWriter<PipeWriter> buffer, string data)
4444
{
4545
if (string.IsNullOrEmpty(data))
4646
{
4747
return;
4848
}
4949

5050
var dest = buffer.Span;
51-
var destLength = dest.Length;
5251
var sourceLength = data.Length;
53-
54-
// Fast path, try copying to the available memory directly
55-
if (sourceLength <= destLength)
52+
// Fast path, try encoding to the available memory directly
53+
if (sourceLength <= dest.Length)
5654
{
57-
fixed (char* input = data)
58-
fixed (byte* output = dest)
59-
{
60-
EncodeAsciiCharsToBytes(input, output, sourceLength);
61-
}
62-
55+
Encoding.ASCII.GetBytes(data, dest);
6356
buffer.Advance(sourceLength);
6457
}
6558
else
@@ -140,123 +133,31 @@ private static void WriteNumericMultiWrite(ref this BufferWriter<PipeWriter> buf
140133
}
141134

142135
[MethodImpl(MethodImplOptions.NoInlining)]
143-
private unsafe static void WriteAsciiMultiWrite(ref this BufferWriter<PipeWriter> buffer, string data)
136+
private static void WriteAsciiMultiWrite(ref this BufferWriter<PipeWriter> buffer, string data)
144137
{
145-
var remaining = data.Length;
146-
147-
fixed (char* input = data)
148-
{
149-
var inputSlice = input;
150-
151-
while (remaining > 0)
152-
{
153-
var writable = Math.Min(remaining, buffer.Span.Length);
154-
155-
if (writable == 0)
156-
{
157-
buffer.Ensure();
158-
continue;
159-
}
160-
161-
fixed (byte* output = buffer.Span)
162-
{
163-
EncodeAsciiCharsToBytes(inputSlice, output, writable);
164-
}
165-
166-
inputSlice += writable;
167-
remaining -= writable;
168-
169-
buffer.Advance(writable);
170-
}
171-
}
172-
}
173-
174-
private static unsafe void EncodeAsciiCharsToBytes(char* input, byte* output, int length)
175-
{
176-
// Note: Not BIGENDIAN or check for non-ascii
177-
const int Shift16Shift24 = (1 << 16) | (1 << 24);
178-
const int Shift8Identity = (1 << 8) | (1);
179-
180-
// Encode as bytes up to the first non-ASCII byte and return count encoded
181-
int i = 0;
182-
// Use Intrinsic switch
183-
if (IntPtr.Size == 8) // 64 bit
184-
{
185-
if (length < 4) goto trailing;
186-
187-
int unaligned = (int)(((ulong)input) & 0x7) >> 1;
188-
// Unaligned chars
189-
for (; i < unaligned; i++)
190-
{
191-
char ch = *(input + i);
192-
*(output + i) = (byte)ch; // Cast convert
193-
}
194-
195-
// Aligned
196-
int ulongDoubleCount = (length - i) & ~0x7;
197-
for (; i < ulongDoubleCount; i += 8)
198-
{
199-
ulong inputUlong0 = *(ulong*)(input + i);
200-
ulong inputUlong1 = *(ulong*)(input + i + 4);
201-
// Pack 16 ASCII chars into 16 bytes
202-
*(uint*)(output + i) =
203-
((uint)((inputUlong0 * Shift16Shift24) >> 24) & 0xffff) |
204-
((uint)((inputUlong0 * Shift8Identity) >> 24) & 0xffff0000);
205-
*(uint*)(output + i + 4) =
206-
((uint)((inputUlong1 * Shift16Shift24) >> 24) & 0xffff) |
207-
((uint)((inputUlong1 * Shift8Identity) >> 24) & 0xffff0000);
208-
}
209-
if (length - 4 > i)
210-
{
211-
ulong inputUlong = *(ulong*)(input + i);
212-
// Pack 8 ASCII chars into 8 bytes
213-
*(uint*)(output + i) =
214-
((uint)((inputUlong * Shift16Shift24) >> 24) & 0xffff) |
215-
((uint)((inputUlong * Shift8Identity) >> 24) & 0xffff0000);
216-
i += 4;
217-
}
218-
219-
trailing:
220-
for (; i < length; i++)
221-
{
222-
char ch = *(input + i);
223-
*(output + i) = (byte)ch; // Cast convert
224-
}
225-
}
226-
else // 32 bit
138+
var dataLength = data.Length;
139+
var offset = 0;
140+
var bytes = buffer.Span;
141+
do
227142
{
228-
// Unaligned chars
229-
if ((unchecked((int)input) & 0x2) != 0)
230-
{
231-
char ch = *input;
232-
i = 1;
233-
*(output) = (byte)ch; // Cast convert
234-
}
235-
236-
// Aligned
237-
int uintCount = (length - i) & ~0x3;
238-
for (; i < uintCount; i += 4)
143+
var writable = Math.Min(dataLength - offset, bytes.Length);
144+
// Zero length spans are possible, though unlikely.
145+
// ASCII.GetBytes and .Advance will both handle them so we won't special case for them.
146+
Encoding.ASCII.GetBytes(data.AsSpan(offset, writable), bytes);
147+
buffer.Advance(writable);
148+
149+
offset += writable;
150+
if (offset >= dataLength)
239151
{
240-
uint inputUint0 = *(uint*)(input + i);
241-
uint inputUint1 = *(uint*)(input + i + 2);
242-
// Pack 4 ASCII chars into 4 bytes
243-
*(ushort*)(output + i) = (ushort)(inputUint0 | (inputUint0 >> 8));
244-
*(ushort*)(output + i + 2) = (ushort)(inputUint1 | (inputUint1 >> 8));
245-
}
246-
if (length - 1 > i)
247-
{
248-
uint inputUint = *(uint*)(input + i);
249-
// Pack 2 ASCII chars into 2 bytes
250-
*(ushort*)(output + i) = (ushort)(inputUint | (inputUint >> 8));
251-
i += 2;
152+
Debug.Assert(offset == dataLength);
153+
// Encoded everything
154+
break;
252155
}
253156

254-
if (i < length)
255-
{
256-
char ch = *(input + i);
257-
*(output + i) = (byte)ch; // Cast convert
258-
}
259-
}
157+
// Get new span, more to encode.
158+
buffer.Ensure();
159+
bytes = buffer.Span;
160+
} while (true);
260161
}
261162

262163
private static byte[] NumericBytesScratch => _numericBytesScratch ?? CreateNumericBytesScratch();

0 commit comments

Comments
 (0)