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

zstd: If first block and 'final', encode direct. #251

Merged
merged 2 commits into from
Mar 27, 2020

Conversation

klauspost
Copy link
Owner

If if writing header (ie first block) and it is the final block, use block compression instead of async.

Addition for #248

If if writing header (ie first block) and it is the final block, use block compression instead of async.

Addition for #248
s.current = s.current[:0]
s.filling = s.filling[:0]
return nil
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not. This just hides the problem if I read this correctly.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not hiding any problems, but handles small < 1 block streaming encodes.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I understand. The benchmark will be faster until some threshold and then it will degrade to previous levels. So from my POV it does not change much and just gives false hopes :)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you are doing multiple blocks you will get the benefit of the concurrency. There will off course be a small dropoff as you get just above 1 block, but overall speed will be faster.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But when we have enough for 1 block we don't know how much more will come, so we start compressing the first block.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but overall speed will be faster

Just want to repeat here as well that in case of HTTP server I don't want code to be faster by using more threads and CPU - those threads and CPU can be used to serve other requests/users. Otherwise I just make 1 request faster by delaying all the other requests.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I respectfully disagree, but will also repeat that this will likely be the case when full parallel compression.

First of all Go has very advanced preemption and handles this nicely.

Secondly you are just postponing the problem. If you have a 2 CPU system, the problem will occur as soon as you have 3 users and so on.

If you just make compression slower by using 1 of 2 cores, it will take (in theory) 2x as long, meaning you are strained for resources 2x as long instead of just getting it done faster AND give a better user experience when you are not limited for resources.

Assuming things scale linearly, you only can do xMB/s on your system and there is no reason to slow down every transaction, since the system is likely to behave the same when you reach that total limit anyway, ie. splitting the resources between the users.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that parallel version has some significant overhead. So instead of 100% CPU it uses 130% CPU and the 2nd user now has only 70% of the 2nd core.

What is even worse is that parallel version is slower on small payloads - not faster. I even tried to compress ~300mb binary and got this result:

BenchmarkCZstd-12      	       1	1758534769 ns/op	 160.09 MB/s	         0.239 compression	1288323136 B/op	      38 allocs/op
BenchmarkGoZstd-12     	       1	3465210251 ns/op	  81.24 MB/s	         0.244 compression	1339945080 B/op	     680 allocs/op
BenchmarkGoZstd2-12    	       1	2187101331 ns/op	 128.72 MB/s	         0.244 compression	757003880 B/op	     186 allocs/op
BenchmarkS2-12         	       2	 746165990 ns/op	 377.29 MB/s	         0.363 compression	1275335332 B/op	    1597 allocs/op

EncodeAll version is still significantly faster...

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You must be doing something strange. Here is a bench that doesn't allocate (as much): https://gist.github.com/klauspost/446e9ab7aeae0b75d7974339b65df815

λ go test -bench=. -benchtime=10s
BenchmarkCZstd-32          13430            889105 ns/op         253.44 MB/s          25 B/op          0 allocs/op
BenchmarkGoZstd-32          7183           1545090 ns/op         145.84 MB/s      251130 B/op          1 allocs/op
BenchmarkGoZstd2-32         5437           2187727 ns/op         103.00 MB/s      112531 B/op          0 allocs/op
BenchmarkS2-32             22752            528732 ns/op         426.19 MB/s         975 B/op          8 allocs/op

@klauspost klauspost merged commit e8d1c04 into master Mar 27, 2020
@klauspost klauspost deleted the zstd-small-blocks-encode-directly branch March 27, 2020 12:09
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.

2 participants