chmduquesne / rollinghash

Implementation of some rolling hashes in go
MIT License
64 stars 11 forks source link

Speed up adler32.Write #1

Closed calmh closed 7 years ago

calmh commented 7 years ago

Write() used to allocate both a full copy of the passed data and a new vanilla hasher. This removes both allocations in the common case and increases throughput by about 36% as a result. It makes this adler32.Write() comparable in performance to the vanilla hash/adler32.Write().

        benchmark              old ns/op     new ns/op     delta
        BenchmarkWriteKB-8     647           476           -26.43%

        benchmark              old MB/s     new MB/s     speedup
        BenchmarkWriteKB-8     1582.46      2148.24      1.36x

        benchmark              old allocs     new allocs     delta
        BenchmarkWriteKB-8     2              0              -100.00%

        benchmark              old bytes     new bytes     delta
        BenchmarkWriteKB-8     1028          0             -100.00%
coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.2%) to 66.667% when pulling 22d4b298007de8b5100c935d044c50e437b873d9 on kastelo:reducealloc into 88b86a92826991b14d01fb43456909fcb8a76b8b on chmduquesne:master.

chmduquesne commented 7 years ago

Thanks for the awesome PR! Merged.

chmduquesne commented 7 years ago

I did not realize that this lib was a dependency of syncthing!

You might want to pull it as

gopkg.in/chmduquesne/rollinghash.v2/adler32

instead of

github.com/chmduquesne/rollinghash/adler32

This repo respects semantic versioning, so you'll be protected against unexpected changes in the interface.

calmh commented 7 years ago

Thanks. I don't especially like gopkg.in and we use vendoring so we'll stick to the original import path as long as we are allowed to. :)

chmduquesne commented 7 years ago

No worries, feel free to handle your dependencies how you like :-). As much as I find gopkg.in convenient, I certainly don't want to force anyone to use it.