Skip to content

Replace parallel row iteration with sequential loops in Crop/Convolution Processors#3113

Open
JimBobSquarePants wants to merge 3 commits intomainfrom
js/parallel-cleanup
Open

Replace parallel row iteration with sequential loops in Crop/Convolution Processors#3113
JimBobSquarePants wants to merge 3 commits intomainfrom
js/parallel-cleanup

Conversation

@JimBobSquarePants
Copy link
Copy Markdown
Member

@JimBobSquarePants JimBobSquarePants commented Apr 10, 2026

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

See #3111 for relevant details.

Changes to how convolution and crop operations are parallelized. The main focus is on improving performance by removing unnecessary parallelization in memory-bandwidth-bound operations, which can actually degrade performance due to cache contention.

Additionally, it updates the gamma correction logic in the Bokeh blur processor for improved performance. Bokeh and Median blur have been left parallel as they contain significant per-task work.

Relevant reference images have also been updated to reflect minor changes in the output due to earlier clamping.

Copy link
Copy Markdown
Member

@antonfirsov antonfirsov left a comment

Choose a reason for hiding this comment

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

I think this should get rid of the operation types, they only scatter the code, which doesn't help readability.


for (int y = interest.Top; y < interest.Bottom; y++)
{
operation.Invoke(y, span);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any reason for Convolution2DRowOperation isolation? IMO removing it/moving it to a private method would simplify code.

in operation);
for (int y = bounds.Top; y < bounds.Bottom; y++)
{
operation.Invoke(y);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The operation type is clearly unneeded here.

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