Skip to content

Allow -1 (unbounded) parallelism; validate settings #3110

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.

Sign up for GitHub

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

Merged
JimBobSquarePants merged 1 commit into from
Apr 9, 2026
+199 −14
Merged

Allow -1 (unbounded) parallelism; validate settings #3110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 9 additions & 2 deletions src/ImageSharp/Advanced/ParallelExecutionSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ public readonly struct ParallelExecutionSettings
/// <summary>
/// Initializes a new instance of the <see cref="ParallelExecutionSettings"/> struct.
/// </summary>
/// <param name="maxDegreeOfParallelism">The value used for initializing <see cref="ParallelOptions.MaxDegreeOfParallelism"/> when using TPL.</param>
/// <param name="maxDegreeOfParallelism">
/// The value used for initializing <see cref="ParallelOptions.MaxDegreeOfParallelism"/> when using TPL.
/// Set to <c>-1</c> to leave the degree of parallelism unbounded.
Copy link
Copy Markdown
Member

@antonfirsov antonfirsov Apr 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@JimBobSquarePants first I didn't want to comment because it felt like nitpicking on terminology, but now I'm afraid the proposed semantics for -1 are odd.

The official docs do not say -1 means unbounded, they only state that

If it is -1, there is no limit on the number of concurrently running operations

This doesn't mean that there will be no reasonable bounds on the number of operations, it means that the ThreadPool will dynamically determine the number.

https://github.com/SixLabors/ImageSharp.Drawing/blob/a4d03787dfc5a18cfe106d13b907a20d31534a8d/src/ImageSharp.Drawing/Processing/Backends/ParallelExecutionHelper.cs#L21-L22

However, it looks like the drawing code really interprets it as unbounded letting it go as high as the number of rows to draw. For CPU-bound work it doesn't make sense to go above the number of CPU-s available.

Long story short, `-1' should mean "let the runtime decide", and we should not attempt to outsmart it.

/// </param>
/// <param name="minimumPixelsProcessedPerTask">The value for <see cref="MinimumPixelsProcessedPerTask"/>.</param>
/// <param name="memoryAllocator">The <see cref="MemoryAllocator"/>.</param>
public ParallelExecutionSettings(
Expand All @@ -44,7 +47,10 @@ public ParallelExecutionSettings(
/// <summary>
/// Initializes a new instance of the <see cref="ParallelExecutionSettings"/> struct.
/// </summary>
/// <param name="maxDegreeOfParallelism">The value used for initializing <see cref="ParallelOptions.MaxDegreeOfParallelism"/> when using TPL.</param>
/// <param name="maxDegreeOfParallelism">
/// The value used for initializing <see cref="ParallelOptions.MaxDegreeOfParallelism"/> when using TPL.
/// Set to <c>-1</c> to leave the degree of parallelism unbounded.
/// </param>
/// <param name="memoryAllocator">The <see cref="MemoryAllocator"/>.</param>
public ParallelExecutionSettings(int maxDegreeOfParallelism, MemoryAllocator memoryAllocator)
: this(maxDegreeOfParallelism, DefaultMinimumPixelsProcessedPerTask, memoryAllocator)
Expand All @@ -58,6 +64,7 @@ public ParallelExecutionSettings(int maxDegreeOfParallelism, MemoryAllocator mem

/// <summary>
/// Gets the value used for initializing <see cref="ParallelOptions.MaxDegreeOfParallelism"/> when using TPL.
/// A value of <c>-1</c> leaves the degree of parallelism unbounded.
/// </summary>
public int MaxDegreeOfParallelism { get; }

Expand Down
86 changes: 74 additions & 12 deletions src/ImageSharp/Advanced/ParallelRowIterator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ public static void IterateRows<T>(
where T : struct, IRowOperation
{
ValidateRectangle(rectangle);
ValidateSettings(parallelSettings);

int top = rectangle.Top;
int bottom = rectangle.Bottom;
int width = rectangle.Width;
int height = rectangle.Height;

int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask);
int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
int numOfSteps = GetNumberOfSteps(width, height, parallelSettings);

// Avoid TPL overhead in this trivial case:
if (numOfSteps == 1)
Expand All @@ -65,7 +65,7 @@ public static void IterateRows<T>(
}

int verticalStep = DivideCeil(rectangle.Height, numOfSteps);
ParallelOptions parallelOptions = new() { MaxDegreeOfParallelism = numOfSteps };
ParallelOptions parallelOptions = CreateParallelOptions(parallelSettings, numOfSteps);
RowOperationWrapper<T> wrappingOperation = new(top, bottom, verticalStep, in operation);

_ = Parallel.For(
Expand Down Expand Up @@ -109,14 +109,14 @@ public static void IterateRows<T, TBuffer>(
where TBuffer : unmanaged
{
ValidateRectangle(rectangle);
ValidateSettings(parallelSettings);

int top = rectangle.Top;
int bottom = rectangle.Bottom;
int width = rectangle.Width;
int height = rectangle.Height;

int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask);
int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
int numOfSteps = GetNumberOfSteps(width, height, parallelSettings);
MemoryAllocator allocator = parallelSettings.MemoryAllocator;
int bufferLength = Unsafe.AsRef(in operation).GetRequiredBufferLength(rectangle);

Expand All @@ -135,7 +135,7 @@ public static void IterateRows<T, TBuffer>(
}

int verticalStep = DivideCeil(height, numOfSteps);
ParallelOptions parallelOptions = new() { MaxDegreeOfParallelism = numOfSteps };
ParallelOptions parallelOptions = CreateParallelOptions(parallelSettings, numOfSteps);
RowOperationWrapper<T, TBuffer> wrappingOperation = new(top, bottom, verticalStep, bufferLength, allocator, in operation);

_ = Parallel.For(
Expand Down Expand Up @@ -174,14 +174,14 @@ public static void IterateRowIntervals<T>(
where T : struct, IRowIntervalOperation
{
ValidateRectangle(rectangle);
ValidateSettings(parallelSettings);

int top = rectangle.Top;
int bottom = rectangle.Bottom;
int width = rectangle.Width;
int height = rectangle.Height;

int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask);
int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
int numOfSteps = GetNumberOfSteps(width, height, parallelSettings);

// Avoid TPL overhead in this trivial case:
if (numOfSteps == 1)
Expand All @@ -192,7 +192,7 @@ public static void IterateRowIntervals<T>(
}

int verticalStep = DivideCeil(rectangle.Height, numOfSteps);
ParallelOptions parallelOptions = new() { MaxDegreeOfParallelism = numOfSteps };
ParallelOptions parallelOptions = CreateParallelOptions(parallelSettings, numOfSteps);
RowIntervalOperationWrapper<T> wrappingOperation = new(top, bottom, verticalStep, in operation);

_ = Parallel.For(
Expand Down Expand Up @@ -236,14 +236,14 @@ public static void IterateRowIntervals<T, TBuffer>(
where TBuffer : unmanaged
{
ValidateRectangle(rectangle);
ValidateSettings(parallelSettings);

int top = rectangle.Top;
int bottom = rectangle.Bottom;
int width = rectangle.Width;
int height = rectangle.Height;

int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask);
int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
int numOfSteps = GetNumberOfSteps(width, height, parallelSettings);
MemoryAllocator allocator = parallelSettings.MemoryAllocator;
int bufferLength = Unsafe.AsRef(in operation).GetRequiredBufferLength(rectangle);

Expand All @@ -259,7 +259,7 @@ public static void IterateRowIntervals<T, TBuffer>(
}

int verticalStep = DivideCeil(height, numOfSteps);
ParallelOptions parallelOptions = new() { MaxDegreeOfParallelism = numOfSteps };
ParallelOptions parallelOptions = CreateParallelOptions(parallelSettings, numOfSteps);
RowIntervalOperationWrapper<T, TBuffer> wrappingOperation = new(top, bottom, verticalStep, bufferLength, allocator, in operation);

_ = Parallel.For(
Expand All @@ -272,6 +272,37 @@ public static void IterateRowIntervals<T, TBuffer>(
[MethodImpl(InliningOptions.ShortMethod)]
private static int DivideCeil(long dividend, int divisor) => (int)Math.Min(1 + ((dividend - 1) / divisor), int.MaxValue);

/// <summary>
/// Creates the <see cref="ParallelOptions"/> for the current iteration.
/// </summary>
/// <param name="parallelSettings">The execution settings.</param>
/// <param name="numOfSteps">The number of row partitions to execute.</param>
/// <returns>The <see cref="ParallelOptions"/> instance.</returns>
[MethodImpl(InliningOptions.ShortMethod)]
private static ParallelOptions CreateParallelOptions(in ParallelExecutionSettings parallelSettings, int numOfSteps)
=> new() { MaxDegreeOfParallelism = parallelSettings.MaxDegreeOfParallelism == -1 ? -1 : numOfSteps };

/// <summary>
/// Calculates the number of row partitions to execute for the given region.
/// </summary>
/// <param name="width">The width of the region.</param>
/// <param name="height">The height of the region.</param>
/// <param name="parallelSettings">The execution settings.</param>
/// <returns>The number of row partitions to execute.</returns>
[MethodImpl(InliningOptions.ShortMethod)]
private static int GetNumberOfSteps(int width, int height, in ParallelExecutionSettings parallelSettings)
{
int maxSteps = DivideCeil(width * (long)height, parallelSettings.MinimumPixelsProcessedPerTask);

if (parallelSettings.MaxDegreeOfParallelism == -1)
{
// Row batching cannot produce more useful partitions than the number of rows available.
return Math.Min(height, maxSteps);
}
Comment on lines +297 to +301
Copy link
Copy Markdown
Member

@antonfirsov antonfirsov Apr 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@JimBobSquarePants given SixLabors/ImageSharp.Drawing@24af798, I assume you also intend to fix this here?

Edit:

Sorry, never mind, it handles the MaxDegreeOfParallelism == -1 case correctly.


return Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps);
}

private static void ValidateRectangle(Rectangle rectangle)
{
Guard.MustBeGreaterThan(
Expand All @@ -284,4 +315,35 @@ private static void ValidateRectangle(Rectangle rectangle)
0,
$"{nameof(rectangle)}.{nameof(rectangle.Height)}");
}

/// <summary>
/// Validates the supplied <see cref="ParallelExecutionSettings"/>.
/// </summary>
/// <param name="parallelSettings">The execution settings.</param>
/// <exception cref="ArgumentOutOfRangeException">
/// Thrown when <see cref="ParallelExecutionSettings.MaxDegreeOfParallelism"/> or
/// <see cref="ParallelExecutionSettings.MinimumPixelsProcessedPerTask"/> is invalid.
/// </exception>
/// <exception cref="ArgumentNullException">
/// Thrown when <see cref="ParallelExecutionSettings.MemoryAllocator"/> is null.
/// This also guards the public <see cref="ParallelExecutionSettings"/> default value, which bypasses constructor validation.
/// </exception>
private static void ValidateSettings(in ParallelExecutionSettings parallelSettings)
Copy link
Copy Markdown
Member

@antonfirsov antonfirsov Apr 11, 2026

Choose a reason for hiding this comment

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

Given ParallelExecutionSettings is already validated in the constructor and it's a readonly struct, what is the point of re-validating it? If that validation logic is incomplete, wouldn't it make more sense to fix that instead?

Copy link
Copy Markdown
Member Author

@JimBobSquarePants JimBobSquarePants Apr 15, 2026

Choose a reason for hiding this comment

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

We don't actually need it. I had it in my head the struct was in the Configuration class but it's not. I'll remove this in #3113

{
// ParallelExecutionSettings is a public struct, so callers can pass default and bypass constructor validation.
if (parallelSettings.MaxDegreeOfParallelism is 0 or < -1)
{
throw new ArgumentOutOfRangeException(
$"{nameof(parallelSettings)}.{nameof(ParallelExecutionSettings.MaxDegreeOfParallelism)}");
}

Guard.MustBeGreaterThan(
parallelSettings.MinimumPixelsProcessedPerTask,
0,
$"{nameof(parallelSettings)}.{nameof(ParallelExecutionSettings.MinimumPixelsProcessedPerTask)}");

Guard.NotNull(
parallelSettings.MemoryAllocator,
$"{nameof(parallelSettings)}.{nameof(ParallelExecutionSettings.MemoryAllocator)}");
}
}
1 change: 1 addition & 0 deletions src/ImageSharp/Configuration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public Configuration(params IImageFormatConfigurationModule[] configurationModul
/// <summary>
/// Gets or sets the maximum number of concurrent tasks enabled in ImageSharp algorithms
/// configured with this <see cref="Configuration"/> instance.
/// Set to <c>-1</c> to leave the degree of parallelism unbounded.
/// Initialized with <see cref="Environment.ProcessorCount"/> by default.
/// </summary>
public int MaxDegreeOfParallelism
Expand Down
115 changes: 115 additions & 0 deletions tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace SixLabors.ImageSharp.Tests.Helpers;

public class ParallelRowIteratorTests
{
public delegate void BufferedRowAction<T>(int y, Span<T> span);
public delegate void RowIntervalAction<T>(RowInterval rows, Span<T> span);

private readonly ITestOutputHelper output;
Expand Down Expand Up @@ -200,6 +201,47 @@ void RowAction(RowInterval rows, Span<Vector4> buffer)
Assert.Equal(expectedData, actualData);
}

[Fact]
public void IterateRows_MaxDegreeOfParallelismMinusOne_ShouldVisitAllRows()
{
ParallelExecutionSettings parallelSettings = new(
-1,
10,
Configuration.Default.MemoryAllocator);

Rectangle rectangle = new(0, 0, 10, 10);
int[] actualData = new int[rectangle.Height];

void RowAction(int y) => actualData[y]++;

TestRowActionOperation operation = new(RowAction);

ParallelRowIterator.IterateRows(
rectangle,
in parallelSettings,
in operation);

Assert.Equal(Enumerable.Repeat(1, rectangle.Height), actualData);
}

[Fact]
public void IterateRowsWithTempBuffer_DefaultSettingsRequireInitialization()
{
ParallelExecutionSettings parallelSettings = default;
Rectangle rect = new(0, 0, 10, 10);

void RowAction(int y, Span<Rgba32> memory)
{
}

TestRowOperation<Rgba32> operation = new(RowAction);

ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(
() => ParallelRowIterator.IterateRows<TestRowOperation<Rgba32>, Rgba32>(rect, in parallelSettings, in operation));

Assert.Contains(nameof(ParallelExecutionSettings.MaxDegreeOfParallelism), ex.Message);
}

public static TheoryData<int, int, int, int, int, int, int> IterateRows_WithEffectiveMinimumPixelsLimit_Data =
new()
{
Expand Down Expand Up @@ -296,6 +338,53 @@ void RowAction(RowInterval rows, Span<Vector4> buffer)
Assert.Equal(expectedNumberOfSteps, actualNumberOfSteps);
}

[Fact]
public void IterateRowIntervalsWithTempBuffer_MaxDegreeOfParallelismMinusOne_ShouldVisitAllRows()
{
ParallelExecutionSettings parallelSettings = new(
-1,
10,
Configuration.Default.MemoryAllocator);

Rectangle rectangle = new(0, 0, 10, 10);
int[] actualData = new int[rectangle.Height];

void RowAction(RowInterval rows, Span<Vector4> buffer)
{
for (int y = rows.Min; y < rows.Max; y++)
{
actualData[y]++;
}
}

TestRowIntervalOperation<Vector4> operation = new(RowAction);

ParallelRowIterator.IterateRowIntervals<TestRowIntervalOperation<Vector4>, Vector4>(
rectangle,
in parallelSettings,
in operation);

Assert.Equal(Enumerable.Repeat(1, rectangle.Height), actualData);
}

[Fact]
public void IterateRows_DefaultSettingsRequireInitialization()
{
ParallelExecutionSettings parallelSettings = default;
Rectangle rect = new(0, 0, 10, 10);

void RowAction(int y)
{
}

TestRowActionOperation operation = new(RowAction);

ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(
() => ParallelRowIterator.IterateRows(rect, in parallelSettings, in operation));

Assert.Contains(nameof(ParallelExecutionSettings.MaxDegreeOfParallelism), ex.Message);
}

public static readonly TheoryData<int, int, int, int, int, int, int> IterateRectangularBuffer_Data =
new()
{
Expand Down Expand Up @@ -445,6 +534,32 @@ public void Invoke(int y)
}
}

private readonly struct TestRowActionOperation : IRowOperation
{
private readonly Action<int> action;

public TestRowActionOperation(Action<int> action)
=> this.action = action;

public void Invoke(int y)
=> this.action(y);
}

private readonly struct TestRowOperation<TBuffer> : IRowOperation<TBuffer>
where TBuffer : unmanaged
{
private readonly BufferedRowAction<TBuffer> action;

public TestRowOperation(BufferedRowAction<TBuffer> action)
=> this.action = action;

public int GetRequiredBufferLength(Rectangle bounds)
=> bounds.Width;

public void Invoke(int y, Span<TBuffer> span)
=> this.action(y, span);
}

private readonly struct TestRowIntervalOperation : IRowIntervalOperation
{
private readonly Action<RowInterval> action;
Expand Down
Loading