Skip to content

Conversation

@valyala
Copy link
Contributor

@valyala valyala commented Mar 21, 2018

Go allocates a copy of slice headers when they are passed to C functions.
See golang/go#24450 for details.

Avoid this by passing unsafe.Pointer as C.uintptr_t into C functions.

joshcarter and others added 30 commits September 6, 2016 16:32
- Removing all .c/.h files from distribution. This will link against
  the shared library instead.

- Updates to error code list. NOTE: the Go wrapper should really not
  be using its own error code map, as the upstream has no qualms about
  changing their error map at any time.

- Updated Decompress() so that it uses zStd to guess the destintion
  buffer size. If zStd doesn't provide an estimate (returns 0) then
  the previous fallback code still operates as it did before.
This takes the c code from commit
cbc5225d38ba65a1cd77701e91a9f45f3cc825ba available at
facebook/zstd@cbc5225
3cc825ba

The error list has been updated to reflect the C code
We will silence ZBUFF deprecation warnings at least for this version
release. Moving to the new interface will need to use C pointers
instead of Go pointers
Update zstd to 1.1.3 for 1.x branch (keeps zbuff interface)
According to https://docs.travis-ci.com/user/languages/go/
> Go builds are not available on the OS X environment.

But on OSX page:
https://docs.travis-ci.com/user/reference/osx/#Languages
> go 1.9.1

Worth a try
Reduce memory allocations in Reader
[travis] Add Go 1.10 to list of tested go versions
- Errors were defined in private C/header files and we needed to change them every release (FIXME: this is very fragile, must map 1-to-1 with zstd's)
- We actually just need DstSizeTooSmall error which we now have a check for and a test
TestFindIsDstSizeTooSmallError now check that there is ONE AND ONLY ONE zstd error that corresponds to DstSizeTooSmall error to make sure the check is not too leniant (before just tested we had at least one)
Add comments to exported variables and methods
Viq111 and others added 9 commits March 14, 2018 12:02
[zstd][errors] Get rid of static error code translation and use C call
We have this function in Go code to not have to do a cgo call (really consuming). This function in a hot loop, that's why we can't call C code every time.

This mirrors latest zstd implementation (they switched to 128kB window)
Go allocates a copy of slice headers when they are passed to C functions.
See golang/go#24450 for details.

Avoid this by passing unsafe.Pointer as C.uintptr_t into C functions.

Benchmark results:

$ PAYLOAD=ZSTD_LICENSE go test -run=111 -bench='k[CD]' -benchmem -count=10

name             old time/op    new time/op    delta
Compression-4      21.2µs ± 5%    20.8µs ± 6%      ~     (p=0.190 n=10+10)
Decompression-4    7.37µs ± 5%    6.93µs ± 6%    -5.95%  (p=0.001 n=10+10)

name             old speed      new speed      delta
Compression-4    62.5MB/s ± 5%  63.7MB/s ± 6%      ~     (p=0.190 n=10+10)
Decompression-4   180MB/s ± 5%   191MB/s ± 6%    +6.35%  (p=0.001 n=10+10)

name             old alloc/op   new alloc/op   delta
Compression-4       96.0B ± 0%      0.0B       -100.00%  (p=0.000 n=10+10)
Decompression-4     96.0B ± 0%      0.0B       -100.00%  (p=0.000 n=10+10)

name             old allocs/op  new allocs/op  delta
Compression-4        4.00 ± 0%      0.00       -100.00%  (p=0.000 n=10+10)
Decompression-4      4.00 ± 0%      0.00       -100.00%  (p=0.000 n=10+10)
Update vendored zstd c lib to 1.3.4
@Viq111
Copy link
Collaborator

Viq111 commented Mar 30, 2018

The master is not used on this repository currently (will update it to mirror 1.x default branch once we want to fork to 2.x)
This PR will be integrated into the default 1.x with #31

@Viq111 Viq111 closed this Mar 30, 2018
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.

6 participants