Skip to content

Commit

Permalink
tracing: avoid temp buffer for structured events in recordFinishedChi…
Browse files Browse the repository at this point in the history
…ldrenLocked

May not be as important after cockroachdb#133307.

This commit reworks the logic in crdbSpan.recordFinishedChildrenLocked to avoid
the use of a temporary buffer for structured events when a child is finished
while a span is recording structured events. Instead of using a temporary buffer,
we now collect the child's events directly into the parents, and then trim the
slice to enforce the byte size limit.

```
name                            old time/op    new time/op    delta
Sysbench/SQL/oltp_read_only       36.2ms ± 2%    35.8ms ± 1%  -1.17%  (p=0.014 n=9+9)
Sysbench/SQL/oltp_write_only      19.0ms ± 1%    19.4ms ± 4%  +2.18%  (p=0.006 n=8+9)
Sysbench/SQL/oltp_read_write      58.4ms ± 3%    58.3ms ± 4%    ~     (p=0.730 n=9+9)
Sysbench/SQL/oltp_point_select    1.61ms ± 2%    1.58ms ± 2%  -1.68%  (p=0.013 n=9+10)
Sysbench/SQL/oltp_begin_commit     587µs ± 2%     587µs ± 2%    ~     (p=0.842 n=10+9)

name                            old alloc/op   new alloc/op   delta
Sysbench/SQL/oltp_read_only       1.05MB ± 1%    1.04MB ± 1%  -0.68%  (p=0.001 n=10+9)
Sysbench/SQL/oltp_write_only       402kB ± 1%     403kB ± 1%    ~     (p=0.661 n=10+9)
Sysbench/SQL/oltp_read_write      1.50MB ± 1%    1.49MB ± 1%    ~     (p=0.579 n=10+10)
Sysbench/SQL/oltp_point_select    35.0kB ± 0%    34.9kB ± 0%  -0.32%  (p=0.002 n=8+9)
Sysbench/SQL/oltp_begin_commit    18.8kB ± 0%    18.8kB ± 0%    ~     (p=0.845 n=10+8)

name                            old allocs/op  new allocs/op  delta
Sysbench/SQL/oltp_read_only        6.96k ± 1%     6.86k ± 0%  -1.51%  (p=0.000 n=10+7)
Sysbench/SQL/oltp_write_only       3.61k ± 0%     3.60k ± 0%  -0.30%  (p=0.009 n=10+9)
Sysbench/SQL/oltp_read_write       11.0k ± 1%     10.9k ± 1%  -0.82%  (p=0.014 n=10+10)
Sysbench/SQL/oltp_point_select       317 ± 0%       314 ± 0%  -1.04%  (p=0.000 n=7+10)
Sysbench/SQL/oltp_begin_commit       139 ± 0%       140 ± 0%    ~     (p=0.828 n=10+9)
```

Epic: None
Release note: None
  • Loading branch information
nvanbenschoten committed Nov 4, 2024
1 parent 8556bc7 commit 72b919f
Showing 1 changed file with 18 additions and 7 deletions.
25 changes: 18 additions & 7 deletions pkg/util/tracing/crdbspan.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ func (s *crdbSpan) getVerboseRecording(includeDetachedChildren bool, finishing b
result.StructuredRecordsSizeBytes += result.Root.StructuredRecordsSizeBytes
for i := range oldEvents {
size := int64(oldEvents[i].Size())
if result.StructuredRecordsSizeBytes+size < maxStructuredBytesPerTrace {
if result.StructuredRecordsSizeBytes+size <= maxStructuredBytesPerTrace {
result.Root.AddStructuredRecord(oldEvents[i])
result.StructuredRecordsSizeBytes += size
}
Expand Down Expand Up @@ -883,14 +883,25 @@ func (s *crdbSpan) recordFinishedChildrenLocked(childRec Trace) {
childRec.Root.ParentSpanID = s.spanID
s.mu.recording.finishedChildren.addChildren([]Trace{childRec}, maxRecordedSpansPerTrace, maxStructuredBytesPerTrace)
case tracingpb.RecordingStructured:
buf := childRec.appendStructuredEventsRecursively(nil /* buffer */)
for i := range buf {
event := &buf[i]
if s.mu.recording.finishedChildren.StructuredRecordsSizeBytes+int64(event.MemorySize()) < maxStructuredBytesPerTrace {
size := s.mu.recording.finishedChildren.Root.AddStructuredRecord(*event)
s.mu.recording.finishedChildren.StructuredRecordsSizeBytes += size
fc := &s.mu.recording.finishedChildren
num := len(fc.Root.StructuredRecords)
fc.Root.StructuredRecords = childRec.appendStructuredEventsRecursively(fc.Root.StructuredRecords)
// Account for the size of the structured records that were appended,
// breaking out of the loop if we hit the byte limit. This incorporates
// the byte size accounting logic from RecordedSpan.AddStructuredRecord.
for ; num < len(fc.Root.StructuredRecords); num++ {
size := int64(fc.Root.StructuredRecords[num].MemorySize())
if fc.StructuredRecordsSizeBytes+size > maxStructuredBytesPerTrace {
break
}
fc.Root.StructuredRecordsSizeBytes += size
fc.StructuredRecordsSizeBytes += size
}
// Trim any remaining entries if we hit the byte limit.
for i := num; i < len(fc.Root.StructuredRecords); i++ {
fc.Root.StructuredRecords[i] = tracingpb.StructuredRecord{}
}
fc.Root.StructuredRecords = fc.Root.StructuredRecords[:num]
case tracingpb.RecordingOff:
break
default:
Expand Down

0 comments on commit 72b919f

Please sign in to comment.