Skip to content

Commit 378f52f

Browse files
craig[bot]michae2mgartnertbgyuzefovich
committed
147554: builtins: add more builtins to DistsqlBlocklist r=yuzefovich a=michae2 While adding a builtin for a test, I noticed that a neighboring builtin that accessed `evalctx.Planner` was not marked with `DistsqlBlocklist`. Then I noticed that many more functions accessing Planner were not marked with `DistsqlBlocklist`. I tried to ignore it, but I could not. Hence this yak-shave. Epic: None Release note: None 147720: bench/tpcc: use 3-node cluster, refactor, and create cleaner profiles r=mgartner a=mgartner #### bench/tpcc: use 3-node cluster BenchmarkTPCC now uses a 3-node cluster instead of a single-node cluster. Starting-up a 3-node cluster takes several seconds, so the benchmark now only does this once for all implementations (literal or optimized) and mixes, rather than once per implemenation/mix combination. Also, we no longer generate a reusable store directory. Instead, we use in-memory stores and initialize the TPC-C workload data each time the benchmark is run. This provides a simpler user experience and the benchmark runs just as quickly as reusing pre-initialized stores. Release note: None #### bench/tpcc: set cluster settings Release note: None #### bench/tpcc: move client code to same process Moving the workload client to the same process vastly simplifies the benchmark. Release note: None #### bench/tpcc: collect CPU and memory profile that exclude setup CPU and heap profiles can be collected while running the benchmarks by passing the "-profile" flag with a prefix for the profile file names. The profile names will include the benchmark names, e.g., `foo_tpcc_literal_new_order.cpu` and `foo_tpcc_literal_new_order.mem` will be created when running the benchmark: ./dev bench pkg/bench/tpcc -f BenchmarkTPCC/literal/new_order \ --test-args '-test.benchtime=30s -profile=foo' These profiles will omit CPU samples and allocations made during the setup phase of the benchmark, unlike profiles collected with the `-test.cpuprofile` and `-test.memprofile` flags. Release note: None #### tpcc: disable stdout in benchmark The workload plumbing is cli-centric, but here we're using tpcc as a library. While invoking it in this way doesn't trigger any workload-level logging (behind the `--display-format` flag), tpcc has its own logging during data loading. This commit makes it possible to override this logging, and does so in BenchmarkTPCC. In an ideal world, there would be configuration options for this sort of thing, but this would be a large and painful refactor. #### bench/tpcc: refactor and disable local client optimization Release note: None #### bench/tpcc: round-robin queries to all three nodes Release note: None #### bench/tpcc: use import data loader Using the import data loader instead of the insert data loader speeds up the setup time for the benchmark significantly. Release note: None #### bench/tpcc: run each mix on its own cluster Each run of a benchmark is now run in its own cluster to make each run independent of each other and avoid skewing results. Release note: None #### bench/tpcc: remove --profile flag The `--profile` flag is no longer supported. Instead, CPU and heap profiles will be collected if `-test.cpuprofile` or `-test.memprofile` are provided. The benchmark will "hijack" the standard profiles and create new profiles that omit CPU samples and allocations made during the setup phase of the benchmark. The profile names will include the prefix of the profile flags and the benchmark names. For example, the profiles "foo_tpcc_literal_new_order.cpu" and "foo_tpcc_literal_new_order.mem" will be created when running the benchmark: ./dev bench pkg/bench/tpcc -f BenchmarkTPCC/literal/new_order \ --test-args '-test.cpuprofile=foo.cpu -test.memprofile=foo.mem' Note that the "foo.cpu" file will not be created because we must stop the global CPU profiler in order to collect profiles that omit setup samples. The "foo.mem" file will created and include all allocations made during the entire duration of the benchmark. Release note: None #### bench/benchprof: add benchprof package The benchprof package has been created to provide utilities for creating CPU and memory profiles in benchmarks. Release note: None #### bench/benchprof: support mutex profiles Release note: None 148209: sql/tests: speed-up sysbench micro-benchmarks r=mgartner a=mgartner The sysbench micro-benchmarks have been sped up by avoiding repeatedly setting up a new cluster while Go's benchmarking harness ramps up `b.N`. Prior to this commit the benchmark command below took ~112 seconds and now takes ~36 seconds on my Mac M1 Pro. ./dev bench pkg/sql/tests \ -f 'BenchmarkSysbench/SQL/1node_local/oltp_point_select' \ --count 3 See the inline comments for more details. Release note: None 148237: tree: avoid creating new DString in AdjustValueToType when possible r=yuzefovich a=yuzefovich Previously, in `AdjustValueToType` we would always create a new `DString` object if the original datum wasn't modified in any way. This seems unnecessary, so we now track whether a modification might have occurred and allocate the new object only in that case, otherwise returning the original datum. We choose to not do the same for collated strings since there it wouldn't have been an equivalent change (we currently initialize a new CollationEnvironment). Noticed this inefficiency when looking at CPU profile of a node running an IMPORT. ``` name old time/op new time/op delta ImportFixture/tpcc/warehouses=1-24 586ms ± 9% 585ms ± 4% ~ (p=0.853 n=10+10) name old speed new speed delta ImportFixture/tpcc/warehouses=1-24 148MB/s ±10% 148MB/s ± 5% ~ (p=0.912 n=10+10) name old alloc/op new alloc/op delta ImportFixture/tpcc/warehouses=1-24 1.13GB ± 1% 1.10GB ± 1% -2.76% (p=0.000 n=10+10) name old allocs/op new allocs/op delta ImportFixture/tpcc/warehouses=1-24 6.08M ± 0% 4.12M ± 0% -32.25% (p=0.000 n=10+10) ``` Epic: None Release note: None Co-authored-by: Michael Erickson <[email protected]> Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Tobias Grieger <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]>
5 parents 8138e3f + 3e870df + bee7014 + 8cd8fbd + 03f9a84 commit 378f52f

File tree

15 files changed

+650
-445
lines changed

15 files changed

+650
-445
lines changed

pkg/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,7 @@ GO_TARGETS = [
858858
"//pkg/base/serverident:serverident",
859859
"//pkg/base:base",
860860
"//pkg/base:base_test",
861+
"//pkg/bench/benchprof:benchprof",
861862
"//pkg/bench/cmd/pgbenchsetup:pgbenchsetup",
862863
"//pkg/bench/cmd/pgbenchsetup:pgbenchsetup_lib",
863864
"//pkg/bench/hashbench:hashbench_test",

pkg/bench/benchprof/BUILD.bazel

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "benchprof",
5+
srcs = ["benchprof.go"],
6+
importpath = "github.com/cockroachdb/cockroach/pkg/bench/benchprof",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//pkg/testutils/sniffarg",
10+
"@com_github_google_pprof//profile",
11+
],
12+
)

pkg/bench/benchprof/benchprof.go

Lines changed: 296 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,296 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package benchprof
7+
8+
import (
9+
"bytes"
10+
"os"
11+
"path/filepath"
12+
"regexp"
13+
"runtime"
14+
runtimepprof "runtime/pprof"
15+
"strings"
16+
"testing"
17+
18+
"github.com/cockroachdb/cockroach/pkg/testutils/sniffarg"
19+
"github.com/google/pprof/profile"
20+
)
21+
22+
// StopFn is a function that stops profiling.
23+
type StopFn func(testing.TB)
24+
25+
// Stop stops profiling.
26+
func (f StopFn) Stop(tb testing.TB) {
27+
if f != nil {
28+
f(tb)
29+
}
30+
}
31+
32+
// StartAllProfiles starts collection of CPU, heap, and mutex profiles, if the
33+
// "-test.cpuprofile", "-test.memprofile", or "-test.mutexprofile" flags are
34+
// set. See StartCPUProfile, StartMemProfile, and StartMutexProfile for more
35+
// details.
36+
//
37+
// Example usage:
38+
//
39+
// func BenchmarkFoo(b *testing.B) {
40+
// // ..
41+
// b.Run("case", func(b *testing.B) {
42+
// defer benchprof.StartAllProfiles(b).Stop(b)
43+
// // Benchmark loop.
44+
// })
45+
// }
46+
func StartAllProfiles(tb testing.TB) StopFn {
47+
cpuStop := StartCPUProfile(tb)
48+
memStop := StartMemProfile(tb)
49+
mutexStop := StartMutexProfile(tb)
50+
return func(b testing.TB) {
51+
cpuStop(b)
52+
memStop(b)
53+
mutexStop(b)
54+
}
55+
}
56+
57+
// StartCPUProfile starts collection of a CPU profile if the "-test.cpuprofile"
58+
// flag has been set. The profile will omit CPU samples collected prior to
59+
// calling StartCPUProfile, and include all allocations made until the returned
60+
// StopFn is called.
61+
//
62+
// Example usage:
63+
//
64+
// func BenchmarkFoo(b *testing.B) {
65+
// // ..
66+
// b.Run("case", func(b *testing.B) {
67+
// defer benchprof.StartCPUProfile(b).Stop(b)
68+
// // Benchmark loop.
69+
// })
70+
// }
71+
//
72+
// The file name of the profile will include the prefix of the profile flags and
73+
// the benchmark names. For example, "foo_benchmark_thing_1.cpu" would be
74+
// created for a "BenchmarkThing/1" benchmark if the "-test.cpuprofile=foo.cpu"
75+
// flag is set.
76+
//
77+
// NB: The "foo.cpu" file will not be created because we must stop the global
78+
// CPU profiler in order to collect profiles that omit setup samples.
79+
func StartCPUProfile(tb testing.TB) StopFn {
80+
var cpuProfFile string
81+
if err := sniffarg.DoEnv("test.cpuprofile", &cpuProfFile); err != nil {
82+
tb.Fatal(err)
83+
}
84+
if cpuProfFile == "" {
85+
// Not CPU profile requested.
86+
return nil
87+
}
88+
89+
prefix := profilePrefix(cpuProfFile)
90+
dir := outputDir(tb)
91+
if dir != "" {
92+
cpuProfFile = filepath.Join(dir, cpuProfFile)
93+
}
94+
95+
// Hijack the harness's profile to make a clean profile.
96+
// The flag is set, so likely a CPU profile started by the Go harness is
97+
// running (unless -count is specified, but StopCPUProfile is idempotent).
98+
runtimepprof.StopCPUProfile()
99+
100+
// Remove the harness's profile file. It would not be an accurate profile.
101+
_ = os.Remove(cpuProfFile)
102+
103+
// Create a new profile file.
104+
fileName := profileFileName(tb, dir, prefix, "cpu")
105+
f, err := os.OpenFile(fileName, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
106+
if err != nil {
107+
tb.Fatal(err)
108+
}
109+
110+
// Start profiling.
111+
if err := runtimepprof.StartCPUProfile(f); err != nil {
112+
tb.Fatal(err)
113+
}
114+
115+
return func(b testing.TB) {
116+
runtimepprof.StopCPUProfile()
117+
if err := f.Close(); err != nil {
118+
b.Fatal(err)
119+
}
120+
}
121+
}
122+
123+
// StartMemProfile starts collection of a heap profile if the "-test.memprofile"
124+
// flag has been set. The profile will omit memory allocations prior to calling
125+
// StartMemProfile, and include all allocations made until the returned StopFn
126+
// is called.
127+
//
128+
// Example usage:
129+
//
130+
// func BenchmarkFoo(b *testing.B) {
131+
// // ..
132+
// b.Run("case", func(b *testing.B) {
133+
// defer benchprof.StartMemProfile(b).Stop(b)
134+
// // Benchmark loop.
135+
// })
136+
// }
137+
//
138+
// The file name of the profile will include the prefix of the profile flags and
139+
// the benchmark names. For example, "foo_benchmark_thing_1.mem" would be
140+
// created for a "BenchmarkThing/1" benchmark if the "-test.memprofile=foo.mem"
141+
// flag is set.
142+
//
143+
// NB: The "foo.mem" file will still be created and include all allocations made
144+
// during the entire duration of the benchmark.
145+
func StartMemProfile(tb testing.TB) StopFn {
146+
var memProfFile string
147+
if err := sniffarg.DoEnv("test.memprofile", &memProfFile); err != nil {
148+
tb.Fatal(err)
149+
}
150+
if memProfFile == "" {
151+
// No heap profile requested.
152+
return nil
153+
}
154+
155+
prefix := profilePrefix(memProfFile)
156+
dir := outputDir(tb)
157+
if dir != "" {
158+
memProfFile = filepath.Join(dir, memProfFile)
159+
}
160+
161+
// Create a new profile file.
162+
fileName := profileFileName(tb, dir, prefix, "mem")
163+
diffAllocs := diffProfile(func() []byte {
164+
p := runtimepprof.Lookup("allocs")
165+
var buf bytes.Buffer
166+
runtime.GC()
167+
if err := p.WriteTo(&buf, 0); err != nil {
168+
tb.Fatal(err)
169+
}
170+
return buf.Bytes()
171+
})
172+
173+
return func(b testing.TB) {
174+
if sl := diffAllocs(b); len(sl) > 0 {
175+
if err := os.WriteFile(fileName, sl, 0644); err != nil {
176+
b.Fatal(err)
177+
}
178+
}
179+
}
180+
}
181+
182+
// StartMutexProfile starts collection of a mutex profile if the
183+
// "-test.cpuprofile" flag has been set. The profile will only collect data
184+
// between calling StartMutexProfile and calling the returned StopFn.
185+
//
186+
// Example usage:
187+
//
188+
// func BenchmarkFoo(b *testing.B) {
189+
// // ..
190+
// b.Run("case", func(b *testing.B) {
191+
// defer benchprof.StartMutexProfile(b).Stop(b)
192+
// // Benchmark loop.
193+
// })
194+
// }
195+
//
196+
// The file name of the profile will include the prefix of the profile flags and
197+
// the benchmark names. For example, "foo_benchmark_thing_1.mutex" would be
198+
// created for a "BenchmarkThing/1" benchmark if the
199+
// "-test.mutexprofile=foo.mutex" flag is set.
200+
func StartMutexProfile(tb testing.TB) StopFn {
201+
var mutexProfFile string
202+
if err := sniffarg.DoEnv("test.mutexprofile", &mutexProfFile); err != nil {
203+
tb.Fatal(err)
204+
}
205+
if mutexProfFile == "" {
206+
// No mutex profile requested.
207+
return nil
208+
}
209+
210+
prefix := profilePrefix(mutexProfFile)
211+
dir := outputDir(tb)
212+
if dir != "" {
213+
mutexProfFile = filepath.Join(dir, mutexProfFile)
214+
}
215+
216+
// Create a new profile file.
217+
fileName := profileFileName(tb, dir, prefix, "mutex")
218+
diffAllocs := diffProfile(func() []byte {
219+
p := runtimepprof.Lookup("mutex")
220+
var buf bytes.Buffer
221+
if err := p.WriteTo(&buf, 0); err != nil {
222+
tb.Fatal(err)
223+
}
224+
return buf.Bytes()
225+
})
226+
227+
return func(b testing.TB) {
228+
if sl := diffAllocs(b); len(sl) > 0 {
229+
if err := os.WriteFile(fileName, sl, 0644); err != nil {
230+
b.Fatal(err)
231+
}
232+
}
233+
}
234+
}
235+
236+
func diffProfile(take func() []byte) func(testing.TB) []byte {
237+
// The below is essentially cribbed from pprof.go in net/http/pprof.
238+
239+
baseBytes := take()
240+
if baseBytes == nil {
241+
return func(tb testing.TB) []byte { return nil }
242+
}
243+
return func(b testing.TB) []byte {
244+
newBytes := take()
245+
pBase, err := profile.ParseData(baseBytes)
246+
if err != nil {
247+
b.Fatal(err)
248+
}
249+
pNew, err := profile.ParseData(newBytes)
250+
if err != nil {
251+
b.Fatal(err)
252+
}
253+
pBase.Scale(-1)
254+
pMerged, err := profile.Merge([]*profile.Profile{pBase, pNew})
255+
if err != nil {
256+
b.Fatal(err)
257+
}
258+
pMerged.TimeNanos = pNew.TimeNanos
259+
pMerged.DurationNanos = pNew.TimeNanos - pBase.TimeNanos
260+
261+
buf := bytes.Buffer{}
262+
if err := pMerged.Write(&buf); err != nil {
263+
b.Fatal(err)
264+
}
265+
return buf.Bytes()
266+
}
267+
}
268+
269+
func outputDir(tb testing.TB) string {
270+
var dir string
271+
if err := sniffarg.DoEnv("test.outputdir", &dir); err != nil {
272+
tb.Fatal(err)
273+
}
274+
return dir
275+
}
276+
277+
func profilePrefix(profileArg string) string {
278+
i := strings.Index(profileArg, ".")
279+
if i == -1 {
280+
return profileArg
281+
}
282+
return profileArg[:i]
283+
}
284+
285+
func profileFileName(tb testing.TB, outputDir, prefix, suffix string) string {
286+
saniRE := regexp.MustCompile(`\W+`)
287+
testName := strings.TrimPrefix(tb.Name(), "Benchmark")
288+
testName = strings.ToLower(testName)
289+
testName = saniRE.ReplaceAllString(testName, "_")
290+
291+
fileName := prefix + "_" + testName + "." + suffix
292+
if outputDir != "" {
293+
fileName = filepath.Join(outputDir, fileName)
294+
}
295+
return fileName
296+
}

pkg/bench/tpcc/BUILD.bazel

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,35 +4,28 @@ go_test(
44
name = "tpcc_test",
55
srcs = [
66
"bench_test.go",
7-
"internal_test.go",
87
"main_test.go",
9-
"utils_test.go",
108
],
119
data = glob(["testdata/**"]),
1210
deps = [
1311
"//pkg/base",
12+
"//pkg/bench/benchprof",
13+
"//pkg/ccl/workloadccl",
14+
"//pkg/rpc/nodedialer",
1415
"//pkg/security/securityassets",
1516
"//pkg/security/securitytest",
1617
"//pkg/server",
17-
"//pkg/storage",
18-
"//pkg/testutils/pgurlutils",
18+
"//pkg/settings/cluster",
19+
"//pkg/sql/stats",
1920
"//pkg/testutils/serverutils",
20-
"//pkg/testutils/skip",
2121
"//pkg/testutils/sqlutils",
2222
"//pkg/testutils/testcluster",
23-
"//pkg/testutils/testfixtures",
24-
"//pkg/util/envutil",
25-
"//pkg/util/leaktest",
23+
"//pkg/ts",
2624
"//pkg/util/log",
27-
"//pkg/util/randutil",
2825
"//pkg/util/stop",
29-
"//pkg/util/syncutil",
3026
"//pkg/workload",
3127
"//pkg/workload/histogram",
3228
"//pkg/workload/tpcc",
3329
"//pkg/workload/workloadsql",
34-
"@com_github_cockroachdb_pebble//vfs",
35-
"@com_github_jackc_pgx_v5//:pgx",
36-
"@com_github_stretchr_testify//require",
3730
],
3831
)

0 commit comments

Comments
 (0)