make sort32 fast#327
Conversation
| /// Two‑pass radix sort (base 2¹⁶) of 32‑bit float bit‑patterns, | ||
| /// descending order (largest keys first). Mirrors the JS `sort32Splats`. | ||
| #[inline(always)] | ||
| unsafe fn prefix_sum_exclusive(buckets: &mut [u32]) -> u32 { |
There was a problem hiding this comment.
Is there a specific reason this is marked unsafe? It compiles just fine without.
There was a problem hiding this comment.
i had many experiments with simd, which didn't make it marginally faster so i removed it for simplicity sake but forgot to remove the unsafe, will clean it up
|
Awesome work, gave it a try and can confirm that it improves sorting performance. In my limited testing I saw ~20% reduction in sorting time (~25% faster).
Without this change the performance gain seems to be roughly the same, or at least I didn't observe any significant difference. The majority of the benefit seems to come from making it branchless. |
|
@mrxz i squeezed a bit more performance ~<=1ms by removing more branches from hot loops, and what you noticed seems about right, it will differ from one wasm engine to another, and arch to another(specially cache sizes and arch) so it's hard to give a solid number but it'll still be a pump in performance |
8c1efb4 to
77253d1
Compare
|
can you remove the changes in the dist directory? |
|
@39ali great work! This looks like a cool win indeed, thanks for the work. Could remove the build in the |
|
Could the macros used for unrolling the loops also be used for the body of remainder loops? Both should be identical, so if we could avoid the duplication we avoid the risk of it ever getting out of sync. |
|
I'll implement the changes |
…the essential optimizations. Removed second branchless optimization. Added comments on why `unsafe` accesses are okay.
|
@39ali really great work here! I've done some benchmarking on your method, and I'm actually getting 2x - 4x speedups in sorting from this. I'm truly shocked that this was possible! This will have a great impact on Spark's sorting performance. On a 10M splat scene on my M3 it goes from 250 ms to 60 ms or so. It's possible that the speedup is not as great on other environments, such as @mrxz was reporting 25% speedups on his system. I went through and carefully separated the optimizations and measured them:
It did seem like there was one error though: the second branchless loop seems problematic... I think writing to the array and only advancing the pointer if it's "valid" could overwrite things. So I removed it. I don't think it does very much for the performance anyway. Finally I reverted some unnecessary changes to make it closer to @mrxz 's original formulation. I think we should merge this in @dmarcos , @mrxz ! WDYT? This should really help with #225 . Interestingly, because the sorting is so much faster, it sort of exposes the next bottleneck more: uploading the ordering frequently to the GPU can cause stuttering sometimes when the counts get large. Now this happens more often! |
There is bound to be some variability between setups, but that's quite the discrepancy between the measurements. My guess would be that besides the gains of being branchless and more cache friendly it somehow avoids a slow-path on the M3? Regardless, it's a net positive, even the comparatively modest speed-up I've measured is a very welcome improvement.
The overwriting was intentional AFAICT. Since the 'invalid' entries aren't tallied you don't want to count them when scattering either. The corollary is that by only advancing after writing a valid entry, you guarantee that the final write to the array at a given position is a valid entry. The only way this goes wrong is once a bucket has been fully scattered, as then it'll point into the start of the next bucket. The inverted That said, I do think the explicit check is more readable and some quick testing doesn't show a meaningful performance difference on my end between the two.
I don't see any reason not to merge this. Regarding #225 it will most definitely reduce the time a stale ordering is visible on screen. Whether or not this will be enough is the question. Similar to the pesky FOUC in web-design, no matter how short it will remain an "issue". Not sure what mkkellogg did differently, but it at the very least it degenerated more gracefully. It does have logic to detect large camera changes and queues partial sorts, though even before a sort update arrives, the rendered frame somehow looks less bad (subjectively). |
|
Thanks everyone. Good stuff! |
try to improve the performance of sort32, on avg it's 30-40% faster .
things that changed :
pass 2 no longer re-reads
keys[],scratchstores a packed u64 of(inverted_key << 32 | original_index). pass 2 reads the high 16 bits directly from scratch withkv >> 48making it a sequential scanhistogram and scatter are now branchless to help llvm vectorize the loop
manually unrolled histogram and both scatter passes to 8-wide