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

Speed up adler32.Write #1

Merged
merged 2 commits into from
Jan 18, 2017
Merged

Speed up adler32.Write #1

merged 2 commits into from
Jan 18, 2017

Conversation

calmh
Copy link
Contributor

@calmh calmh commented Jan 18, 2017

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%

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
Copy link

Coverage Status

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

@chmduquesne chmduquesne merged commit 001f237 into chmduquesne:master Jan 18, 2017
@chmduquesne
Copy link
Owner

Thanks for the awesome PR! Merged.

@chmduquesne
Copy link
Owner

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
Copy link
Contributor Author

calmh commented Jan 18, 2017

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
Copy link
Owner

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.

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.

None yet

3 participants