Skip to content

Commit 0e21d28

Browse files
alefranzalefranz
committed
Addressed feedback and refactored
1 parent b245eb8 commit 0e21d28

File tree

3 files changed

+61
-89
lines changed

3 files changed

+61
-89
lines changed

‎src/Http/WebUtilities/src/FileBufferingPipeWriter.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ public sealed class FileBufferingPipeWriter : PipeWriter, IAsyncDisposable
2727
private readonly long? _bufferLimit;
2828
private readonly Func<string> _tempFileDirectoryAccessor;
2929

30+
private long _pipeWrittenBytes = 0;
31+
3032
/// <summary>
3133
/// Initializes a new instance of <see cref="FileBufferingWriteStream"/>.
3234
/// </summary>
@@ -121,6 +123,7 @@ private static void ThrowArgumentException(byte[] buffer, int offset, int count)
121123

122124
public override void Advance(int bytes)
123125
{
126+
_pipeWrittenBytes += bytes;
124127
_pipeWriter.Advance(bytes);
125128
}
126129

@@ -136,16 +139,21 @@ public override void Complete(Exception? exception = null)
136139

137140
public override Memory<byte> GetMemory(int sizeHint = 0)
138141
{
142+
if (_pipeWrittenBytes + sizeHint > _memoryThreshold) throw new NotImplementedException();
143+
139144
return _pipeWriter.GetMemory(sizeHint);
140145
}
141146

142147
public override Span<byte> GetSpan(int sizeHint = 0)
143148
{
149+
if (_pipeWrittenBytes + sizeHint > _memoryThreshold) throw new NotImplementedException();
150+
144151
return _pipeWriter.GetSpan(sizeHint);
145152
}
146153

147154
public override ValueTask<FlushResult> FlushAsync(CancellationToken cancellationToken = default)
148155
{
156+
_pipeWrittenBytes = 0;
149157
return _pipeWriter.FlushAsync(cancellationToken);
150158
}
151159

‎src/Http/WebUtilities/src/HttpResponsePipeWriter.cs

Lines changed: 49 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public class HttpResponsePipeWriter : TextWriter
2424

2525
public override Encoding Encoding { get; }
2626

27+
private int _uncommittedBytes = 0;
2728
private bool _disposed;
2829

2930
public HttpResponsePipeWriter(
@@ -38,122 +39,72 @@ public HttpResponsePipeWriter(
3839

3940
public override void Write(char value)
4041
{
41-
if (_disposed)
42-
{
43-
throw new ObjectDisposedException(nameof(HttpResponseStreamWriter));
44-
}
45-
4642
_singleCharArray[0] = value;
47-
var span = new Span<char>(_singleCharArray, 0, 1);
48-
Write(span);
43+
WriteInternal(_singleCharArray.AsSpan(0, 1));
4944
}
5045

5146
public override void Write(char[] values, int index, int count)
5247
{
53-
if (_disposed)
54-
{
55-
throw new ObjectDisposedException(nameof(HttpResponseStreamWriter));
56-
}
57-
58-
if (values == null || count == 0)
48+
if (values == null)
5949
{
6050
return;
6151
}
6252

63-
var value = new Span<char>(values, index, count);
64-
Write(value);
53+
WriteInternal(values.AsSpan(index, count));
6554
}
6655

6756
public override void Write(ReadOnlySpan<char> value)
6857
{
69-
if (_disposed)
70-
{
71-
throw new ObjectDisposedException(nameof(HttpResponseStreamWriter));
72-
}
73-
7458
if (value == null)
7559
{
7660
return;
7761
}
7862

79-
var length = _encoder.GetByteCount(value, false);
80-
var buffer = _writer.GetSpan(length);
81-
_encoder.GetBytes(value, buffer, false);
82-
_writer.Advance(length);
63+
WriteInternal(value);
8364
}
8465

8566
public override void Write(string? value)
8667
{
87-
if (_disposed)
88-
{
89-
throw new ObjectDisposedException(nameof(HttpResponseStreamWriter));
90-
}
91-
9268
if (value == null)
9369
{
9470
return;
9571
}
9672

97-
Write(value.AsSpan());
73+
WriteInternal(value.AsSpan());
9874
}
9975

10076
public override void WriteLine(ReadOnlySpan<char> value)
101-
{
102-
if (_disposed)
103-
{
104-
throw new ObjectDisposedException(nameof(HttpResponseStreamWriter));
105-
}
106-
107-
Write(value);
108-
Write(NewLine);
109-
}
77+
=> WriteInternal(value, addNewLine: true);
11078

11179
public override Task WriteAsync(char value)
11280
{
113-
if (_disposed)
114-
{
115-
return GetObjectDisposedTask();
116-
}
117-
11881
_singleCharArray[0] = value;
11982

120-
return WriteAsync(_singleCharArray, 0, 1);
83+
return WriteInternalAsync(_singleCharArray.AsSpan(0, 1));
12184
}
12285

12386
public override Task WriteAsync(char[] values, int index, int count)
124-
{
125-
if (_disposed)
126-
{
127-
return GetObjectDisposedTask();
128-
}
129-
130-
if (values == null || count == 0)
131-
{
132-
return Task.CompletedTask;
133-
}
134-
135-
var value = new Span<char>(values, index, count);
136-
Write(value);
137-
138-
return Task.CompletedTask;
139-
}
87+
=> WriteInternalAsync(values.AsSpan(index, count));
14088

14189
public override Task WriteAsync(string? value)
14290
{
143-
if (_disposed)
91+
if (value == null)
14492
{
145-
return GetObjectDisposedTask();
93+
return Task.CompletedTask;
14694
}
14795

148-
var length = _encoder.GetByteCount(value, false);
149-
var buffer = _writer.GetSpan(length);
150-
_encoder.GetBytes(value, buffer, false);
151-
_writer.Advance(length);
152-
153-
return Task.CompletedTask;
96+
return WriteInternalAsync(value.AsSpan());
15497
}
15598

15699
public override Task WriteAsync(ReadOnlyMemory<char> value, CancellationToken cancellationToken = default)
100+
=> WriteInternalAsync(value.Span, cancellationToken);
101+
102+
103+
public override Task WriteLineAsync(ReadOnlyMemory<char> value, CancellationToken cancellationToken = default)
104+
=> WriteInternalAsync(value.Span, cancellationToken, addNewLine: true);
105+
106+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
107+
private Task WriteInternalAsync(ReadOnlySpan<char> value, CancellationToken cancellationToken = default, bool addNewLine = false)
157108
{
158109
if (_disposed)
159110
{
@@ -165,37 +116,49 @@ public override Task WriteAsync(ReadOnlyMemory<char> value, CancellationToken ca
165116
return Task.FromCanceled(cancellationToken);
166117
}
167118

168-
if (value.IsEmpty)
119+
if (value.IsEmpty && !addNewLine)
169120
{
170121
return Task.CompletedTask;
171122
}
172123

173-
Write(value.Span);
124+
WriteInternal(value, addNewLine);
174125

175-
return Task.CompletedTask;
126+
return LazyFlushAsync(cancellationToken);
176127
}
177128

178-
public override Task WriteLineAsync(ReadOnlyMemory<char> value, CancellationToken cancellationToken = default)
129+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
130+
private Task LazyFlushAsync(CancellationToken cancellationToken = default)
179131
{
180-
if (_disposed)
132+
// The max size of a chunk is 4089.
133+
if (_uncommittedBytes >= 4089)
181134
{
182-
return GetObjectDisposedTask();
135+
_uncommittedBytes = 0;
136+
return _writer.FlushAsync(cancellationToken).AsTask();
183137
}
184138

185-
if (cancellationToken.IsCancellationRequested)
186-
{
187-
return Task.FromCanceled(cancellationToken);
188-
}
139+
return Task.CompletedTask;
140+
}
189141

190-
if (value.IsEmpty && NewLine.Length == 0)
142+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
143+
private void WriteInternal(ReadOnlySpan<char> value, bool addNewLine = false)
144+
{
145+
if (_disposed)
191146
{
192-
return Task.CompletedTask;
147+
throw new ObjectDisposedException(nameof(HttpResponseStreamWriter));
193148
}
194149

195-
Write(value.Span);
196-
Write(NewLine);
150+
WriteSpan(value);
151+
if (addNewLine) WriteSpan(NewLine);
152+
}
197153

198-
return Task.CompletedTask;
154+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
155+
private void WriteSpan(ReadOnlySpan<char> value)
156+
{
157+
var length = _encoder.GetByteCount(value, false);
158+
var buffer = _writer.GetSpan(length);
159+
_uncommittedBytes += length;
160+
_encoder.GetBytes(value, buffer, false);
161+
_writer.Advance(length);
199162
}
200163

201164
public override void Flush()
@@ -206,7 +169,7 @@ public override void Flush()
206169
}
207170

208171
FlushEncoder();
209-
// TOOD: flush
172+
// TOOD: flush?
210173
}
211174

212175
// Perf: FlushAsync is invoked to ensure any buffered content is asynchronously written to the underlying
@@ -228,7 +191,6 @@ public override async ValueTask DisposeAsync()
228191
{
229192
_disposed = true;
230193
await FlushInternalAsync();
231-
_writer.Complete();
232194
}
233195

234196
await base.DisposeAsync();
@@ -240,8 +202,7 @@ protected override void Dispose(bool disposing)
240202
{
241203
_disposed = true;
242204
FlushEncoder();
243-
// TOOD: flush
244-
_writer.Complete();
205+
// Flush not needed
245206
}
246207

247208
base.Dispose(disposing);
@@ -255,7 +216,6 @@ private ValueTask<FlushResult> FlushInternalAsync()
255216

256217
private void FlushEncoder()
257218
{
258-
// flush encoder
259219
var empty = new ReadOnlySpan<char>();
260220
var length = _encoder.GetByteCount(empty, true);
261221
if (length > 0)

‎src/Mvc/MvcNoDeps.slnf

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
"solution": {
33
"path": "..\\..\\AspNetCore.sln",
44
"projects": [
5+
"src\\Http\\WebUtilities\\perf\\Microsoft.AspNetCore.WebUtilities.Performance\\Microsoft.AspNetCore.WebUtilities.Performance.csproj",
6+
"src\\Http\\WebUtilities\\src\\Microsoft.AspNetCore.WebUtilities.csproj",
7+
"src\\Http\\WebUtilities\\test\\Microsoft.AspNetCore.WebUtilities.Tests.csproj",
58
"src\\Mvc\\Mvc.Abstractions\\src\\Microsoft.AspNetCore.Mvc.Abstractions.csproj",
69
"src\\Mvc\\Mvc.Abstractions\\test\\Microsoft.AspNetCore.Mvc.Abstractions.Test.csproj",
710
"src\\Mvc\\Mvc.Analyzers\\src\\Microsoft.AspNetCore.Mvc.Analyzers.csproj",
@@ -36,6 +39,7 @@
3639
"src\\Mvc\\Mvc.ViewFeatures\\test\\Microsoft.AspNetCore.Mvc.ViewFeatures.Test.csproj",
3740
"src\\Mvc\\Mvc\\src\\Microsoft.AspNetCore.Mvc.csproj",
3841
"src\\Mvc\\Mvc\\test\\Microsoft.AspNetCore.Mvc.Test.csproj",
42+
"src\\Mvc\\benchmarks\\Microsoft.AspNetCore.Mvc.Performance.Views\\Microsoft.AspNetCore.Mvc.Performance.Views.csproj",
3943
"src\\Mvc\\benchmarks\\Microsoft.AspNetCore.Mvc.Performance\\Microsoft.AspNetCore.Mvc.Performance.csproj",
4044
"src\\Mvc\\samples\\MvcSandbox\\MvcSandbox.csproj",
4145
"src\\Mvc\\shared\\Mvc.Core.TestCommon\\Microsoft.AspNetCore.Mvc.Core.TestCommon.csproj",

0 commit comments

Comments
 (0)