Skip to content

Commit 599dbd3

Browse files
committed
Revert "Switch Json's PooledByteBufferWriter to shared ArrayBuffer helper (dotnet#111348)"
This reverts commit d454419.
1 parent 4365cd2 commit 599dbd3

25 files changed

+361
-58
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,93 +1,249 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Buffers;
45
using System.Diagnostics;
6+
using System.Diagnostics.CodeAnalysis;
57
using System.IO;
68
using System.IO.Pipelines;
7-
using System.Net;
9+
using System.Runtime.CompilerServices;
810
using System.Threading;
911
using System.Threading.Tasks;
1012

1113
namespace System.Text.Json
1214
{
1315
internal sealed class PooledByteBufferWriter : PipeWriter, IDisposable
1416
{
17+
// This class allows two possible configurations: if rentedBuffer is not null then
18+
// it can be used as an IBufferWriter and holds a buffer that should eventually be
19+
// returned to the shared pool. If rentedBuffer is null, then the instance is in a
20+
// cleared/disposed state and it must re-rent a buffer before it can be used again.
21+
private byte[]? _rentedBuffer;
22+
private int _index;
23+
private readonly Stream? _stream;
24+
1525
private const int MinimumBufferSize = 256;
1626

17-
private ArrayBuffer _buffer;
18-
private readonly Stream? _stream;
27+
// Value copied from Array.MaxLength in System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Array.cs.
28+
public const int MaximumBufferSize = 0X7FFFFFC7;
29+
30+
private PooledByteBufferWriter()
31+
{
32+
#if NET
33+
// Ensure we are in sync with the Array.MaxLength implementation.
34+
Debug.Assert(MaximumBufferSize == Array.MaxLength);
35+
#endif
36+
}
1937

20-
public PooledByteBufferWriter(int initialCapacity)
38+
public PooledByteBufferWriter(int initialCapacity) : this()
2139
{
22-
_buffer = new ArrayBuffer(initialCapacity, usePool: true);
40+
Debug.Assert(initialCapacity > 0);
41+
42+
_rentedBuffer = ArrayPool<byte>.Shared.Rent(initialCapacity);
43+
_index = 0;
2344
}
2445

2546
public PooledByteBufferWriter(int initialCapacity, Stream stream) : this(initialCapacity)
2647
{
2748
_stream = stream;
2849
}
2950

30-
public ReadOnlySpan<byte> WrittenSpan => _buffer.ActiveSpan;
51+
public ReadOnlyMemory<byte> WrittenMemory
52+
{
53+
get
54+
{
55+
Debug.Assert(_rentedBuffer != null);
56+
Debug.Assert(_index <= _rentedBuffer.Length);
57+
return _rentedBuffer.AsMemory(0, _index);
58+
}
59+
}
3160

32-
public ReadOnlyMemory<byte> WrittenMemory => _buffer.ActiveMemory;
61+
public int WrittenCount
62+
{
63+
get
64+
{
65+
Debug.Assert(_rentedBuffer != null);
66+
return _index;
67+
}
68+
}
3369

34-
public int Capacity => _buffer.Capacity;
70+
public int Capacity
71+
{
72+
get
73+
{
74+
Debug.Assert(_rentedBuffer != null);
75+
return _rentedBuffer.Length;
76+
}
77+
}
3578

36-
public void Clear() => _buffer.Discard(_buffer.ActiveLength);
79+
public int FreeCapacity
80+
{
81+
get
82+
{
83+
Debug.Assert(_rentedBuffer != null);
84+
return _rentedBuffer.Length - _index;
85+
}
86+
}
87+
88+
public void Clear()
89+
{
90+
ClearHelper();
91+
}
3792

38-
public void ClearAndReturnBuffers() => _buffer.ClearAndReturnBuffer();
93+
public void ClearAndReturnBuffers()
94+
{
95+
Debug.Assert(_rentedBuffer != null);
3996

40-
public void Dispose() => _buffer.Dispose();
97+
ClearHelper();
98+
byte[] toReturn = _rentedBuffer;
99+
_rentedBuffer = null;
100+
ArrayPool<byte>.Shared.Return(toReturn);
101+
}
102+
103+
private void ClearHelper()
104+
{
105+
Debug.Assert(_rentedBuffer != null);
106+
Debug.Assert(_index <= _rentedBuffer.Length);
107+
108+
_rentedBuffer.AsSpan(0, _index).Clear();
109+
_index = 0;
110+
}
111+
112+
// Returns the rented buffer back to the pool
113+
public void Dispose()
114+
{
115+
if (_rentedBuffer == null)
116+
{
117+
return;
118+
}
119+
120+
ClearHelper();
121+
byte[] toReturn = _rentedBuffer;
122+
_rentedBuffer = null;
123+
ArrayPool<byte>.Shared.Return(toReturn);
124+
}
41125

42126
public void InitializeEmptyInstance(int initialCapacity)
43127
{
44128
Debug.Assert(initialCapacity > 0);
45-
Debug.Assert(_buffer.ActiveLength == 0);
129+
Debug.Assert(_rentedBuffer is null);
46130

47-
_buffer.EnsureAvailableSpace(initialCapacity);
131+
_rentedBuffer = ArrayPool<byte>.Shared.Rent(initialCapacity);
132+
_index = 0;
48133
}
49134

50-
public static PooledByteBufferWriter CreateEmptyInstanceForCaching() => new PooledByteBufferWriter(initialCapacity: 0);
135+
public static PooledByteBufferWriter CreateEmptyInstanceForCaching() => new PooledByteBufferWriter();
51136

52-
public override void Advance(int count) => _buffer.Commit(count);
137+
public override void Advance(int count)
138+
{
139+
Debug.Assert(_rentedBuffer != null);
140+
Debug.Assert(count >= 0);
141+
Debug.Assert(_index <= _rentedBuffer.Length - count);
142+
_index += count;
143+
}
53144

54145
public override Memory<byte> GetMemory(int sizeHint = MinimumBufferSize)
55146
{
56-
Debug.Assert(sizeHint > 0);
57-
58-
_buffer.EnsureAvailableSpace(sizeHint);
59-
return _buffer.AvailableMemory;
147+
CheckAndResizeBuffer(sizeHint);
148+
return _rentedBuffer.AsMemory(_index);
60149
}
61150

62151
public override Span<byte> GetSpan(int sizeHint = MinimumBufferSize)
63152
{
64-
Debug.Assert(sizeHint > 0);
65-
66-
_buffer.EnsureAvailableSpace(sizeHint);
67-
return _buffer.AvailableSpan;
153+
CheckAndResizeBuffer(sizeHint);
154+
return _rentedBuffer.AsSpan(_index);
68155
}
69156

70157
#if NET
71-
internal void WriteToStream(Stream destination) => destination.Write(_buffer.ActiveSpan);
158+
internal void WriteToStream(Stream destination)
159+
{
160+
destination.Write(WrittenMemory.Span);
161+
}
72162
#else
73-
internal void WriteToStream(Stream destination) => destination.Write(_buffer.ActiveMemory);
163+
internal void WriteToStream(Stream destination)
164+
{
165+
Debug.Assert(_rentedBuffer != null);
166+
destination.Write(_rentedBuffer, 0, _index);
167+
}
74168
#endif
75169

170+
private void CheckAndResizeBuffer(int sizeHint)
171+
{
172+
Debug.Assert(_rentedBuffer != null);
173+
Debug.Assert(sizeHint > 0);
174+
175+
int currentLength = _rentedBuffer.Length;
176+
int availableSpace = currentLength - _index;
177+
178+
// If we've reached ~1GB written, grow to the maximum buffer
179+
// length to avoid incessant minimal growths causing perf issues.
180+
if (_index >= MaximumBufferSize / 2)
181+
{
182+
sizeHint = Math.Max(sizeHint, MaximumBufferSize - currentLength);
183+
}
184+
185+
if (sizeHint > availableSpace)
186+
{
187+
int growBy = Math.Max(sizeHint, currentLength);
188+
189+
int newSize = currentLength + growBy;
190+
191+
if ((uint)newSize > MaximumBufferSize)
192+
{
193+
newSize = currentLength + sizeHint;
194+
if ((uint)newSize > MaximumBufferSize)
195+
{
196+
ThrowHelper.ThrowOutOfMemoryException_BufferMaximumSizeExceeded((uint)newSize);
197+
}
198+
}
199+
200+
byte[] oldBuffer = _rentedBuffer;
201+
202+
_rentedBuffer = ArrayPool<byte>.Shared.Rent(newSize);
203+
204+
Debug.Assert(oldBuffer.Length >= _index);
205+
Debug.Assert(_rentedBuffer.Length >= _index);
206+
207+
Span<byte> oldBufferAsSpan = oldBuffer.AsSpan(0, _index);
208+
oldBufferAsSpan.CopyTo(_rentedBuffer);
209+
oldBufferAsSpan.Clear();
210+
ArrayPool<byte>.Shared.Return(oldBuffer);
211+
}
212+
213+
Debug.Assert(_rentedBuffer.Length - _index > 0);
214+
Debug.Assert(_rentedBuffer.Length - _index >= sizeHint);
215+
}
216+
76217
public override async ValueTask<FlushResult> FlushAsync(CancellationToken cancellationToken = default)
77218
{
78219
Debug.Assert(_stream is not null);
220+
#if NET
79221
await _stream.WriteAsync(WrittenMemory, cancellationToken).ConfigureAwait(false);
222+
#else
223+
Debug.Assert(_rentedBuffer != null);
224+
await _stream.WriteAsync(_rentedBuffer, 0, _index, cancellationToken).ConfigureAwait(false);
225+
#endif
80226
Clear();
81227

82228
return new FlushResult(isCanceled: false, isCompleted: false);
83229
}
84230

85231
public override bool CanGetUnflushedBytes => true;
86-
public override long UnflushedBytes => _buffer.ActiveLength;
232+
public override long UnflushedBytes => _index;
87233

88234
// This type is used internally in JsonSerializer to help buffer and flush bytes to the underlying Stream.
89235
// It's only pretending to be a PipeWriter and doesn't need Complete or CancelPendingFlush for the internal usage.
90236
public override void CancelPendingFlush() => throw new NotImplementedException();
91237
public override void Complete(Exception? exception = null) => throw new NotImplementedException();
92238
}
239+
240+
internal static partial class ThrowHelper
241+
{
242+
[DoesNotReturn]
243+
[MethodImpl(MethodImplOptions.NoInlining)]
244+
public static void ThrowOutOfMemoryException_BufferMaximumSizeExceeded(uint capacity)
245+
{
246+
throw new OutOfMemoryException(SR.Format(SR.BufferMaximumSizeExceeded, capacity));
247+
}
248+
}
93249
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.IO;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
9+
/// <summary>Extensions to enable the tests to use the span-based Read/Write methods that only exist in netcoreapp.</summary>
10+
internal static class StreamSpanExtensions
11+
{
12+
// These implementations are inefficient and are just for testing purposes.
13+
14+
public static int Read(this Stream stream, Span<byte> destination)
15+
{
16+
byte[] array = new byte[destination.Length];
17+
int bytesRead = stream.Read(array, 0, array.Length);
18+
new Span<byte>(array, 0, bytesRead).CopyTo(destination);
19+
return bytesRead;
20+
}
21+
22+
public static void Write(this Stream stream, ReadOnlySpan<byte> source) =>
23+
stream.Write(source.ToArray(), 0, source.Length);
24+
25+
public static ValueTask<int> ReadAsync(this Stream stream, Memory<byte> destination, CancellationToken cancellationToken = default(CancellationToken))
26+
{
27+
byte[] array = new byte[destination.Length];
28+
return new ValueTask<int>(stream.ReadAsync(array, 0, array.Length, cancellationToken).ContinueWith(t =>
29+
{
30+
int bytesRead = t.GetAwaiter().GetResult();
31+
new Span<byte>(array, 0, bytesRead).CopyTo(destination.Span);
32+
return bytesRead;
33+
}));
34+
}
35+
36+
public static Task WriteAsync(this Stream stream, ReadOnlyMemory<byte> source, CancellationToken cancellationToken = default(CancellationToken)) =>
37+
stream.WriteAsync(source.ToArray(), 0, source.Length, cancellationToken);
38+
}

src/libraries/Common/tests/System/Net/Http/DefaultCredentialsTest.cs

-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Generic;
5-
using System.IO;
65
using System.Security.Principal;
76
using System.Threading.Tasks;
87

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.IO;
5+
using System.Runtime.InteropServices;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Xunit;
9+
10+
namespace System.Net
11+
{
12+
public static class StreamArrayExtensions
13+
{
14+
public static ValueTask WriteAsync(this Stream stream, ReadOnlyMemory<byte> memory)
15+
{
16+
bool isArray = MemoryMarshal.TryGetArray(memory, out ArraySegment<byte> segment);
17+
Assert.True(isArray);
18+
19+
return new ValueTask(stream.WriteAsync(segment.Array, segment.Offset, segment.Count));
20+
}
21+
22+
public static ValueTask WriteAsync(this StreamWriter writer, string text)
23+
{
24+
return new ValueTask(writer.WriteAsync(text.ToCharArray(), 0, text.Length));
25+
}
26+
27+
public static ValueTask<int> ReadAsync(this Stream stream, ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken)
28+
{
29+
bool isArray = MemoryMarshal.TryGetArray(buffer, out ArraySegment<byte> segment);
30+
Assert.True(isArray);
31+
32+
return new ValueTask<int>(stream.ReadAsync(segment.Array, segment.Offset, segment.Count, cancellationToken));
33+
}
34+
}
35+
}

src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatter.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ private void WriteInternal(IExternalScopeProvider? scopeProvider, TextWriter tex
9696
writer.Flush();
9797
}
9898

99-
var messageBytes = output.WrittenSpan;
99+
var messageBytes = output.WrittenMemory.Span;
100100
var logMessageBuffer = ArrayPool<char>.Shared.Rent(Encoding.UTF8.GetMaxCharCount(messageBytes.Length));
101101
try
102102
{

src/libraries/Microsoft.Extensions.Logging.Console/src/Microsoft.Extensions.Logging.Console.csproj

-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
<ItemGroup>
1818
<Compile Include="$(CommonPath)Extensions\Logging\NullExternalScopeProvider.cs" Link="Common\src\Extensions\Logging\NullExternalScopeProvider.cs" />
1919
<Compile Include="$(CommonPath)Extensions\Logging\NullScope.cs" Link="Common\src\Extensions\Logging\NullScope.cs" />
20-
<Compile Include="$(CommonPath)System\Net\ArrayBuffer.cs" Link="Common\System\Net\ArrayBuffer.cs" />
2120
<Compile Include="$(CommonPath)System\Text\Json\PooledByteBufferWriter.cs" Link="Common\System\Text\Json\PooledByteBufferWriter.cs" />
2221
<Compile Include="$(CommonPath)System\ThrowHelper.cs" Link="Common\System\ThrowHelper.cs" />
2322
</ItemGroup>
@@ -30,7 +29,6 @@
3029
</ItemGroup>
3130

3231
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
33-
<Compile Include="$(CommonPath)System\IO\StreamExtensions.netstandard.cs" Link="Common\System\IO\StreamExtensions.netstandard.cs" />
3432
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicallyAccessedMembersAttribute.cs" />
3533
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicallyAccessedMemberTypes.cs" />
3634
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicDependencyAttribute.cs" />

src/libraries/Microsoft.Extensions.Logging.Console/src/Resources/Strings.resx

+3
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@
117117
<resheader name="writer">
118118
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
119119
</resheader>
120+
<data name="BufferMaximumSizeExceeded" xml:space="preserve">
121+
<value>Cannot allocate a buffer of size {0}.</value>
122+
</data>
120123
<data name="QueueModeNotSupported" xml:space="preserve">
121124
<value>{0} is not a supported queue mode value.</value>
122125
</data>

0 commit comments

Comments
 (0)