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: reduce allocs in execStmtInOpenState [~5% allocs] #135908

Open
tbg opened this issue Nov 21, 2024 · 0 comments
Open

sql: reduce allocs in execStmtInOpenState [~5% allocs] #135908

tbg opened this issue Nov 21, 2024 · 0 comments
Assignees
Labels
branch-master Failures and bugs on the master branch. C-performance Perf of queries or internals. Solution not expected to change functional behavior. o-perf-efficiency Related to performance efficiency P-1 Issues/test failures with a fix SLA of 1 month T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) T-sql-queries SQL Queries Team

Comments

@tbg
Copy link
Member

tbg commented Nov 21, 2024

Under sysbench oltp_read_only this shows up very prominently, as one of the top hitters in peek:

Type: alloc_objects
Time: 2024-11-21 13:25:41 CET
Duration: 30.17s, Total samples = 158593784 
Showing nodes accounting for 158593784, 100% of 158593784 total
----------------------------------------------------------+-------------
      flat  flat%   sum%        cum   cum%   calls calls% + context 	 	 
----------------------------------------------------------+-------------
                                           2031648   100% |   github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt.func1 pkg/sql/conn_executor_exec.go:141
   2031648  1.28%  9.75%    2031648  1.28%                | github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState pkg/sql/conn_executor_exec.go:275
----------------------------------------------------------+-------------
image

I'm not actually sure which one is right (does this function allocate 2031648 times or 5072675 times which is 2.5x of the former?) but it's clear that it allocates all over the place. Summing up the "Total:" lines in the listing1 gives 5807416 which is yet another number, but it's close to the latter. So, we think this is around 2.5*1.28% or around 5%! Much of that seems avoidable at first glance and someone should spend some quality time with this code.

Even its three return parameters leak to the heap (because they are named and get passed to all kinds of functions inside of execStmtInOpenState). You can see this here alongside all the other allocations.

I was so confused by this that I used scripts/goescape which also told me that the ctx arg gets moved to the heap:

./conn_executor_exec.go:276:2: moved to heap: ctx
./conn_executor_exec.go:282:4: moved to heap: retEv
./conn_executor_exec.go:282:21: moved to heap: retPayload
./conn_executor_exec.go:282:50: moved to heap: retErr

presumably because ctx is leaked into the portal here.

Epic: CRDB-42584

Jira issue: CRDB-44787

Footnotes

  1. $ grep 'Total' src.txt | awk '{ print $2 }' | grep -E '[0-9]+' | perl -lne '$x += $_; END { print $x; }' 5807416

@tbg tbg added C-performance Perf of queries or internals. Solution not expected to change functional behavior. branch-master Failures and bugs on the master branch. P-1 Issues/test failures with a fix SLA of 1 month o-perf-efficiency Related to performance efficiency T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Nov 21, 2024
@mgartner mgartner added the T-sql-queries SQL Queries Team label Nov 21, 2024
@github-project-automation github-project-automation bot moved this to Triage in SQL Queries Nov 21, 2024
@mgartner mgartner moved this from Triage to Active in SQL Queries Nov 21, 2024
@mgartner mgartner self-assigned this Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures and bugs on the master branch. C-performance Perf of queries or internals. Solution not expected to change functional behavior. o-perf-efficiency Related to performance efficiency P-1 Issues/test failures with a fix SLA of 1 month T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) T-sql-queries SQL Queries Team
Projects
Status: Active
Development

No branches or pull requests

2 participants