-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
106155: sql/sqlstats: avoid heap allocations when using stats iterators r=maryliag a=nvanbenschoten This PR contains two changes that each avoid per-transaction heap allocations in the `sqlstats` package. ### sql/sqlstats: return iterators by value from constructors This commit switches to returning sqlstats iterations by value from their constructors, instead of by pointer. This avoids causing the iterators to escape and needing to allocate on the heap. Instead, they can be kept on the stack, avoiding heap allocations in methods like `IterateStatementStats`. ```sh ➜ benchdiff --run='BenchmarkKV/./SQL/rows=1$$' --count=25 ./pkg/sql/tests name old time/op new time/op delta KV/Insert/SQL/rows=1-10 173µs ±13% 172µs ±15% ~ (p=0.939 n=25+25) KV/Update/SQL/rows=1-10 285µs ±16% 280µs ±11% ~ (p=0.356 n=25+23) KV/Delete/SQL/rows=1-10 212µs ±25% 221µs ±19% ~ (p=0.345 n=25+25) KV/Scan/SQL/rows=1-10 127µs ±11% 127µs ±11% ~ (p=0.878 n=25+24) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 32.2kB ± 1% 32.0kB ± 1% -0.36% (p=0.006 n=24+24) KV/Insert/SQL/rows=1-10 59.0kB ± 1% 58.9kB ± 1% ~ (p=0.140 n=25+25) KV/Update/SQL/rows=1-10 70.6kB ± 1% 70.4kB ± 1% ~ (p=0.050 n=24+24) KV/Delete/SQL/rows=1-10 84.9kB ± 1% 84.9kB ± 1% ~ (p=0.859 n=25+25) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 368 ± 1% 366 ± 1% -0.62% (p=0.001 n=24+24) KV/Update/SQL/rows=1-10 726 ± 1% 723 ± 2% -0.45% (p=0.013 n=23+24) KV/Insert/SQL/rows=1-10 492 ± 2% 490 ± 1% -0.41% (p=0.044 n=25+25) KV/Delete/SQL/rows=1-10 613 ± 3% 613 ± 3% ~ (p=0.179 n=25+25) ``` ### sql/sqlstats: pass `sqlstats.IteratorOptions` by value, not pointer This commit switches from passing the `sqlstats.IteratorOptions` around by pointer to passing it by value. The struct is 2 bytes large (8 when padded), so there's little benefit to passing it around by pointer. Meanwhile, passing the object by pointer through interface methods prevents escape analysis from keeping it on the stack, forcing a heap allocation. ``` ➜ benchdiff --run='BenchmarkKV/./SQL/rows=1$$' --count=25 ./pkg/sql/tests name old time/op new time/op delta KV/Insert/SQL/rows=1-10 169µs ±13% 168µs ±17% ~ (p=0.603 n=25+25) KV/Update/SQL/rows=1-10 282µs ± 9% 282µs ± 8% ~ (p=0.788 n=25+25) KV/Delete/SQL/rows=1-10 227µs ±26% 206µs ±27% ~ (p=0.074 n=25+25) KV/Scan/SQL/rows=1-10 126µs ±12% 127µs ±16% ~ (p=0.908 n=25+25) name old alloc/op new alloc/op delta KV/Delete/SQL/rows=1-10 84.9kB ± 1% 84.5kB ± 1% -0.50% (p=0.008 n=25+21) KV/Insert/SQL/rows=1-10 58.9kB ± 1% 58.7kB ± 1% -0.23% (p=0.020 n=25+23) KV/Update/SQL/rows=1-10 70.5kB ± 1% 70.5kB ± 1% ~ (p=0.894 n=24+24) KV/Scan/SQL/rows=1-10 32.0kB ± 1% 32.0kB ± 1% ~ (p=0.631 n=25+24) name old allocs/op new allocs/op delta KV/Delete/SQL/rows=1-10 613 ± 3% 604 ± 1% -1.52% (p=0.000 n=25+21) KV/Insert/SQL/rows=1-10 489 ± 1% 486 ± 1% -0.69% (p=0.001 n=25+23) KV/Scan/SQL/rows=1-10 365 ± 1% 363 ± 1% -0.42% (p=0.024 n=25+25) KV/Update/SQL/rows=1-10 724 ± 2% 722 ± 2% ~ (p=0.302 n=25+24) ``` Epic: None Release note: None 106157: sql: add tests for CTAS, CMVAS with every vtable r=chengxiong-ruan a=ecwall Informs #105895 Adds tests for CREATE TABLE AS, CREATE MATERIALIZED VIEW AS sourcing from all vtables. Release note: None Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Evan Wall <[email protected]>
- Loading branch information
Showing
22 changed files
with
227 additions
and
88 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| // Copyright 2023 The Cockroach Authors. | ||
| // | ||
| // Use of this software is governed by the Business Source License | ||
| // included in the file licenses/BSL.txt. | ||
| // | ||
| // As of the Change Date specified in that file, in accordance with | ||
| // the Business Source License, use of this software will be governed | ||
| // by the Apache License, Version 2.0, included in the file | ||
| // licenses/APL.txt. | ||
|
|
||
| package sql | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/base" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/parser" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/types" | ||
| "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" | ||
| "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" | ||
| "github.com/cockroachdb/cockroach/pkg/util/leaktest" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestCreateAsVTable verifies that all vtables can be used as the source of | ||
| // CREATE TABLE AS and CREATE MATERIALIZED VIEW AS. | ||
| func TestCreateAsVTable(t *testing.T) { | ||
| defer leaktest.AfterTest(t)() | ||
|
|
||
| // These are the vtables that need to be fixed. | ||
| // The map should be empty if all vtables are supported. | ||
| brokenTables := map[string]struct{}{ | ||
| // TODO(sql-foundations): Fix nil pointer dereference. | ||
| // See https://github.com/cockroachdb/cockroach/issues/106166. | ||
| `pg_catalog.pg_prepared_statements`: {}, | ||
| // TODO(sql-foundations): Fix nil pointer dereference. | ||
| // See https://github.com/cockroachdb/cockroach/issues/106167. | ||
| `pg_catalog.pg_cursors`: {}, | ||
| // TODO(sql-foundations): Fix nil pointer dereference. | ||
| // See https://github.com/cockroachdb/cockroach/issues/106168. | ||
| `"".crdb_internal.create_statements`: {}, | ||
| } | ||
|
|
||
| ctx := context.Background() | ||
| testCluster := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{}) | ||
| defer testCluster.Stopper().Stop(ctx) | ||
| sqlRunner := sqlutils.MakeSQLRunner(testCluster.ServerConn(0)) | ||
| var p parser.Parser | ||
|
|
||
| i := 0 | ||
| for _, vSchema := range virtualSchemas { | ||
| for _, vSchemaDef := range vSchema.tableDefs { | ||
| if vSchemaDef.isUnimplemented() { | ||
| continue | ||
| } | ||
|
|
||
| var name tree.TableName | ||
| var ctasColumns []string | ||
| schema := vSchemaDef.getSchema() | ||
| statements, err := p.Parse(schema) | ||
| require.NoErrorf(t, err, schema) | ||
| require.Lenf(t, statements, 1, schema) | ||
| switch stmt := statements[0].AST.(type) { | ||
| case *tree.CreateTable: | ||
| name = stmt.Table | ||
| for _, def := range stmt.Defs { | ||
| if colDef, ok := def.(*tree.ColumnTableDef); ok { | ||
| if colDef.Hidden { | ||
| continue | ||
| } | ||
| // Filter out vector columns to prevent error in CTAS: | ||
| // "VECTOR column types are unsupported". | ||
| if colDef.Type == types.Int2Vector || colDef.Type == types.OidVector { | ||
| continue | ||
| } | ||
| ctasColumns = append(ctasColumns, colDef.Name.String()) | ||
| } | ||
| } | ||
| case *tree.CreateView: | ||
| name = stmt.Name | ||
| ctasColumns = []string{"*"} | ||
| default: | ||
| require.Failf(t, "missing case", "unexpected type %T for schema %s", stmt, schema) | ||
| } | ||
|
|
||
| fqName := name.FQString() | ||
| if _, ok := brokenTables[fqName]; ok { | ||
| continue | ||
| } | ||
|
|
||
| // Filter by trace_id to prevent error when selecting from | ||
| // crdb_internal.cluster_inflight_traces: | ||
| // "pq: a trace_id value needs to be specified". | ||
| var where string | ||
| if fqName == `"".crdb_internal.cluster_inflight_traces` { | ||
| where = " WHERE trace_id = 1" | ||
| } | ||
|
|
||
| createTableStmt := fmt.Sprintf( | ||
| "CREATE TABLE test_table_%d AS SELECT %s FROM %s%s", | ||
| i, strings.Join(ctasColumns, ", "), fqName, where, | ||
| ) | ||
| sqlRunner.Exec(t, createTableStmt) | ||
| createViewStmt := fmt.Sprintf( | ||
| "CREATE MATERIALIZED VIEW test_view_%d AS SELECT * FROM %s%s", | ||
| i, fqName, where, | ||
| ) | ||
| sqlRunner.Exec(t, createViewStmt) | ||
| i++ | ||
| } | ||
| } | ||
|
|
||
| waitForJobsSuccess(t, sqlRunner) | ||
| } | ||
|
|
||
| func waitForJobsSuccess(t *testing.T, sqlRunner *sqlutils.SQLRunner) { | ||
| query := `SELECT job_id, status, error, description | ||
| FROM [SHOW JOBS] | ||
| WHERE job_type IN ('SCHEMA CHANGE', 'NEW SCHEMA CHANGE') | ||
| AND status != 'succeeded'` | ||
| sqlRunner.CheckQueryResultsRetry(t, query, [][]string{}) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.