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

performance regression after extending statement scope guard #14590

Closed
avikivity opened this issue Jul 9, 2023 · 4 comments
Closed

performance regression after extending statement scope guard #14590

avikivity opened this issue Jul 9, 2023 · 4 comments
Assignees
Labels
P1 Urgent status/regression symptom/performance Issues causing performance problems
Milestone

Comments

@avikivity
Copy link
Member

Bad commit: c42a91e

perf-simple-query --smp 1

before:

216489.88 tps ( 61.1 allocs/op, 13.1 tasks/op, 43558 insns/op, 0 errors)
217708.69 tps ( 61.1 allocs/op, 13.1 tasks/op, 43542 insns/op, 0 errors)
219495.02 tps ( 61.1 allocs/op, 13.1 tasks/op, 43538 insns/op, 0 errors)
216863.84 tps ( 61.1 allocs/op, 13.1 tasks/op, 43567 insns/op, 0 errors)
218936.48 tps ( 61.1 allocs/op, 13.1 tasks/op, 43546 insns/op, 0 errors)

after:

201773.52 tps ( 63.1 allocs/op, 15.1 tasks/op, 44600 insns/op, 0 errors)
210875.48 tps ( 63.1 allocs/op, 15.1 tasks/op, 44558 insns/op, 0 errors)
210186.55 tps ( 63.1 allocs/op, 15.1 tasks/op, 44588 insns/op, 0 errors)
211021.76 tps ( 63.1 allocs/op, 15.1 tasks/op, 44569 insns/op, 0 errors)
208597.52 tps ( 63.1 allocs/op, 15.1 tasks/op, 44587 insns/op, 0 errors)

Two extra allocations, two extra tasks, 1k extra instructions, for something that is DDL only.

@avikivity avikivity added the P1 Urgent label Jul 9, 2023
@mykaul mykaul added status/regression symptom/performance Issues causing performance problems labels Jul 10, 2023
@kbr-scylla
Copy link
Contributor

I'll revert the patch.

@kbr-scylla
Copy link
Contributor

cc @gleb-cloudius

@gleb-cloudius
Copy link
Contributor

Two extra allocations, two extra tasks, 1k extra instructions, for something that is DDL only.

I'll investigate where two tasks are coming from, but the claim that it is DDL only is incorrect. We plan to move truncate and auth to group0 as well. Non of them are DDL but will require the same treatment.

@DoronArazii DoronArazii added this to the 5.4 milestone Aug 29, 2023
@avikivity
Copy link
Member Author

No vulnerable branches, not backporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Urgent status/regression symptom/performance Issues causing performance problems
Projects
None yet
Development

No branches or pull requests

7 participants