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

Bump SixLabors.ImageSharp to v2.1.0 #5087

Merged
merged 6 commits into from
Apr 2, 2022
Merged

Conversation

frenzibyte
Copy link
Member

This is required for supporting macOS image clipboards, requiring TIFF encoder/decoder which is supported in ImageSharp v2.1.0.

Breaking changes come from SixLabors/ImageSharp#1730.

The important one is that ArrayPoolMemoryAllocator has been completely replaced with the new and default UniformUnmanagedMemoryPoolMemoryAllocator, which is still partially ArrayPool-based for smaller allocations, but uses a different pool implementation for larger allocations.

I can't check on the performance side of this, so would appreciate that being done prior to merge.

// recommended middle-ground https://github.com/SixLabors/docs/blob/master/articles/ImageSharp/MemoryManagement.md#working-in-memory-constrained-environments
SixLabors.ImageSharp.Configuration.Default.MemoryAllocator = ArrayPoolMemoryAllocator.CreateWithModeratePooling();
// recommended middle-ground https://github.com/SixLabors/docs/blob/main/articles/imagesharp/memorymanagement.md#configuring-the-pool-size
SixLabors.ImageSharp.Configuration.Default.MemoryAllocator = MemoryAllocator.Create(new MemoryAllocatorOptions { MaximumPoolSizeMegabytes = 1 });
Copy link
Member Author

Choose a reason for hiding this comment

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

Maximum pool size number comes from the previously used method CreateWithModeratePooling() (1024 * 1024).

Note that the default memory allocator uses one eighth of the platform's available memory according to GC.GetMemoryInfo(), which might consider this logic no longer necessary, unsure.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

After reading their new implementation, I believe we should remove this custom spec completely and see how things perform. The reason we had this was that the pool never cleaned up, but the documentation says it now does.

@peppy peppy enabled auto-merge April 2, 2022 02:08
@peppy peppy merged commit c81ff30 into ppy:master Apr 2, 2022
@frenzibyte frenzibyte deleted the imagesharp-2.1.0 branch April 2, 2022 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants