sparse_strips: Bump fearless_simd - #1853
Conversation
| @@ -789,11 +786,8 @@ impl Tiles { | |||
| let target_row = &mut self.windings.partial[y_idx as usize]; | |||
| let current = f32x4::from_slice(s, target_row); | |||
|
|
|||
| let double_count = if crosses_top { | |||
| f_dir_v | |||
| } else { | |||
| f32x4::splat(s, 0.0) | |||
| }; | |||
| let double_count = mask32x4::splat(s, crosses_top) | |||
| .select(f_dir_v, f32x4::splat(s, 0.0)); | |||
There was a problem hiding this comment.
On AVX2 wit the old code I got a regression of around 25% with new fearless_simd, because the branchless selects were turned into branches. These two changes fix this.
There was a problem hiding this comment.
You can also use std's select_unpredictable here to keep the logic in the scalar domain. I don't know if that will give you any performance benefits or not.
There was a problem hiding this comment.
Hmm, I think I'll keep it for now since I've already benchmarked this, but thanks for pointing it out!
| - name: cargo test --doc | ||
| run: cargo test --doc --workspace --locked --all-features --no-fail-fast | ||
|
|
||
| test-avx512: |
There was a problem hiding this comment.
I’m concerned this adds more CI time by rerunning nearly the entire test suite for relatively uncommon AVX-512 hardware. Could we keep SSE2 coverage but run AVX-512 only when SIMD-related code changes, or in a nightly/weekly scheduled workflow instead of on every PR?
There was a problem hiding this comment.
This is not nearly as bad as it appears. CI time for this job is under 3 minutes; it does not make CI take longer overall.
And it's not as uncommon as you might expect. Steam hardware survey puts AVX-512 availability at 23%, which is far ahead of both SSE4.2-only (2%) and SSE2-only (1%) systems in their data.
There was a problem hiding this comment.
I would agree with @Shnatsel here, it definitely doesn't impact CI run times since it's not even close to being the bottleneck, and AVX-512 is not that uncommon anymore, especially on server hardware, which is also relevant for our internal use cases. The only downside is that it makes the CI file uglier. So unless you are strongly opposed, I think it would be better to keep this.
There was a problem hiding this comment.
Beyond runner time, this brings the workflow to 21 jobs while Linebender’s Free plan allows 20 concurrent GitHub-hosted jobs. The AVX-512 job took ~4 minutes and finished well before the slowest job, so it likely won’t materially increase total CI duration, but it can still occupy a scarce slot and cause queueing when the organization is busy.
Anyway, I’m not blocking this — just expressing concern about the gradual increase in CI infra usage, which I think we’ll need to address in the near future.
There was a problem hiding this comment.
SSE4.2-only (2%) and SSE2-only (1%)
Just to clarify, what do you mean by “SSE4.2-only (2%) and SSE2-only (1%)” here? The Steam survey seems to report support for each instruction set independently rather than mutually exclusive “only” buckets.
There was a problem hiding this comment.
I subtracted the AVX2 percentage from the SSE4.2 to get "SSE4.2 only". Same for "SSE2 only": SSE2 - SSE4.2.
There was a problem hiding this comment.
Interesting, didn't know about the 20 concurrent jobs limit, thanks for pointing it out!
| crossbeam-channel = "0.5.15" | ||
| ordered-channel = { version = "1.2.0", features = ["crossbeam-channel"] } | ||
| fearless_simd = { version = "0.4.0", default-features = false } | ||
| fearless_simd = { version = "0.7.0", default-features = false } |
There was a problem hiding this comment.
Could we add browser wasm support to the benchmark harness? The web is an important target for us, and this would give us much better insight into its performance; once supported, it would also be useful to rerun the benchmarks for this fearless_simd bump to identify any improvements/regressions.
0ebec80 to
eab3e0a
Compare
|
Added a make-shift WASM harness: laurenz-canva/vello_bench2@master...wasm_bench Here are the results on my Mac:
I also tried running on an older MacBook on BrowserStack as well as a phone, but the results are just way too noisy to draw any real conclusions, but the results were usually withint +/- 10%. But I haven't noticed any real big regressions that were consistently reproducible. |
grebmeg
left a comment
grebmeg
left a comment
There was a problem hiding this comment.
I also tried running on an older MacBook on BrowserStack as well as a phone, but the results are just way too noisy to draw any real conclusions, but the results were usually withint +/- 10%.
But I haven't noticed any real big regressions that were consistently reproducible.
Nice! LGTM! Thanks for doing this! As a follow-up, could we productionize your branch and add it here?
| - name: cargo test --doc | ||
| run: cargo test --doc --workspace --locked --all-features --no-fail-fast | ||
|
|
||
| test-avx512: |
There was a problem hiding this comment.
Beyond runner time, this brings the workflow to 21 jobs while Linebender’s Free plan allows 20 concurrent GitHub-hosted jobs. The AVX-512 job took ~4 minutes and finished well before the slowest job, so it likely won’t materially increase total CI duration, but it can still occupy a scarce slot and cause queueing when the organization is busy.
Anyway, I’m not blocking this — just expressing concern about the gradual increase in CI infra usage, which I think we’ll need to address in the near future.
|
Will see what I can do. But as mentioned, unforutnately it's not straightforward because because our criterion-based setup doesn't really work in WASM. But I also don't want to duplicate all benchmark methods just for WASM! |
3 participants
This PR bumps fearless_simd to version 0.7, which unfortunately contains quite a few changes. Supersedes #1802.
Neon
Benchmarks are mostly neutral. There are two slight regressions: one for translucent unpremultiplication (where it seems like the new code results in different lowering from LLVM) and
fill/transparent_short_u8_neon, where slightly different inlining decisions seem to be made. Unfortunately, there doesn't seem to be an easy fix for either, but they are so small that I don't think it's a problem. The other ones are mostly noise.AVX 2
Some improvements + regressions, but overall also within the noise threshold.
WASM
I didn't run extensive benchmarks but I've tried rendering GhostScript tiger on my potato tablet, and the frame rate seems unchanged across the two commits.