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

make newFirstChunk more effective #9396

Closed
hicqu opened this issue Feb 21, 2019 · 7 comments
Closed

make newFirstChunk more effective #9396

hicqu opened this issue Feb 21, 2019 · 7 comments
Assignees
Labels
sig/execution SIG execution type/enhancement The issue or PR belongs to an enhancement. type/performance

Comments

@hicqu
Copy link
Contributor

hicqu commented Feb 21, 2019

General Question

Here is a call graph about TiDB's CPU usage in attached file.
profile001

We can see newFirstChunk utilizes about 10% CPU of total.

I have tried to use a sync.Pool to cache *columns to improve it, but the result is not better. With the patch sync.Pool itself becomes the new bottle neck.

So please try to find a good wait to cache *columns to improve it.

@winoros
Copy link
Member

winoros commented Feb 21, 2019

What's your workload?
@zz-jason What about testing column pool in this environment?

@zz-jason
Copy link
Member

There are many ways to implement a column pool. For example, sync.Pool, lock free list, channel based resource reuse.

  • sync.Pool: the put and get operations is very fast, but the content in that pool is freed when golang GC is triggered.
  • lock free list, channel: the put and get operations is slow compared with sync.Pool, but the content in the pool can be control by ourself.

There is still some problem to solve in the column pool implementation, I think we can leave this optimization in the future. For now, let's focused on chunk size control to improve the query latency.

@zz-jason
Copy link
Member

There are some pool implementations and the benchmark results in this gist: https://gist.github.com/zz-jason/c98d942dddbb32c134c9cadffd983f61

This is a simple lock free list implementation: https://github.com/zz-jason/lockfree, the benchmark can be gained by running BenchmarkPushPop()

@hicqu
Copy link
Contributor Author

hicqu commented Feb 21, 2019

I have tested sync.Pool but the result is bad. I prefer to use some lockfree collections or a channel, but I'm blocked on 1 thing: we must release *column manual and return it back to the pool. Could you help me? @zz-jason

@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented Feb 22, 2019

I'm blocked on 1 thing: we must release *column manual and return it back to the pool.

Can you describe more on this part?

@SunRunAway
Copy link
Contributor

We can give sync.Pool a try again using go1.13beta. See golang/go#22950

@SunRunAway SunRunAway added type/enhancement The issue or PR belongs to an enhancement. and removed type/enhancement The issue or PR belongs to an enhancement. type/performance labels Jul 18, 2019
@SunRunAway
Copy link
Contributor

I used sysbench oltp_read_only to test on the master branch with go1.13beta1, it seems newFirstChunk is not a hot path. I'll close this issue and if someone finds another performance problem with newFirstChunk, feel free to reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution type/enhancement The issue or PR belongs to an enhancement. type/performance
Projects
None yet
6 participants