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

sql: initial batch sizes too large with no row estimate hint #62524

Closed
erikgrinaker opened this issue Mar 24, 2021 · 0 comments · Fixed by #62534
Closed

sql: initial batch sizes too large with no row estimate hint #62524

erikgrinaker opened this issue Mar 24, 2021 · 0 comments · Fixed by #62534
Assignees
Labels
A-sql-execution Relating to SQL execution. C-performance Perf of queries or internals. Solution not expected to change functional behavior.

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Mar 24, 2021

While reviewing #62282, kv95 benchmarks were shown to regress somewhat. This still holds after landing on master, with e.g. kv95/enc=false/nodes=1/cpu=32 giving 66304 ops/s before the patch and 58862 ops/s after the patch. When reverting to using 1 as the minimum capacity instead of estimatedRowCount in the ResetMaybeReallocate call the performance goes back to the baseline:

rf.typs, rf.machine.batch, estimatedRowCount, rf.memoryLimit,

While debugging, it was found that estimatedRowCount would often be 0, which would cause ResetMaybeReallocate to allocate a batch of coldata.BatchSize() (i.e. 1024) every time, instead of 1 which was the old behavior. This probably explains the performance drop.

} else if minCapacity == 0 || minCapacity > coldata.BatchSize() {
minCapacity = coldata.BatchSize()
}

@erikgrinaker erikgrinaker added C-performance Perf of queries or internals. Solution not expected to change functional behavior. A-sql-execution Relating to SQL execution. labels Mar 24, 2021
@erikgrinaker erikgrinaker self-assigned this Mar 24, 2021
@craig craig bot closed this as completed in 8aace6d Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-execution Relating to SQL execution. C-performance Perf of queries or internals. Solution not expected to change functional behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant