Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use RecyclableMemoryStream #16949

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
See https://github.com/OrchardCMS/OrchardCore/pull/16057 for more information.
-->
<PackageVersion Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.1.0" />
<PackageVersion Include="Microsoft.IO.RecyclableMemoryStream" Version="3.0.1" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
<PackageVersion Include="MimeKit" Version="4.8.0" />
<PackageVersion Include="MiniProfiler.AspNetCore.Mvc" Version="4.3.8" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

<ItemGroup>
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Indexing.Abstractions\OrchardCore.Indexing.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Infrastructure.Abstractions\OrchardCore.Infrastructure.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Media.Abstractions\OrchardCore.Media.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Module.Targets\OrchardCore.Module.Targets.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.ResourceManagement\OrchardCore.ResourceManagement.csproj" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Cysharp.Text;
using Microsoft.IO;
using UglyToad.PdfPig;

namespace OrchardCore.Media.Indexing;
Expand All @@ -11,14 +12,14 @@ public async Task<string> GetTextAsync(string path, Stream fileStream)
// https://github.com/UglyToad/PdfPig/blob/master/src/UglyToad.PdfPig.Core/StreamInputBytes.cs#L45.
// Thus if it isn't, which is the case with e.g. Azure Blob Storage, we need to copy it to a new, seekable
// Stream.
MemoryStream seekableStream = null;
RecyclableMemoryStream seekableStream = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should think about copying the stream on the file system instead of memory. #16955

try
{
if (!fileStream.CanSeek)
{
// Since fileStream.Length might not be supported either, we can't preconfigure the capacity of the
// MemoryStream.
seekableStream = new MemoryStream();
seekableStream = MemoryStreamFactory.GetStream();
// While this involves loading the file into memory, we don't really have a choice.
await fileStream.CopyToAsync(seekableStream);
seekableStream.Position = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public async Task<IActionResult> Index(string sitemapId, CancellationToken cance

document.Declaration = new XDeclaration("1.0", "utf-8", null);

var stream = new MemoryStream();
using var stream = MemoryStreamFactory.GetStream();
await document.SaveAsync(stream, SaveOptions.None, cancellationToken);

if (stream.Length >= ErrorLength)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public async Task<IActionResult> ServiceEndpoint([ModelBinder(BinderType = typeo
};

// Save to an intermediate MemoryStream to preserve the encoding declaration.
using var stream = new MemoryStream();
using var stream = MemoryStreamFactory.GetStream();
using (var w = XmlWriter.Create(stream, settings))
{
var result = _writer.MapMethodResponse(methodResponse);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Infrastructure.Abstractions\OrchardCore.Infrastructure.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Module.Targets\OrchardCore.Module.Targets.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.XmlRpc.Abstractions\OrchardCore.XmlRpc.Abstractions.csproj" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,17 @@ public class BlobFileStore : IFileStore
private readonly IClock _clock;
private readonly BlobContainerClient _blobContainer;
private readonly IContentTypeProvider _contentTypeProvider;

private readonly string _basePrefix;

public BlobFileStore(BlobStorageOptions options, IClock clock, IContentTypeProvider contentTypeProvider)
public BlobFileStore(
BlobStorageOptions options,
IClock clock,
IContentTypeProvider contentTypeProvider)
{
_options = options;
_clock = clock;
_contentTypeProvider = contentTypeProvider;

_blobContainer = new BlobContainerClient(_options.ConnectionString, _options.ContainerName);

if (!string.IsNullOrEmpty(_options.BasePath))
Expand Down Expand Up @@ -436,6 +439,7 @@ private async Task CreateDirectoryAsync(string path)

// Create a directory marker file to make this directory appear when listing directories.
using var stream = new MemoryStream(MarkerFileContent);

await placeholderBlob.UploadAsync(stream);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<ItemGroup>
<ProjectReference Include="..\OrchardCore.Abstractions\OrchardCore.Abstractions.csproj" />
<ProjectReference Include="..\OrchardCore.FileStorage.Abstractions\OrchardCore.FileStorage.Abstractions.csproj" />
<ProjectReference Include="..\OrchardCore.Infrastructure.Abstractions\OrchardCore.Infrastructure.Abstractions.csproj" />
<ProjectReference Include="..\OrchardCore.Liquid.Abstractions\OrchardCore.Liquid.Abstractions.csproj" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using Microsoft.IO;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an actual infrastructure package that is no "abstractions" or a common project where we put helpers?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is Infrastructure project. but not all of these projects depend on Infrastructure they depend on Infrastructure.Abstractions` I am also not happy to add this to the abstraction project. I am open to better placement.


namespace OrchardCore;

public static class MemoryStreamFactory
{
private static readonly RecyclableMemoryStreamManager _manager = new();

public static RecyclableMemoryStream GetStream(string tag = null)
=> _manager.GetStream(tag);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
<FrameworkReference Include="Microsoft.AspNetCore.App" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.IO.RecyclableMemoryStream" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\OrchardCore.Abstractions\OrchardCore.Abstractions.csproj" />
<ProjectReference Include="..\OrchardCore.Data.Abstractions\OrchardCore.Data.Abstractions.csproj" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ public Task<TDocument> DeserializeAsync<TDocument>(byte[] data)
data = Decompress(data);
}

using var ms = new MemoryStream(data);

var document = JsonSerializer.Deserialize<TDocument>(ms, _serializerOptions);
var document = JsonSerializer.Deserialize<TDocument>(data, _serializerOptions);

return Task.FromResult(document);
}
Expand All @@ -60,7 +58,7 @@ internal static bool IsCompressed(byte[] data)
internal static byte[] Compress(byte[] data)
{
using var input = new MemoryStream(data);
using var output = new MemoryStream();
using var output = MemoryStreamFactory.GetStream();
using (var gzip = new GZipStream(output, CompressionMode.Compress))
{
input.CopyTo(gzip);
Expand All @@ -77,7 +75,7 @@ internal static byte[] Compress(byte[] data)
internal static byte[] Decompress(byte[] data)
{
using var input = new MemoryStream(data);
using var output = new MemoryStream();
using var output = MemoryStreamFactory.GetStream();
using (var gzip = new GZipStream(input, CompressionMode.Decompress))
{
gzip.CopyTo(output);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,16 @@ public class CommonGeneratorMethods : IGlobalMethodProvider
/// Converts a Base64 encoded gzip stream to an uncompressed Base64 string.
/// See http://www.txtwizard.net/compression.
/// </summary>
private static readonly GlobalMethod _gZip = new()
private readonly GlobalMethod _gZip = new()
{
Name = "gzip",
Method = serviceProvider => (Func<string, string>)(encoded =>
{
var bytes = Convert.FromBase64String(encoded);
using var gzip = new GZipStream(new MemoryStream(bytes), CompressionMode.Decompress);
using var stream = new MemoryStream(bytes);
using var gzip = new GZipStream(stream, CompressionMode.Decompress);

var decompressed = new MemoryStream();
using var decompressed = MemoryStreamFactory.GetStream();
var buffer = new byte[1024];
int nRead;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ public object Evaluate(IScriptingScope scope, string script)
}

using var fileStream = fileInfo.CreateReadStream();
using var ms = new MemoryStream();
fileStream.CopyTo(ms);
return Convert.ToBase64String(ms.ToArray());
using var memoryStream = MemoryStreamFactory.GetStream();

return Convert.ToBase64String(memoryStream.GetBuffer());
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ public async Task AddSourcesAsync(string tenant, IConfigurationBuilder builder)
if (configuration is not null)
{
var configurationString = configuration.ToJsonString(JOptions.Default);
builder.AddTenantJsonStream(new MemoryStream(Encoding.UTF8.GetBytes(configurationString)));
using var stream = new MemoryStream(Encoding.UTF8.GetBytes(configurationString));

builder.AddTenantJsonStream(stream);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ public async Task AddSourcesAsync(IConfigurationBuilder builder)
if (document.ShellsSettings is not null)
{
var shellsSettingsString = document.ShellsSettings.ToJsonString(JOptions.Default);
builder.AddTenantJsonStream(new MemoryStream(Encoding.UTF8.GetBytes(shellsSettingsString)));
using var stream = new MemoryStream(Encoding.UTF8.GetBytes(shellsSettingsString));

builder.AddTenantJsonStream(stream);
}
}

Expand All @@ -53,7 +55,9 @@ public async Task AddSourcesAsync(string tenant, IConfigurationBuilder builder)
{
var shellSettings = new JsonObject { [tenant] = document.ShellsSettings[tenant] };
var shellSettingsString = shellSettings.ToJsonString(JOptions.Default);
builder.AddTenantJsonStream(new MemoryStream(Encoding.UTF8.GetBytes(shellSettingsString)));
using var stream = new MemoryStream(Encoding.UTF8.GetBytes(shellSettingsString));

builder.AddTenantJsonStream(stream);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ public async Task SaveAsync(string tenant, IDictionary<string, string> data)

public async Task RemoveAsync(string tenant)
{
var appsettings = IFileStoreExtensions.Combine(null, _container, tenant, OrchardCoreConstants.Configuration.ApplicationSettingsFileName);
var appSettings = IFileStoreExtensions.Combine(null, _container, tenant, OrchardCoreConstants.Configuration.ApplicationSettingsFileName);

var fileInfo = await _shellsFileStore.GetFileInfoAsync(appsettings);
var fileInfo = await _shellsFileStore.GetFileInfoAsync(appSettings);
if (fileInfo != null)
{
await _shellsFileStore.RemoveFileAsync(appsettings);
await _shellsFileStore.RemoveFileAsync(appSettings);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
<ItemGroup>
<ProjectReference Include="..\OrchardCore.Abstractions\OrchardCore.Abstractions.csproj" />
<ProjectReference Include="..\OrchardCore.FileStorage.AzureBlob\OrchardCore.FileStorage.AzureBlob.csproj" />
<ProjectReference Include="..\OrchardCore.Infrastructure.Abstractions\OrchardCore.Infrastructure.Abstractions.csproj" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public static JsonObject GetContentStepRecipe(ContentItem contentItem, Action<Js

public async Task<HttpResponseMessage> PostRecipeAsync(JsonObject recipe, bool ensureSuccess = true)
{
using var zipStream = new MemoryStream();
await using var zipStream = MemoryStreamFactory.GetStream();
using (var zip = new ZipArchive(zipStream, ZipArchiveMode.Create, true))
{
var entry = zip.CreateEntry("Recipe.json");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public void HtmlLocalizerDoesNotFormatTwiceIfFormattedTranslationContainsCurlyBr
{
var htmlLocalizer = new PortableObjectHtmlLocalizer(localizer);
var unformatted = htmlLocalizer["The page (ID:{0}) was deleted.", "{1}"];
var memStream = new MemoryStream();
var memStream = MemoryStreamFactory.GetStream();
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
var textWriter = new StreamWriter(memStream);
var textReader = new StreamReader(memStream);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public async Task DisposesMediaCreatingStreams()
#pragma warning restore CA1859
try
{
inputStream = new MemoryStream();
inputStream = MemoryStreamFactory.GetStream();
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
originalStream = inputStream;

// Add original stream to streams to maintain reference to test disposal.
Expand Down Expand Up @@ -78,7 +78,7 @@ public class TestMediaEventHandler : IMediaCreatingEventHandler
{
public async Task<Stream> MediaCreatingAsync(MediaCreatingContext context, Stream inputStream)
{
var outStream = new MemoryStream();
var outStream = MemoryStreamFactory.GetStream();
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
await inputStream.CopyToAsync(outStream);

return outStream;
Expand Down
2 changes: 1 addition & 1 deletion test/OrchardCore.Tests/Stubs/MemoryFileBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class MemoryFileBuilder

public async Task SetFileAsync(string subpath, Stream stream)
{
using var ms = new MemoryStream();
using var ms = MemoryStreamFactory.GetStream();
await stream.CopyToAsync(ms);
VirtualFiles[subpath] = ms.ToArray();
}
Expand Down