Skip to content
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.

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

Simple RVV implementation of adler32. #55

Closed
wants to merge 1 commit into from

Conversation

sh1boot
Copy link

@sh1boot sh1boot commented Apr 7, 2024

This implementation works in short 22-iteration batches, which avoids overflowing 16-bit counters so we get better parallelism in the inner loop.

This implementation works in short 22-iteration batches, which avoids
overflowing 16-bit counters so we get better parallelism in the inner
loop.
aarongable pushed a commit to chromium/chromium that referenced this pull request Apr 9, 2024
Replace SiFive code for an alternative checksum implementation
that works in short 22-iteration batches thus avoiding overflowing
16-bit counters.

As a result, it has better parallelism in the inner loop, yielding
a +20% faster checksum speed on a K230 board.

The average *decompression* gain while using the zlib wrapper for
the snappy data corpus was +2.15%, but with near +4% for HTML.

Patch by Simon Hosie, from:
cloudflare/zlib#55

Bug: 329282661
Change-Id: I72e2ce9bb9b3d8626dedb33cf026f1af9b9b4a33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5433273
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Adenilson Cavalcanti <cavalcantii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1284684}
hubot pushed a commit to aosp-mirror/platform_external_zlib that referenced this pull request Apr 9, 2024
Replace SiFive code for an alternative checksum implementation
that works in short 22-iteration batches thus avoiding overflowing
16-bit counters.

As a result, it has better parallelism in the inner loop, yielding
a +20% faster checksum speed on a K230 board.

The average *decompression* gain while using the zlib wrapper for
the snappy data corpus was +2.15%, but with near +4% for HTML.

Patch by Simon Hosie, from:
cloudflare/zlib#55

Bug: 329282661
Change-Id: I72e2ce9bb9b3d8626dedb33cf026f1af9b9b4a33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5433273
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Adenilson Cavalcanti <cavalcantii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1284684}
NOKEYCHECK=True
GitOrigin-RevId: f68eb88e6ac1139355bad9d1f1eff784e9e82afb
@kornelski
Copy link

Unfortunately we're not using RISC-V, so this is hard to review and test, and we can't commit to maintaining it.

@sh1boot
Copy link
Author

sh1boot commented Apr 12, 2024

Yeah, that makes sense.

The original reasoning for choosing the Cloudflare fork as the basis for RISC-V work was that it was the healthiest-looking version in GitHub and I didn't want to start another fork. But that was before I understood how active Google was with Android and Chrome on RISC-V, so that's probably the best place to work from.

@sh1boot sh1boot closed this Apr 12, 2024
@sh1boot
Copy link
Author

sh1boot commented Apr 12, 2024

Will focus risc-v patches upstream in chromium and aosp (as per the "referenced by" links already added here)

@RJVB
Copy link

RJVB commented Apr 12, 2024

The original reasoning for choosing the Cloudflare fork as the basis for RISC-V work was that it was the healthiest-looking version in GitHub and I didn't want to start another fork.

You were aware of zlib-ng?
https://github.com/zlib-ng/zlib-ng/tree/develop/arch/riscv

I think it's safe to say that zlib-ng is a more forward-looking, future-proof alternative with, as far as I can tell, perfect ABI compatibility if configured for that (-DZLIB_COMPAT=ON). Maximum speed was always a secondary goal for them, but in my latest comparison of the heads of both repos on X86 their performance was equal and maybe even a tad better. (Maybe because based in zlib 1.3.1?)

@kornelski
Copy link

kornelski commented Apr 12, 2024

I wouldn’t mind getting such patches backported from the upstream zlib or zlib-ng, because then hopefully they would be maintaining these features, and it’s better for us to have fewer differences between zlib forks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants