Skip to content

Commit d95a996

Browse files
JimBobSquarePantsJimBobSquarePants
authored
Merge pull request #1316 from SixLabors/js/fix-non-seekable-load
Fix non-seekable stream reading.
2 parents 8e2792e + 582000d commit d95a996

File tree

13 files changed

+1144
-203
lines changed

13 files changed

+1144
-203
lines changed

‎src/ImageSharp/IO/ChunkedMemoryStream.cs‎

Lines changed: 570 additions & 0 deletions
Large diffs are not rendered by default.

‎src/ImageSharp/IO/FixedCapacityPooledMemoryStream.cs‎

Lines changed: 0 additions & 69 deletions
This file was deleted.

‎src/ImageSharp/Image.FromStream.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Threading;
99
using System.Threading.Tasks;
1010
using SixLabors.ImageSharp.Formats;
11+
using SixLabors.ImageSharp.IO;
1112
using SixLabors.ImageSharp.Memory;
1213
using SixLabors.ImageSharp.PixelFormats;
1314

@@ -731,7 +732,7 @@ private static T WithSeekableStream<T>(
731732
}
732733

733734
// We want to be able to load images from things like HttpContext.Request.Body
734-
using MemoryStream memoryStream = configuration.MemoryAllocator.AllocateFixedCapacityMemoryStream(stream.Length);
735+
using var memoryStream = new ChunkedMemoryStream(configuration.MemoryAllocator);
735736
stream.CopyTo(memoryStream, configuration.StreamProcessingBufferSize);
736737
memoryStream.Position = 0;
737738

@@ -775,7 +776,7 @@ private static async Task<T> WithSeekableStreamAsync<T>(
775776
return await action(stream, cancellationToken).ConfigureAwait(false);
776777
}
777778

778-
using MemoryStream memoryStream = configuration.MemoryAllocator.AllocateFixedCapacityMemoryStream(stream.Length);
779+
using var memoryStream = new ChunkedMemoryStream(configuration.MemoryAllocator);
779780
await stream.CopyToAsync(memoryStream, configuration.StreamProcessingBufferSize, cancellationToken).ConfigureAwait(false);
780781
memoryStream.Position = 0;
781782

‎src/ImageSharp/Memory/MemoryAllocatorExtensions.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,5 @@ internal static MemoryGroup<T> AllocateGroup<T>(
100100
AllocationOptions options = AllocationOptions.None)
101101
where T : struct
102102
=> MemoryGroup<T>.Allocate(memoryAllocator, totalLength, bufferAlignment, options);
103-
104-
internal static MemoryStream AllocateFixedCapacityMemoryStream(this MemoryAllocator allocator, long length) =>
105-
new FixedCapacityPooledMemoryStream(length, allocator);
106103
}
107104
}

‎src/ImageSharp/Memory/MemoryOwnerExtensions.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace SixLabors.ImageSharp.Memory
1313
/// </summary>
1414
internal static class MemoryOwnerExtensions
1515
{
16+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
1617
public static Span<T> GetSpan<T>(this IMemoryOwner<T> buffer)
1718
=> buffer.Memory.Span;
1819

‎tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs‎

Lines changed: 86 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,12 @@ public class BufferedStreams
2121
private MemoryStream stream4;
2222
private MemoryStream stream5;
2323
private MemoryStream stream6;
24+
private ChunkedMemoryStream chunkedMemoryStream1;
25+
private ChunkedMemoryStream chunkedMemoryStream2;
2426
private BufferedReadStream bufferedStream1;
2527
private BufferedReadStream bufferedStream2;
28+
private BufferedReadStream bufferedStream3;
29+
private BufferedReadStream bufferedStream4;
2630
private BufferedReadStreamWrapper bufferedStreamWrap1;
2731
private BufferedReadStreamWrapper bufferedStreamWrap2;
2832

@@ -35,8 +39,20 @@ public void CreateStreams()
3539
this.stream4 = new MemoryStream(this.buffer);
3640
this.stream5 = new MemoryStream(this.buffer);
3741
this.stream6 = new MemoryStream(this.buffer);
42+
this.stream6 = new MemoryStream(this.buffer);
43+
44+
this.chunkedMemoryStream1 = new ChunkedMemoryStream(Configuration.Default.MemoryAllocator);
45+
this.chunkedMemoryStream1.Write(this.buffer);
46+
this.chunkedMemoryStream1.Position = 0;
47+
48+
this.chunkedMemoryStream2 = new ChunkedMemoryStream(Configuration.Default.MemoryAllocator);
49+
this.chunkedMemoryStream2.Write(this.buffer);
50+
this.chunkedMemoryStream2.Position = 0;
51+
3852
this.bufferedStream1 = new BufferedReadStream(Configuration.Default, this.stream3);
3953
this.bufferedStream2 = new BufferedReadStream(Configuration.Default, this.stream4);
54+
this.bufferedStream3 = new BufferedReadStream(Configuration.Default, this.chunkedMemoryStream1);
55+
this.bufferedStream4 = new BufferedReadStream(Configuration.Default, this.chunkedMemoryStream2);
4056
this.bufferedStreamWrap1 = new BufferedReadStreamWrapper(this.stream5);
4157
this.bufferedStreamWrap2 = new BufferedReadStreamWrapper(this.stream6);
4258
}
@@ -46,8 +62,12 @@ public void DestroyStreams()
4662
{
4763
this.bufferedStream1?.Dispose();
4864
this.bufferedStream2?.Dispose();
65+
this.bufferedStream3?.Dispose();
66+
this.bufferedStream4?.Dispose();
4967
this.bufferedStreamWrap1?.Dispose();
5068
this.bufferedStreamWrap2?.Dispose();
69+
this.chunkedMemoryStream1?.Dispose();
70+
this.chunkedMemoryStream2?.Dispose();
5171
this.stream1?.Dispose();
5272
this.stream2?.Dispose();
5373
this.stream3?.Dispose();
@@ -86,6 +106,21 @@ public int BufferedReadStreamRead()
86106
return r;
87107
}
88108

109+
[Benchmark]
110+
public int BufferedReadStreamChunkedRead()
111+
{
112+
int r = 0;
113+
BufferedReadStream reader = this.bufferedStream3;
114+
byte[] b = this.chunk2;
115+
116+
for (int i = 0; i < reader.Length / 2; i++)
117+
{
118+
r += reader.Read(b, 0, 2);
119+
}
120+
121+
return r;
122+
}
123+
89124
[Benchmark]
90125
public int BufferedReadStreamWrapRead()
91126
{
@@ -129,6 +164,20 @@ public int BufferedReadStreamReadByte()
129164
return r;
130165
}
131166

167+
[Benchmark]
168+
public int BufferedReadStreamChunkedReadByte()
169+
{
170+
int r = 0;
171+
BufferedReadStream reader = this.bufferedStream4;
172+
173+
for (int i = 0; i < reader.Length; i++)
174+
{
175+
r += reader.ReadByte();
176+
}
177+
178+
return r;
179+
}
180+
132181
[Benchmark]
133182
public int BufferedReadStreamWrapReadByte()
134183
{
@@ -167,40 +216,46 @@ private static byte[] CreateTestBytes()
167216
}
168217

169218
/*
170-
BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041
219+
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.450 (2004/?/20H1)
171220
Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores
172-
.NET Core SDK=3.1.301
173-
[Host] : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
174-
Job-LKLBOT : .NET Framework 4.8 (4.8.4180.0), X64 RyuJIT
175-
Job-RSTMKF : .NET Core 2.1.19 (CoreCLR 4.6.28928.01, CoreFX 4.6.28928.04), X64 RyuJIT
176-
Job-PZIHIV : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
221+
.NET Core SDK=3.1.401
222+
[Host] : .NET Core 3.1.7 (CoreCLR 4.700.20.36602, CoreFX 4.700.20.37001), X64 RyuJIT
223+
Job-OKZLUV : .NET Framework 4.8 (4.8.4084.0), X64 RyuJIT
224+
Job-CPYMXV : .NET Core 2.1.21 (CoreCLR 4.6.29130.01, CoreFX 4.6.29130.02), X64 RyuJIT
225+
Job-BSGVGU : .NET Core 3.1.7 (CoreCLR 4.700.20.36602, CoreFX 4.700.20.37001), X64 RyuJIT
177226
178227
IterationCount=3 LaunchCount=1 WarmupCount=3
179228
180-
| Method | Runtime | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated |
181-
|------------------------------- |-------------- |----------:|------------:|-----------:|------:|--------:|------:|------:|------:|----------:|
182-
| StandardStreamRead | .NET 4.7.2 | 63.238 us | 49.7827 us | 2.7288 us | 0.66 | 0.13 | - | - | - | - |
183-
| BufferedReadStreamRead | .NET 4.7.2 | 66.092 us | 0.4273 us | 0.0234 us | 0.69 | 0.11 | - | - | - | - |
184-
| BufferedReadStreamWrapRead | .NET 4.7.2 | 26.216 us | 3.0527 us | 0.1673 us | 0.27 | 0.04 | - | - | - | - |
185-
| StandardStreamReadByte | .NET 4.7.2 | 97.900 us | 261.7204 us | 14.3458 us | 1.00 | 0.00 | - | - | - | - |
186-
| BufferedReadStreamReadByte | .NET 4.7.2 | 97.260 us | 1.2979 us | 0.0711 us | 1.01 | 0.15 | - | - | - | - |
187-
| BufferedReadStreamWrapReadByte | .NET 4.7.2 | 19.170 us | 2.2296 us | 0.1222 us | 0.20 | 0.03 | - | - | - | - |
188-
| ArrayReadByte | .NET 4.7.2 | 12.878 us | 11.1292 us | 0.6100 us | 0.13 | 0.02 | - | - | - | - |
189-
| | | | | | | | | | | |
190-
| StandardStreamRead | .NET Core 2.1 | 60.618 us | 131.7038 us | 7.2191 us | 0.78 | 0.10 | - | - | - | - |
191-
| BufferedReadStreamRead | .NET Core 2.1 | 30.006 us | 25.2499 us | 1.3840 us | 0.38 | 0.02 | - | - | - | - |
192-
| BufferedReadStreamWrapRead | .NET Core 2.1 | 29.241 us | 6.5020 us | 0.3564 us | 0.37 | 0.01 | - | - | - | - |
193-
| StandardStreamReadByte | .NET Core 2.1 | 78.074 us | 15.8463 us | 0.8686 us | 1.00 | 0.00 | - | - | - | - |
194-
| BufferedReadStreamReadByte | .NET Core 2.1 | 14.737 us | 20.1510 us | 1.1045 us | 0.19 | 0.01 | - | - | - | - |
195-
| BufferedReadStreamWrapReadByte | .NET Core 2.1 | 13.234 us | 1.4711 us | 0.0806 us | 0.17 | 0.00 | - | - | - | - |
196-
| ArrayReadByte | .NET Core 2.1 | 9.373 us | 0.6108 us | 0.0335 us | 0.12 | 0.00 | - | - | - | - |
197-
| | | | | | | | | | | |
198-
| StandardStreamRead | .NET Core 3.1 | 52.151 us | 19.9456 us | 1.0933 us | 0.65 | 0.03 | - | - | - | - |
199-
| BufferedReadStreamRead | .NET Core 3.1 | 29.217 us | 0.2490 us | 0.0136 us | 0.36 | 0.01 | - | - | - | - |
200-
| BufferedReadStreamWrapRead | .NET Core 3.1 | 32.962 us | 7.1382 us | 0.3913 us | 0.41 | 0.02 | - | - | - | - |
201-
| StandardStreamReadByte | .NET Core 3.1 | 80.310 us | 45.0350 us | 2.4685 us | 1.00 | 0.00 | - | - | - | - |
202-
| BufferedReadStreamReadByte | .NET Core 3.1 | 13.092 us | 0.6268 us | 0.0344 us | 0.16 | 0.00 | - | - | - | - |
203-
| BufferedReadStreamWrapReadByte | .NET Core 3.1 | 13.282 us | 3.8689 us | 0.2121 us | 0.17 | 0.01 | - | - | - | - |
204-
| ArrayReadByte | .NET Core 3.1 | 9.349 us | 2.9860 us | 0.1637 us | 0.12 | 0.00 | - | - | - | - |
229+
| Method | Job | Runtime | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated |
230+
|---------------------------------- |----------- |-------------- |-----------:|----------:|----------:|------:|--------:|------:|------:|------:|----------:|
231+
| StandardStreamRead | Job-OKZLUV | .NET 4.7.2 | 66.785 us | 15.768 us | 0.8643 us | 0.83 | 0.01 | - | - | - | - |
232+
| BufferedReadStreamRead | Job-OKZLUV | .NET 4.7.2 | 97.389 us | 17.658 us | 0.9679 us | 1.21 | 0.01 | - | - | - | - |
233+
| BufferedReadStreamChunkedRead | Job-OKZLUV | .NET 4.7.2 | 96.006 us | 16.286 us | 0.8927 us | 1.20 | 0.02 | - | - | - | - |
234+
| BufferedReadStreamWrapRead | Job-OKZLUV | .NET 4.7.2 | 37.064 us | 14.640 us | 0.8024 us | 0.46 | 0.02 | - | - | - | - |
235+
| StandardStreamReadByte | Job-OKZLUV | .NET 4.7.2 | 80.315 us | 26.676 us | 1.4622 us | 1.00 | 0.00 | - | - | - | - |
236+
| BufferedReadStreamReadByte | Job-OKZLUV | .NET 4.7.2 | 118.706 us | 38.013 us | 2.0836 us | 1.48 | 0.00 | - | - | - | - |
237+
| BufferedReadStreamChunkedReadByte | Job-OKZLUV | .NET 4.7.2 | 115.437 us | 33.352 us | 1.8282 us | 1.44 | 0.01 | - | - | - | - |
238+
| BufferedReadStreamWrapReadByte | Job-OKZLUV | .NET 4.7.2 | 16.449 us | 11.400 us | 0.6249 us | 0.20 | 0.00 | - | - | - | - |
239+
| ArrayReadByte | Job-OKZLUV | .NET 4.7.2 | 10.416 us | 1.866 us | 0.1023 us | 0.13 | 0.00 | - | - | - | - |
240+
| | | | | | | | | | | | |
241+
| StandardStreamRead | Job-CPYMXV | .NET Core 2.1 | 71.425 us | 50.441 us | 2.7648 us | 0.82 | 0.03 | - | - | - | - |
242+
| BufferedReadStreamRead | Job-CPYMXV | .NET Core 2.1 | 32.816 us | 6.655 us | 0.3648 us | 0.38 | 0.01 | - | - | - | - |
243+
| BufferedReadStreamChunkedRead | Job-CPYMXV | .NET Core 2.1 | 31.995 us | 7.751 us | 0.4249 us | 0.37 | 0.01 | - | - | - | - |
244+
| BufferedReadStreamWrapRead | Job-CPYMXV | .NET Core 2.1 | 31.970 us | 4.170 us | 0.2286 us | 0.37 | 0.01 | - | - | - | - |
245+
| StandardStreamReadByte | Job-CPYMXV | .NET Core 2.1 | 86.909 us | 18.565 us | 1.0176 us | 1.00 | 0.00 | - | - | - | - |
246+
| BufferedReadStreamReadByte | Job-CPYMXV | .NET Core 2.1 | 14.596 us | 10.889 us | 0.5969 us | 0.17 | 0.01 | - | - | - | - |
247+
| BufferedReadStreamChunkedReadByte | Job-CPYMXV | .NET Core 2.1 | 13.629 us | 1.569 us | 0.0860 us | 0.16 | 0.00 | - | - | - | - |
248+
| BufferedReadStreamWrapReadByte | Job-CPYMXV | .NET Core 2.1 | 13.566 us | 1.743 us | 0.0956 us | 0.16 | 0.00 | - | - | - | - |
249+
| ArrayReadByte | Job-CPYMXV | .NET Core 2.1 | 9.771 us | 6.658 us | 0.3650 us | 0.11 | 0.00 | - | - | - | - |
250+
| | | | | | | | | | | | |
251+
| StandardStreamRead | Job-BSGVGU | .NET Core 3.1 | 53.265 us | 65.819 us | 3.6078 us | 0.81 | 0.05 | - | - | - | - |
252+
| BufferedReadStreamRead | Job-BSGVGU | .NET Core 3.1 | 33.163 us | 9.569 us | 0.5245 us | 0.51 | 0.01 | - | - | - | - |
253+
| BufferedReadStreamChunkedRead | Job-BSGVGU | .NET Core 3.1 | 33.001 us | 6.114 us | 0.3351 us | 0.50 | 0.01 | - | - | - | - |
254+
| BufferedReadStreamWrapRead | Job-BSGVGU | .NET Core 3.1 | 29.448 us | 7.120 us | 0.3902 us | 0.45 | 0.01 | - | - | - | - |
255+
| StandardStreamReadByte | Job-BSGVGU | .NET Core 3.1 | 65.619 us | 6.732 us | 0.3690 us | 1.00 | 0.00 | - | - | - | - |
256+
| BufferedReadStreamReadByte | Job-BSGVGU | .NET Core 3.1 | 13.989 us | 3.464 us | 0.1899 us | 0.21 | 0.00 | - | - | - | - |
257+
| BufferedReadStreamChunkedReadByte | Job-BSGVGU | .NET Core 3.1 | 13.806 us | 1.710 us | 0.0938 us | 0.21 | 0.00 | - | - | - | - |
258+
| BufferedReadStreamWrapReadByte | Job-BSGVGU | .NET Core 3.1 | 13.690 us | 1.523 us | 0.0835 us | 0.21 | 0.00 | - | - | - | - |
259+
| ArrayReadByte | Job-BSGVGU | .NET Core 3.1 | 10.792 us | 8.228 us | 0.4510 us | 0.16 | 0.01 | - | - | - | - |
205260
*/
206261
}

0 commit comments

Comments
 (0)