Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Cross compilation fails due to gozstd #55

Closed
rolandjitsu opened this issue Jan 8, 2020 · 14 comments
Closed

Cross compilation fails due to gozstd #55

rolandjitsu opened this issue Jan 8, 2020 · 14 comments

Comments

@rolandjitsu
Copy link

rolandjitsu commented Jan 8, 2020

When I try to cross-compile a project that includes this lib I get the following errors:

# github.com/valyala/gozstd
../../go/pkg/mod/github.com/valyala/gozstd@v1.6.2/stream.go:31:59: undefined: CDict
../../go/pkg/mod/github.com/valyala/gozstd@v1.6.2/stream.go:35:64: undefined: CDict
../../go/pkg/mod/github.com/valyala/gozstd@v1.6.2/stream.go:47:20: undefined: Writer

I build using the following:

env GOOS=linux GOARCH=arm go build ./cmd/api/main.go

I do this on a macOS 10.15.2.

If I build on the mac (no cross-compilation) it works without errors.

@rolandjitsu rolandjitsu changed the title Build fails due to gozstd Cross compilation fails due to gozstd Jan 8, 2020
@rolandjitsu
Copy link
Author

Why not use the native Go compress/gzip or some other native Go lib?

@vmihailenco
Copy link
Owner

gzip is many times slower by design. And https://github.com/klauspost/compress/tree/master/zstd was at least 2 times slower when I last tried it...

The only way forward I see atm is to hide zstd under some build tag.

@rolandjitsu
Copy link
Author

gzip is many times slower by design. And https://github.com/klauspost/compress/tree/master/zstd was at least 2 times slower when I last tried it...

The only way forward I see atm is to hide zstd under some build tag.

Is speed that important when we're talking about async tasks? I'd say that cross-platform compatibility is probably better to have than faster compression 🤔 Especially if the diff between gzip and gozstd is in the order of microseconds.

But I guess that build tags might also work if you want to give ppl the option to pick one compression lib over the other.

@klauspost
Copy link

klauspost commented Jan 10, 2020

And https://github.com/klauspost/compress/tree/master/zstd was at least 2 times slower when I last tried it...

It has to my knowledge never been 2 times slower, so I am interested in seeing your actual benchmarks.

Also, small blocks got a nice speedup recently.

@vmihailenco
Copy link
Owner

@klauspost thanks for looking into this. Your work on compression is truly amazing 👍

I have never spent much time on benchmarks, but I've quickly re-created something simple and got this result

BenchmarkZstd-12      	   11724	    103757 ns/op
BenchmarkGoZstd-12    	     168	   6675344 ns/op

Probably I will get better results by reusing encoder & decoder...

@vmihailenco
Copy link
Owner

PS This is variant that reuses encoder and decoder - timing is not much better...

@klauspost
Copy link

https://gist.github.com/klauspost/7f15d32dd6c78d5ce7d67f0353958e8a

BenchmarkZstd-12    	   21874	     55899 ns/op	 135.08 MB/s	   30720 B/op	       5 allocs/op
BenchmarkGoZstd-12    	   11325	    105766 ns/op	  71.39 MB/s	    2080 B/op	       0 allocs/op

Writing the output from your encoder and decompressing with zstd doesn't work, so I cannot tell what you are storing, if it has CRC, etc. Obviously not adding CRC is faster.

@klauspost
Copy link

klauspost commented Jan 10, 2020

Never draw conclusions on a single sample, and never with a bad benchmark.

Just as an example, benchmark 'compressing' random data:

BenchmarkZstd-12    	   77143	     14803 ns/op	 510.10 MB/s	   30720 B/op	       5 allocs/op
BenchmarkGoZstd-12    	  122734	      9134 ns/op	 826.70 MB/s	     188 B/op	       0 allocs/op

And you should probably look into why your encoder isn't outputting zstandard compatible output.

@vmihailenco
Copy link
Owner

vmihailenco commented Jan 10, 2020

why your encoder isn't outputting zstandard compatible output

Hm, there is nothing mine in the benchmark I've posted... One benchmark uses C zstd encoder and another one uses Encoder from your package...

Writing the output from your encoder and decompressing with zstd doesn't work, so I cannot tell what you are storing, if it has CRC, et

Again, what do you mean by my encoder? I am using zstd.NewWriter to write to a buffer and I don't see what is wrong about that... I don't know anything about CRC also, but I guess there is streaming and non-streaming mode and they are different in some aspects. That is just a guess - I am not sure where I am supposed to learn about that and if that is needed to properly use zstd.

PS github.com/valyala/gozstd is a C wrapper around original zstd...

@klauspost
Copy link

Well, the output it returns is not zstandard compatible.

You can add this to your encode when you have flushed:

		ioutil.WriteFile("blah.zst", buf.Bytes(), os.ModePerm)
		os.Exit(0)

If you use the zstd commandline tool to decompress you get:

λ zstd -d blah.zst
blah.zst             : 0 MB...     blah.zst : Read error (39) : premature end

@klauspost
Copy link

klauspost commented Jan 10, 2020

Your last block is not marked as the last block:

next block: Steam Size: 3020, Type: blockTypeCompressed, Last: false, Window: 2097152

@vmihailenco
Copy link
Owner

Okay, now "Writing the output from your encoder and decompressing with zstd doesn't work" makes sense. TBH this stuff is beyond me - I've just picked first C wrapper available. I am not sure what is the right thing here (may be omitting CRC is fine?), but I see that it is significantly faster than pure Go variant in some cases. No doubt that compressing random data is important (skipping already compressed bytes), but it would be nice to have comparable speed in other cases too.

Anyway all this stuff is beyond me - I just wanted to share my naive benchmark. Hopefully it is not altogether wrong.

@klauspost
Copy link

TBH this stuff is beyond me

Just pointing out that what your wrapper is writing is not zstandard compatible.

nice to have comparable speed

Sure, but you do pay a price for memory safety, debug-ability and cross compilation. Setting that as a bar will make it impossible to reach. If speed is important, use zstd.WithEncoderLevel(zstd.SpeedFastest), you lose a few bytes of compression, but gain about 25% speed.

Hopefully it is not altogether wrong.

Well, I would call it misleading and a sample set of one data type is not something you can use for conclusions.

@vmihailenco
Copy link
Owner

Closed by 07ac935

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants