Skip to content

sparse_strips: Better inlining of SIMD kernels for improved x86 performance#1688

Merged
LaurenzV merged 7 commits into
mainfrom
laurenz/inline
May 30, 2026
Merged

sparse_strips: Better inlining of SIMD kernels for improved x86 performance#1688
LaurenzV merged 7 commits into
mainfrom
laurenz/inline

Conversation

@LaurenzV

@LaurenzV LaurenzV commented May 30, 2026

Copy link
Copy Markdown
Collaborator

This is an alternative to #1678, while trying to be a bit more surgical about where to add inlines and where not. I measured this on my x86 machine, and similarly got 85%-95% better performance for blending. Image rendering of high quality also improved by 85%. Other stuff mostly unchanged.

#1678 applied vectorization in the benchmark functions as well, but this should not be needed because we just call fill and any method inside of that will have a corresponding vectorize call, so that should be enough. This way, we also don't have to inline it.

Commit 1 just applies inline to three kernel methods (although it seems like they mostly already where inlined).

Commit 2 adds vectorize methods to the new methods of the painter structs. I decided to choose this route instead of force inlining and forcing the parent to do the vectorization because those methods are only called very few times anyway, so this seems like the better route. The compiler can then decide itself whether to inline or not.

Commit 3 makes sure that the next methods of all iterators are inlined. In this case, it is the responsibility of the parent to make sure vectorization is in place. We want these to be inlined because those will be called for each pixel chunk, so it's important to not have function call overhead here.

Commit 4 Applies fixes to make sure target features are properly propagated for blending.

Commit 5 applies some more small fixes that I missed before.

There is obviously a bit of bikeshedding you could do about in which cases you want the method to contain vectorize and in which cases you force inline it and have the parent be responsible for providing the vectorization, but I think the current state should be good for now.

@LaurenzV LaurenzV requested a review from Shnatsel May 30, 2026 10:59

@Shnatsel Shnatsel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good by an large.

Could you also add comments on functions that have neither vectorize() nor inline(always) that this is deliberate and that the callee contains a call to vectorize()?

Here's a list of remaining functions without annotations:

encode.rs (line 781): u8_lut, f32_lut
multi_threaded.rs (line 361): rasterize_with
single_threaded.rs (line 129): rasterize_with, rasterize_with_filters, process_layer_tile, rasterize_simple, composite_at_offset_with

single_threaded can probably use one comment instead of annotating every function.

@LaurenzV LaurenzV enabled auto-merge May 30, 2026 12:05
@LaurenzV LaurenzV added this pull request to the merge queue May 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 30, 2026
@LaurenzV LaurenzV added this pull request to the merge queue May 30, 2026
Merged via the queue into main with commit 35a4279 May 30, 2026
19 checks passed
@LaurenzV LaurenzV deleted the laurenz/inline branch May 30, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants