Attention is currently required from: Michael Pratt.
Patch set 1:Run-TryBot +1
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Michael Knyszek.
2 comments:
File src/internal/trace/parser.go:
Patch Set #1, Line 1149: func runtime_stwReasonString(r byte) string
This feels wrong to me. The trace parser can parse traces from any version of Go, it doesn't seem that it should be tied to runtime internals.
I suppose this works for now, but if a future version changes the strings, we'd need to copy this function into internal/trace in order to handle older 1021 traces.
File src/runtime/proc.go:
Patch Set #1, Line 1189: "GC mark termination",
Use named entries?
```suggestion
stwGCMarkTerm: "GC mark termination",
```
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Michael Pratt.
1 comment:
File src/internal/trace/parser.go:
Patch Set #1, Line 1149: func runtime_stwReasonString(r byte) string
This feels wrong to me. […]
yeah I had the same thought shortly after logging out yesterday. I suppose we need to copy the values, version them, and have a test.
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.
File src/runtime/proc.go:
Patch Set #1, Line 1189: "GC mark termination",
Use named entries? […]
:O you can do that!? omg, that rules. https://go.dev/play/p/KyKYDo9GtoV
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Michael Pratt.
Patch set 2:Run-TryBot +1
Attention is currently required from: Michael Pratt.
2 comments:
File src/internal/trace/parser.go:
Patch Set #1, Line 1149: func runtime_stwReasonString(r byte) string
yeah I had the same thought shortly after logging out yesterday. […]
I'm actually not exactly sure how to test this. maybe one of the other tests should validate that the STW reason is not "unknown" (given the new branch)?
File src/runtime/proc.go:
Patch Set #1, Line 1189: "GC mark termination",
:O you can do that!? omg, that rules. https://go. […]
Done, though in hindsight waitReason already did this, I just never noticed.
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Michael Pratt.
Patch set 3:Run-TryBot +1
Attention is currently required from: Michael Pratt.
Patch set 4:Run-TryBot +1
Attention is currently required from: Michael Pratt.
Patch set 7:Run-TryBot +1
Attention is currently required from: Michael Pratt.
Patch set 12:Run-TryBot +1
1 comment:
Patchset:
Felix and Nick FYI. this is really cleanup that'll make changing tracers easier later, but it is a change to the tracer.
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Michael Knyszek.
Patch set 12:Code-Review +2
1 comment:
File src/internal/trace/parser.go:
Patch Set #1, Line 1149: func runtime_stwReasonString(r byte) string
I'm actually not exactly sure how to test this. […]
My best thought would be to trace a program that does ReadMemStats (something not done in every program), and look for a "ReadMemStats" STW trace entry.
I'm not sure if we have such end-to-end tests currently though.
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 12:Run-TryBot +1
Patch Set #1, Line 1149: func runtime_stwReasonString(r byte) string
My best thought would be to trace a program that does ReadMemStats (something not done in every prog […]
turns out the traces produced by mkcanned already call both ReadMemStats and GOMAXPROCS. I just regenerated mkcanned and added an extra check to the test.
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 13:Run-TryBot +1
Michael Knyszek submitted this change.
12 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/internal/trace/testdata/user_task_region_1_21_good
Insertions: 0, Deletions: 0.
@@ -154,6 +154,7 @@
_ = x[InvalidUnsafeString-146]
_ = x[InvalidClear-148]
_ = x[TypeTooLarge-149]
+ _ = x[InvalidMinMaxOperand-150]
}
const (
@@ -162,7 +163,7 @@
_Code_name_2 = "InvalidPtrEmbedBadRecvInvalidRecvDuplicateFieldAndMethodDuplicateMethodInvalidBlankInvalidIotaMissingInitBodyInvalidInitSigInvalidInitDeclInvalidMainDeclTooManyValuesNotAnExprTruncatedFloatNumericOverflowUndefinedOpMismatchedTypesDivByZeroNonNumericIncDecUnaddressableOperandInvalidIndirectionNonIndexableOperandInvalidIndexSwappedSliceIndicesNonSliceableOperandInvalidSliceExprInvalidShiftCountInvalidShiftOperandInvalidReceiveInvalidSendDuplicateLitKeyMissingLitKeyInvalidLitIndexOversizeArrayLitMixedStructLitInvalidStructLitMissingLitFieldDuplicateLitFieldUnexportedLitFieldInvalidLitFieldUntypedLitInvalidLitAmbiguousSelectorUndeclaredImportedNameUnexportedNameUndeclaredNameMissingFieldOrMethodBadDotDotDotSyntaxNonVariadicDotDotDotMisplacedDotDotDot"
_Code_name_3 = "InvalidDotDotDotUncalledBuiltinInvalidAppendInvalidCapInvalidCloseInvalidCopyInvalidComplexInvalidDeleteInvalidImagInvalidLenSwappedMakeArgsInvalidMakeInvalidRealInvalidAssertImpossibleAssertInvalidConversionInvalidUntypedConversionBadOffsetofSyntaxInvalidOffsetofUnusedExprUnusedVarMissingReturnWrongResultCountOutOfScopeResultInvalidCondInvalidPostDecl"
_Code_name_4 = "InvalidIterVarInvalidRangeExprMisplacedBreakMisplacedContinueMisplacedFallthroughDuplicateCaseDuplicateDefaultBadTypeKeywordInvalidTypeSwitchInvalidExprSwitchInvalidSelectCaseUndeclaredLabelDuplicateLabelMisplacedLabelUnusedLabelJumpOverDeclJumpIntoBlockInvalidMethodExprWrongArgCountInvalidCallUnusedResultsInvalidDeferInvalidGoBadDeclRepeatedDeclInvalidUnsafeAddInvalidUnsafeSliceUnsupportedFeatureNotAGenericTypeWrongTypeArgCountCannotInferTypeArgsInvalidTypeArgInvalidInstanceCycleInvalidUnionMisplacedConstraintIfaceInvalidMethodTypeParamsMisplacedTypeParamInvalidUnsafeSliceDataInvalidUnsafeString"
- _Code_name_5 = "InvalidClearTypeTooLarge"
+ _Code_name_5 = "InvalidClearTypeTooLargeInvalidMinMaxOperand"
)
var (
@@ -170,7 +171,7 @@
_Code_index_2 = [...]uint16{0, 15, 22, 33, 56, 71, 83, 94, 109, 123, 138, 153, 166, 175, 189, 204, 215, 230, 239, 255, 275, 293, 312, 324, 343, 362, 378, 395, 414, 428, 439, 454, 467, 482, 498, 512, 528, 543, 560, 578, 593, 603, 613, 630, 652, 666, 680, 700, 718, 738, 756}
_Code_index_3 = [...]uint16{0, 16, 31, 44, 54, 66, 77, 91, 104, 115, 125, 140, 151, 162, 175, 191, 208, 232, 249, 264, 274, 283, 296, 312, 328, 339, 354}
_Code_index_4 = [...]uint16{0, 14, 30, 44, 61, 81, 94, 110, 124, 141, 158, 175, 190, 204, 218, 229, 241, 254, 271, 284, 295, 308, 320, 329, 336, 348, 364, 382, 400, 415, 432, 451, 465, 485, 497, 521, 544, 562, 584, 603}
- _Code_index_5 = [...]uint8{0, 12, 24}
+ _Code_index_5 = [...]uint8{0, 12, 24, 44}
)
func (i Code) String() string {
@@ -189,7 +190,7 @@
case 108 <= i && i <= 146:
i -= 108
return _Code_name_4[_Code_index_4[i]:_Code_index_4[i+1]]
- case 148 <= i && i <= 149:
+ case 148 <= i && i <= 150:
i -= 148
return _Code_name_5[_Code_index_5[i]:_Code_index_5[i+1]]
default:
```
```
The name of the file: src/internal/trace/parser.go
Insertions: 1, Deletions: 1.
@@ -1152,7 +1152,7 @@
EvCPUSample: {"CPUSample", 1019, true, []string{"ts", "p", "g"}, nil},
}
-// Keep in sync with src/runtime/proc.go.
+// Copied from src/runtime/proc.go:stwReasonStrings in Go 1.21.
var stwReasonStringsGo121 = [...]string{
"unknown",
"GC mark termination",
```
```
The name of the file: src/internal/trace/testdata/http_1_21_good
Insertions: 0, Deletions: 0.
@@ -154,6 +154,7 @@
_ = x[InvalidUnsafeString-146]
_ = x[InvalidClear-148]
_ = x[TypeTooLarge-149]
+ _ = x[InvalidMinMaxOperand-150]
}
const (
@@ -162,7 +163,7 @@
_Code_name_2 = "InvalidPtrEmbedBadRecvInvalidRecvDuplicateFieldAndMethodDuplicateMethodInvalidBlankInvalidIotaMissingInitBodyInvalidInitSigInvalidInitDeclInvalidMainDeclTooManyValuesNotAnExprTruncatedFloatNumericOverflowUndefinedOpMismatchedTypesDivByZeroNonNumericIncDecUnaddressableOperandInvalidIndirectionNonIndexableOperandInvalidIndexSwappedSliceIndicesNonSliceableOperandInvalidSliceExprInvalidShiftCountInvalidShiftOperandInvalidReceiveInvalidSendDuplicateLitKeyMissingLitKeyInvalidLitIndexOversizeArrayLitMixedStructLitInvalidStructLitMissingLitFieldDuplicateLitFieldUnexportedLitFieldInvalidLitFieldUntypedLitInvalidLitAmbiguousSelectorUndeclaredImportedNameUnexportedNameUndeclaredNameMissingFieldOrMethodBadDotDotDotSyntaxNonVariadicDotDotDotMisplacedDotDotDot"
_Code_name_3 = "InvalidDotDotDotUncalledBuiltinInvalidAppendInvalidCapInvalidCloseInvalidCopyInvalidComplexInvalidDeleteInvalidImagInvalidLenSwappedMakeArgsInvalidMakeInvalidRealInvalidAssertImpossibleAssertInvalidConversionInvalidUntypedConversionBadOffsetofSyntaxInvalidOffsetofUnusedExprUnusedVarMissingReturnWrongResultCountOutOfScopeResultInvalidCondInvalidPostDecl"
_Code_name_4 = "InvalidIterVarInvalidRangeExprMisplacedBreakMisplacedContinueMisplacedFallthroughDuplicateCaseDuplicateDefaultBadTypeKeywordInvalidTypeSwitchInvalidExprSwitchInvalidSelectCaseUndeclaredLabelDuplicateLabelMisplacedLabelUnusedLabelJumpOverDeclJumpIntoBlockInvalidMethodExprWrongArgCountInvalidCallUnusedResultsInvalidDeferInvalidGoBadDeclRepeatedDeclInvalidUnsafeAddInvalidUnsafeSliceUnsupportedFeatureNotAGenericTypeWrongTypeArgCountCannotInferTypeArgsInvalidTypeArgInvalidInstanceCycleInvalidUnionMisplacedConstraintIfaceInvalidMethodTypeParamsMisplacedTypeParamInvalidUnsafeSliceDataInvalidUnsafeString"
- _Code_name_5 = "InvalidClearTypeTooLarge"
+ _Code_name_5 = "InvalidClearTypeTooLargeInvalidMinMaxOperand"
)
var (
@@ -170,7 +171,7 @@
_Code_index_2 = [...]uint16{0, 15, 22, 33, 56, 71, 83, 94, 109, 123, 138, 153, 166, 175, 189, 204, 215, 230, 239, 255, 275, 293, 312, 324, 343, 362, 378, 395, 414, 428, 439, 454, 467, 482, 498, 512, 528, 543, 560, 578, 593, 603, 613, 630, 652, 666, 680, 700, 718, 738, 756}
_Code_index_3 = [...]uint16{0, 16, 31, 44, 54, 66, 77, 91, 104, 115, 125, 140, 151, 162, 175, 191, 208, 232, 249, 264, 274, 283, 296, 312, 328, 339, 354}
_Code_index_4 = [...]uint16{0, 14, 30, 44, 61, 81, 94, 110, 124, 141, 158, 175, 190, 204, 218, 229, 241, 254, 271, 284, 295, 308, 320, 329, 336, 348, 364, 382, 400, 415, 432, 451, 465, 485, 497, 521, 544, 562, 584, 603}
- _Code_index_5 = [...]uint8{0, 12, 24}
+ _Code_index_5 = [...]uint8{0, 12, 24, 44}
)
func (i Code) String() string {
@@ -189,7 +190,7 @@
case 108 <= i && i <= 146:
i -= 108
return _Code_name_4[_Code_index_4[i]:_Code_index_4[i+1]]
- case 148 <= i && i <= 149:
+ case 148 <= i && i <= 150:
i -= 148
return _Code_name_5[_Code_index_5[i]:_Code_index_5[i+1]]
default:
```
```
The name of the file: src/internal/trace/testdata/stress_1_21_good
Insertions: 0, Deletions: 0.
@@ -154,6 +154,7 @@
_ = x[InvalidUnsafeString-146]
_ = x[InvalidClear-148]
_ = x[TypeTooLarge-149]
+ _ = x[InvalidMinMaxOperand-150]
}
const (
@@ -162,7 +163,7 @@
_Code_name_2 = "InvalidPtrEmbedBadRecvInvalidRecvDuplicateFieldAndMethodDuplicateMethodInvalidBlankInvalidIotaMissingInitBodyInvalidInitSigInvalidInitDeclInvalidMainDeclTooManyValuesNotAnExprTruncatedFloatNumericOverflowUndefinedOpMismatchedTypesDivByZeroNonNumericIncDecUnaddressableOperandInvalidIndirectionNonIndexableOperandInvalidIndexSwappedSliceIndicesNonSliceableOperandInvalidSliceExprInvalidShiftCountInvalidShiftOperandInvalidReceiveInvalidSendDuplicateLitKeyMissingLitKeyInvalidLitIndexOversizeArrayLitMixedStructLitInvalidStructLitMissingLitFieldDuplicateLitFieldUnexportedLitFieldInvalidLitFieldUntypedLitInvalidLitAmbiguousSelectorUndeclaredImportedNameUnexportedNameUndeclaredNameMissingFieldOrMethodBadDotDotDotSyntaxNonVariadicDotDotDotMisplacedDotDotDot"
_Code_name_3 = "InvalidDotDotDotUncalledBuiltinInvalidAppendInvalidCapInvalidCloseInvalidCopyInvalidComplexInvalidDeleteInvalidImagInvalidLenSwappedMakeArgsInvalidMakeInvalidRealInvalidAssertImpossibleAssertInvalidConversionInvalidUntypedConversionBadOffsetofSyntaxInvalidOffsetofUnusedExprUnusedVarMissingReturnWrongResultCountOutOfScopeResultInvalidCondInvalidPostDecl"
_Code_name_4 = "InvalidIterVarInvalidRangeExprMisplacedBreakMisplacedContinueMisplacedFallthroughDuplicateCaseDuplicateDefaultBadTypeKeywordInvalidTypeSwitchInvalidExprSwitchInvalidSelectCaseUndeclaredLabelDuplicateLabelMisplacedLabelUnusedLabelJumpOverDeclJumpIntoBlockInvalidMethodExprWrongArgCountInvalidCallUnusedResultsInvalidDeferInvalidGoBadDeclRepeatedDeclInvalidUnsafeAddInvalidUnsafeSliceUnsupportedFeatureNotAGenericTypeWrongTypeArgCountCannotInferTypeArgsInvalidTypeArgInvalidInstanceCycleInvalidUnionMisplacedConstraintIfaceInvalidMethodTypeParamsMisplacedTypeParamInvalidUnsafeSliceDataInvalidUnsafeString"
- _Code_name_5 = "InvalidClearTypeTooLarge"
+ _Code_name_5 = "InvalidClearTypeTooLargeInvalidMinMaxOperand"
)
var (
@@ -170,7 +171,7 @@
_Code_index_2 = [...]uint16{0, 15, 22, 33, 56, 71, 83, 94, 109, 123, 138, 153, 166, 175, 189, 204, 215, 230, 239, 255, 275, 293, 312, 324, 343, 362, 378, 395, 414, 428, 439, 454, 467, 482, 498, 512, 528, 543, 560, 578, 593, 603, 613, 630, 652, 666, 680, 700, 718, 738, 756}
_Code_index_3 = [...]uint16{0, 16, 31, 44, 54, 66, 77, 91, 104, 115, 125, 140, 151, 162, 175, 191, 208, 232, 249, 264, 274, 283, 296, 312, 328, 339, 354}
_Code_index_4 = [...]uint16{0, 14, 30, 44, 61, 81, 94, 110, 124, 141, 158, 175, 190, 204, 218, 229, 241, 254, 271, 284, 295, 308, 320, 329, 336, 348, 364, 382, 400, 415, 432, 451, 465, 485, 497, 521, 544, 562, 584, 603}
- _Code_index_5 = [...]uint8{0, 12, 24}
+ _Code_index_5 = [...]uint8{0, 12, 24, 44}
)
func (i Code) String() string {
@@ -189,7 +190,7 @@
case 108 <= i && i <= 146:
i -= 108
return _Code_name_4[_Code_index_4[i]:_Code_index_4[i+1]]
- case 148 <= i && i <= 149:
+ case 148 <= i && i <= 150:
i -= 148
return _Code_name_5[_Code_index_5[i]:_Code_index_5[i+1]]
default:
```
```
The name of the file: src/internal/trace/parser_test.go
Insertions: 14, Deletions: 1.
@@ -52,12 +52,13 @@
}
// Instead of Parse that requires a proper binary name for old traces,
// we use 'parse' that omits symbol lookup if an empty string is given.
- _, _, err = parse(bytes.NewReader(data), "")
+ ver, res, err := parse(bytes.NewReader(data), "")
switch {
case strings.HasSuffix(f.Name(), "_good"):
if err != nil {
t.Errorf("failed to parse good trace %v: %v", f.Name(), err)
}
+ checkTrace(t, ver, res)
case strings.HasSuffix(f.Name(), "_unordered"):
if err != ErrTimeOrder {
t.Errorf("unordered trace is not detected %v: %v", f.Name(), err)
@@ -68,6 +69,18 @@
}
}
+// checkTrace walks over a good trace and makes a bunch of additional checks
+// that may not cause the parser to outright fail.
+func checkTrace(t *testing.T, ver int, res ParseResult) {
+ for _, ev := range res.Events {
+ if ver >= 1021 {
+ if ev.Type == EvSTWStart && ev.SArgs[0] == "unknown" {
+ t.Errorf("found unknown STW event; update stwReasonStrings?")
+ }
+ }
+ }
+}
+
func TestParseVersion(t *testing.T) {
tests := map[string]int{
"go 1.5 trace\x00\x00\x00\x00": 1005,
```
```
The name of the file: src/internal/trace/testdata/stress_start_stop_1_21_good
Insertions: 0, Deletions: 0.
@@ -154,6 +154,7 @@
_ = x[InvalidUnsafeString-146]
_ = x[InvalidClear-148]
_ = x[TypeTooLarge-149]
+ _ = x[InvalidMinMaxOperand-150]
}
const (
@@ -162,7 +163,7 @@
_Code_name_2 = "InvalidPtrEmbedBadRecvInvalidRecvDuplicateFieldAndMethodDuplicateMethodInvalidBlankInvalidIotaMissingInitBodyInvalidInitSigInvalidInitDeclInvalidMainDeclTooManyValuesNotAnExprTruncatedFloatNumericOverflowUndefinedOpMismatchedTypesDivByZeroNonNumericIncDecUnaddressableOperandInvalidIndirectionNonIndexableOperandInvalidIndexSwappedSliceIndicesNonSliceableOperandInvalidSliceExprInvalidShiftCountInvalidShiftOperandInvalidReceiveInvalidSendDuplicateLitKeyMissingLitKeyInvalidLitIndexOversizeArrayLitMixedStructLitInvalidStructLitMissingLitFieldDuplicateLitFieldUnexportedLitFieldInvalidLitFieldUntypedLitInvalidLitAmbiguousSelectorUndeclaredImportedNameUnexportedNameUndeclaredNameMissingFieldOrMethodBadDotDotDotSyntaxNonVariadicDotDotDotMisplacedDotDotDot"
_Code_name_3 = "InvalidDotDotDotUncalledBuiltinInvalidAppendInvalidCapInvalidCloseInvalidCopyInvalidComplexInvalidDeleteInvalidImagInvalidLenSwappedMakeArgsInvalidMakeInvalidRealInvalidAssertImpossibleAssertInvalidConversionInvalidUntypedConversionBadOffsetofSyntaxInvalidOffsetofUnusedExprUnusedVarMissingReturnWrongResultCountOutOfScopeResultInvalidCondInvalidPostDecl"
_Code_name_4 = "InvalidIterVarInvalidRangeExprMisplacedBreakMisplacedContinueMisplacedFallthroughDuplicateCaseDuplicateDefaultBadTypeKeywordInvalidTypeSwitchInvalidExprSwitchInvalidSelectCaseUndeclaredLabelDuplicateLabelMisplacedLabelUnusedLabelJumpOverDeclJumpIntoBlockInvalidMethodExprWrongArgCountInvalidCallUnusedResultsInvalidDeferInvalidGoBadDeclRepeatedDeclInvalidUnsafeAddInvalidUnsafeSliceUnsupportedFeatureNotAGenericTypeWrongTypeArgCountCannotInferTypeArgsInvalidTypeArgInvalidInstanceCycleInvalidUnionMisplacedConstraintIfaceInvalidMethodTypeParamsMisplacedTypeParamInvalidUnsafeSliceDataInvalidUnsafeString"
- _Code_name_5 = "InvalidClearTypeTooLarge"
+ _Code_name_5 = "InvalidClearTypeTooLargeInvalidMinMaxOperand"
)
var (
@@ -170,7 +171,7 @@
_Code_index_2 = [...]uint16{0, 15, 22, 33, 56, 71, 83, 94, 109, 123, 138, 153, 166, 175, 189, 204, 215, 230, 239, 255, 275, 293, 312, 324, 343, 362, 378, 395, 414, 428, 439, 454, 467, 482, 498, 512, 528, 543, 560, 578, 593, 603, 613, 630, 652, 666, 680, 700, 718, 738, 756}
_Code_index_3 = [...]uint16{0, 16, 31, 44, 54, 66, 77, 91, 104, 115, 125, 140, 151, 162, 175, 191, 208, 232, 249, 264, 274, 283, 296, 312, 328, 339, 354}
_Code_index_4 = [...]uint16{0, 14, 30, 44, 61, 81, 94, 110, 124, 141, 158, 175, 190, 204, 218, 229, 241, 254, 271, 284, 295, 308, 320, 329, 336, 348, 364, 382, 400, 415, 432, 451, 465, 485, 497, 521, 544, 562, 584, 603}
- _Code_index_5 = [...]uint8{0, 12, 24}
+ _Code_index_5 = [...]uint8{0, 12, 24, 44}
)
func (i Code) String() string {
@@ -189,7 +190,7 @@
case 108 <= i && i <= 146:
i -= 108
return _Code_name_4[_Code_index_4[i]:_Code_index_4[i+1]]
- case 148 <= i && i <= 149:
+ case 148 <= i && i <= 150:
i -= 148
return _Code_name_5[_Code_index_5[i]:_Code_index_5[i+1]]
default:
```
runtime: emit STW events for all pauses, not just those for the GC
Currently STW events are only emitted for GC STWs. There's little reason
why the trace can't contain events for every STW: they're rare so don't
take up much space in the trace, yet being able to see when the world
was stopped is often critical to debugging certain latency issues,
especially when they stem from user-level APIs.
This change adds new "kinds" to the EvGCSTWStart event, renames the
GCSTW events to just "STW," and lets the parser deal with unknown STW
kinds for future backwards compatibility.
But, this change must break trace compatibility, so it bumps the trace
version to Go 1.21.
This change also includes a small cleanup in the trace command, which
previously checked for STW events when deciding whether user tasks
overlapped with a GC. Looking at the source, I don't see a way for STW
events to ever enter the stream that that code looks at, so that
condition has been deleted.
Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/494495
Reviewed-by: Michael Pratt <mpr...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Michael Knyszek <mkny...@google.com>
---
M src/cmd/trace/annotations.go
M src/cmd/trace/trace.go
M src/internal/trace/gc.go
M src/internal/trace/parser.go
M src/internal/trace/parser_test.go
A src/internal/trace/testdata/http_1_21_good
A src/internal/trace/testdata/stress_1_21_good
A src/internal/trace/testdata/stress_start_stop_1_21_good
A src/internal/trace/testdata/user_task_region_1_21_good
M src/runtime/debug.go
M src/runtime/export_debuglog_test.go
M src/runtime/export_test.go
M src/runtime/heapdump.go
M src/runtime/mgc.go
M src/runtime/mprof.go
M src/runtime/mstats.go
M src/runtime/os_linux.go
M src/runtime/proc.go
M src/runtime/trace.go
19 files changed, 177 insertions(+), 69 deletions(-)
diff --git a/src/cmd/trace/annotations.go b/src/cmd/trace/annotations.go
index 9ffce1b..0addc24 100644
--- a/src/cmd/trace/annotations.go
+++ b/src/cmd/trace/annotations.go
@@ -494,7 +494,7 @@
func (task *taskDesc) overlappingGCDuration(evs []*trace.Event) (overlapping time.Duration) {
for _, ev := range evs {
// make sure we only consider the global GC events.
- if typ := ev.Type; typ != trace.EvGCStart && typ != trace.EvGCSTWStart {
+ if typ := ev.Type; typ != trace.EvGCStart {
continue
}
diff --git a/src/cmd/trace/trace.go b/src/cmd/trace/trace.go
index 8951299..618df42 100644
--- a/src/cmd/trace/trace.go
+++ b/src/cmd/trace/trace.go
@@ -764,12 +764,12 @@
case trace.EvGCStart:
ctx.emitSlice(ev, "GC")
case trace.EvGCDone:
- case trace.EvGCSTWStart:
+ case trace.EvSTWStart:
if ctx.mode&modeGoroutineOriented != 0 {
continue
}
ctx.emitSlice(ev, fmt.Sprintf("STW (%s)", ev.SArgs[0]))
- case trace.EvGCSTWDone:
+ case trace.EvSTWDone:
case trace.EvGCMarkAssistStart:
// Mark assists can continue past preemptions, so truncate to the
// whichever comes first. We'll synthesize another slice if
diff --git a/src/internal/trace/gc.go b/src/internal/trace/gc.go
index c1bc862..3bd284e 100644
--- a/src/internal/trace/gc.go
+++ b/src/internal/trace/gc.go
@@ -27,6 +27,7 @@
const (
// UtilSTW means utilization should account for STW events.
+ // This includes non-GC STW events, which are typically user-requested.
UtilSTW UtilFlags = 1 << iota
// UtilBackground means utilization should account for
// background mark workers.
@@ -93,11 +94,11 @@
}
ps = append(ps, perP{series: series})
}
- case EvGCSTWStart:
+ case EvSTWStart:
if flags&UtilSTW != 0 {
stw++
}
- case EvGCSTWDone:
+ case EvSTWDone:
if flags&UtilSTW != 0 {
stw--
}
diff --git a/src/internal/trace/parser.go b/src/internal/trace/parser.go
index 0376e91..67fa60b 100644
--- a/src/internal/trace/parser.go
+++ b/src/internal/trace/parser.go
@@ -151,7 +151,7 @@
return
}
switch ver {
- case 1005, 1007, 1008, 1009, 1010, 1011, 1019:
+ case 1005, 1007, 1008, 1009, 1010, 1011, 1019, 1021:
// Note: When adding a new version, confirm that canned traces from the
// old version are part of the test suite. Add them using mkcanned.bash.
break
@@ -420,18 +420,29 @@
if raw.typ == EvGoStartLabel {
e.SArgs = []string{strings[e.Args[2]]}
}
- case EvGCSTWStart:
+ case EvSTWStart:
e.G = 0
- switch e.Args[0] {
- case 0:
- e.SArgs = []string{"mark termination"}
- case 1:
- e.SArgs = []string{"sweep termination"}
- default:
- err = fmt.Errorf("unknown STW kind %d", e.Args[0])
- return
+ if ver < 1021 {
+ switch e.Args[0] {
+ case 0:
+ e.SArgs = []string{"mark termination"}
+ case 1:
+ e.SArgs = []string{"sweep termination"}
+ default:
+ err = fmt.Errorf("unknown STW kind %d", e.Args[0])
+ return
+ }
+ } else if ver == 1021 {
+ if kind := e.Args[0]; kind < uint64(len(stwReasonStringsGo121)) {
+ e.SArgs = []string{stwReasonStringsGo121[kind]}
+ } else {
+ e.SArgs = []string{"unknown"}
+ }
+ } else {
+ // Can't make any assumptions.
+ e.SArgs = []string{"unknown"}
}
- case EvGCStart, EvGCDone, EvGCSTWDone:
+ case EvGCStart, EvGCDone, EvSTWDone:
e.G = 0
case EvGoEnd, EvGoStop, EvGoSched, EvGoPreempt,
EvGoSleep, EvGoBlock, EvGoBlockSend, EvGoBlockRecv,
@@ -653,20 +664,20 @@
}
evGC.Link = ev
evGC = nil
- case EvGCSTWStart:
+ case EvSTWStart:
evp := &evSTW
if ver < 1010 {
- // Before 1.10, EvGCSTWStart was per-P.
+ // Before 1.10, EvSTWStart was per-P.
evp = &p.evSTW
}
if *evp != nil {
return fmt.Errorf("previous STW is not ended before a new one (offset %v, time %v)", ev.Off, ev.Ts)
}
*evp = ev
- case EvGCSTWDone:
+ case EvSTWDone:
evp := &evSTW
if ver < 1010 {
- // Before 1.10, EvGCSTWDone was per-P.
+ // Before 1.10, EvSTWDone was per-P.
evp = &p.evSTW
}
if *evp == nil {
@@ -1015,7 +1026,7 @@
if ver < 1007 {
narg-- // 1.7 added an additional seq arg
}
- case EvGCSTWStart:
+ case EvSTWStart:
if ver < 1010 {
narg-- // 1.10 added an argument
}
@@ -1038,8 +1049,8 @@
EvProcStop = 6 // stop of P [timestamp]
EvGCStart = 7 // GC start [timestamp, seq, stack id]
EvGCDone = 8 // GC done [timestamp]
- EvGCSTWStart = 9 // GC mark termination start [timestamp, kind]
- EvGCSTWDone = 10 // GC mark termination done [timestamp]
+ EvSTWStart = 9 // GC mark termination start [timestamp, kind]
+ EvSTWDone = 10 // GC mark termination done [timestamp]
EvGCSweepStart = 11 // GC sweep start [timestamp, stack id]
EvGCSweepDone = 12 // GC sweep done [timestamp, swept, reclaimed]
EvGoCreate = 13 // goroutine creation [timestamp, new goroutine id, new stack id, stack id]
@@ -1098,8 +1109,8 @@
EvProcStop: {"ProcStop", 1005, false, []string{}, nil},
EvGCStart: {"GCStart", 1005, true, []string{"seq"}, nil}, // in 1.5 format it was {}
EvGCDone: {"GCDone", 1005, false, []string{}, nil},
- EvGCSTWStart: {"GCSTWStart", 1005, false, []string{"kindid"}, []string{"kind"}}, // <= 1.9, args was {} (implicitly {0})
- EvGCSTWDone: {"GCSTWDone", 1005, false, []string{}, nil},
+ EvSTWStart: {"STWStart", 1005, false, []string{"kindid"}, []string{"kind"}}, // <= 1.9, args was {} (implicitly {0})
+ EvSTWDone: {"STWDone", 1005, false, []string{}, nil},
EvGCSweepStart: {"GCSweepStart", 1005, true, []string{}, nil},
EvGCSweepDone: {"GCSweepDone", 1005, false, []string{"swept", "reclaimed"}, nil}, // before 1.9, format was {}
EvGoCreate: {"GoCreate", 1005, true, []string{"g", "stack"}, nil},
@@ -1140,3 +1151,24 @@
EvUserLog: {"UserLog", 1011, true, []string{"id", "keyid"}, []string{"category", "message"}},
EvCPUSample: {"CPUSample", 1019, true, []string{"ts", "p", "g"}, nil},
}
+
+// Copied from src/runtime/proc.go:stwReasonStrings in Go 1.21.
+var stwReasonStringsGo121 = [...]string{
+ "unknown",
+ "GC mark termination",
+ "GC sweep termination",
+ "write heap dump",
+ "goroutine profile",
+ "goroutine profile cleanup",
+ "all goroutines stack trace",
+ "read mem stats",
+ "AllThreadsSyscall",
+ "GOMAXPROCS",
+ "start trace",
+ "stop trace",
+ "CountPagesInUse (test)",
+ "ReadMetricsSlow (test)",
+ "ReadMemStatsSlow (test)",
+ "PageCachePagesLeaked (test)",
+ "ResetDebugLog (test)",
+}
diff --git a/src/internal/trace/parser_test.go b/src/internal/trace/parser_test.go
index cdab95a..fce660c 100644
--- a/src/internal/trace/parser_test.go
+++ b/src/internal/trace/parser_test.go
@@ -52,12 +52,13 @@
}
// Instead of Parse that requires a proper binary name for old traces,
// we use 'parse' that omits symbol lookup if an empty string is given.
- _, _, err = parse(bytes.NewReader(data), "")
+ ver, res, err := parse(bytes.NewReader(data), "")
switch {
case strings.HasSuffix(f.Name(), "_good"):
if err != nil {
t.Errorf("failed to parse good trace %v: %v", f.Name(), err)
}
+ checkTrace(t, ver, res)
case strings.HasSuffix(f.Name(), "_unordered"):
if err != ErrTimeOrder {
t.Errorf("unordered trace is not detected %v: %v", f.Name(), err)
@@ -68,6 +69,18 @@
}
}
+// checkTrace walks over a good trace and makes a bunch of additional checks
+// that may not cause the parser to outright fail.
+func checkTrace(t *testing.T, ver int, res ParseResult) {
+ for _, ev := range res.Events {
+ if ver >= 1021 {
+ if ev.Type == EvSTWStart && ev.SArgs[0] == "unknown" {
+ t.Errorf("found unknown STW event; update stwReasonStrings?")
+ }
+ }
+ }
+}
+
func TestParseVersion(t *testing.T) {
tests := map[string]int{
"go 1.5 trace\x00\x00\x00\x00": 1005,
diff --git a/src/internal/trace/testdata/http_1_21_good b/src/internal/trace/testdata/http_1_21_good
new file mode 100644
index 0000000..b3295f9
--- /dev/null
+++ b/src/internal/trace/testdata/http_1_21_good
Binary files differ
diff --git a/src/internal/trace/testdata/stress_1_21_good b/src/internal/trace/testdata/stress_1_21_good
new file mode 100644
index 0000000..1ade5e0
--- /dev/null
+++ b/src/internal/trace/testdata/stress_1_21_good
Binary files differ
diff --git a/src/internal/trace/testdata/stress_start_stop_1_21_good b/src/internal/trace/testdata/stress_start_stop_1_21_good
new file mode 100644
index 0000000..fff46a9
--- /dev/null
+++ b/src/internal/trace/testdata/stress_start_stop_1_21_good
Binary files differ
diff --git a/src/internal/trace/testdata/user_task_region_1_21_good b/src/internal/trace/testdata/user_task_region_1_21_good
new file mode 100644
index 0000000..5c01a64
--- /dev/null
+++ b/src/internal/trace/testdata/user_task_region_1_21_good
Binary files differ
diff --git a/src/runtime/debug.go b/src/runtime/debug.go
index 669c36f..9a92b45f 100644
--- a/src/runtime/debug.go
+++ b/src/runtime/debug.go
@@ -25,7 +25,7 @@
return ret
}
- stopTheWorldGC("GOMAXPROCS")
+ stopTheWorldGC(stwGOMAXPROCS)
// newprocs will be processed by startTheWorld
newprocs = int32(n)
diff --git a/src/runtime/export_debuglog_test.go b/src/runtime/export_debuglog_test.go
index c9dfdcb..f12aab0 100644
--- a/src/runtime/export_debuglog_test.go
+++ b/src/runtime/export_debuglog_test.go
@@ -35,7 +35,7 @@
}
func ResetDebugLog() {
- stopTheWorld("ResetDebugLog")
+ stopTheWorld(stwForTestResetDebugLog)
for l := allDloggers; l != nil; l = l.allLink {
l.w.write = 0
l.w.tick, l.w.nano = 0, 0
diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go
index c04b76e..db91bc6 100644
--- a/src/runtime/export_test.go
+++ b/src/runtime/export_test.go
@@ -276,7 +276,7 @@
var ReadUnaligned64 = readUnaligned64
func CountPagesInUse() (pagesInUse, counted uintptr) {
- stopTheWorld("CountPagesInUse")
+ stopTheWorld(stwForTestCountPagesInUse)
pagesInUse = uintptr(mheap_.pagesInUse.Load())
@@ -319,7 +319,7 @@
}
func ReadMetricsSlow(memStats *MemStats, samplesp unsafe.Pointer, len, cap int) {
- stopTheWorld("ReadMetricsSlow")
+ stopTheWorld(stwForTestReadMetricsSlow)
// Initialize the metrics beforehand because this could
// allocate and skew the stats.
@@ -347,7 +347,7 @@
// ReadMemStatsSlow returns both the runtime-computed MemStats and
// MemStats accumulated by scanning the heap.
func ReadMemStatsSlow() (base, slow MemStats) {
- stopTheWorld("ReadMemStatsSlow")
+ stopTheWorld(stwForTestReadMemStatsSlow)
// Run on the system stack to avoid stack growth allocation.
systemstack(func() {
@@ -1193,7 +1193,7 @@
}
func PageCachePagesLeaked() (leaked uintptr) {
- stopTheWorld("PageCachePagesLeaked")
+ stopTheWorld(stwForTestPageCachePagesLeaked)
// Walk over destroyed Ps and look for unflushed caches.
deadp := allp[len(allp):cap(allp)]
diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go
index e3f8011..8ddec8b 100644
--- a/src/runtime/heapdump.go
+++ b/src/runtime/heapdump.go
@@ -19,7 +19,7 @@
//go:linkname runtime_debug_WriteHeapDump runtime/debug.WriteHeapDump
func runtime_debug_WriteHeapDump(fd uintptr) {
- stopTheWorld("write heap dump")
+ stopTheWorld(stwWriteHeapDump)
// Keep m on this G's stack instead of the system stack.
// Both readmemstats_m and writeheapdump_m have pretty large
diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index bb60a3c..599f688 100644
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -658,10 +658,7 @@
now := nanotime()
work.tSweepTerm = now
work.pauseStart = now
- if traceEnabled() {
- traceGCSTWStart(1)
- }
- systemstack(stopTheWorldWithSema)
+ systemstack(func() { stopTheWorldWithSema(stwGCSweepTerm) })
// Finish sweep before we start concurrent scan.
systemstack(func() {
finishsweep_m()
@@ -726,7 +723,7 @@
// Concurrent mark.
systemstack(func() {
- now = startTheWorldWithSema(traceEnabled())
+ now = startTheWorldWithSema()
work.pauseNS += now - work.pauseStart
work.tMark = now
memstats.gcPauseDist.record(now - work.pauseStart)
@@ -848,10 +845,7 @@
work.tMarkTerm = now
work.pauseStart = now
getg().m.preemptoff = "gcing"
- if traceEnabled() {
- traceGCSTWStart(0)
- }
- systemstack(stopTheWorldWithSema)
+ systemstack(func() { stopTheWorldWithSema(stwGCMarkTerm) })
// The gcphase is _GCmark, it will transition to _GCmarktermination
// below. The important thing is that the wb remains active until
// all marking is complete. This includes writes made by the GC.
@@ -878,7 +872,7 @@
if restart {
getg().m.preemptoff = ""
systemstack(func() {
- now := startTheWorldWithSema(traceEnabled())
+ now := startTheWorldWithSema()
work.pauseNS += now - work.pauseStart
memstats.gcPauseDist.record(now - work.pauseStart)
})
@@ -1092,7 +1086,7 @@
throw("failed to set sweep barrier")
}
- systemstack(func() { startTheWorldWithSema(traceEnabled()) })
+ systemstack(func() { startTheWorldWithSema() })
// Flush the heap profile so we can start a new cycle next GC.
// This is relatively expensive, so we don't do it with the
diff --git a/src/runtime/mprof.go b/src/runtime/mprof.go
index 174ceb0..308ebae 100644
--- a/src/runtime/mprof.go
+++ b/src/runtime/mprof.go
@@ -901,7 +901,7 @@
ourg := getg()
- stopTheWorld("profile")
+ stopTheWorld(stwGoroutineProfile)
// Using gcount while the world is stopped should give us a consistent view
// of the number of live goroutines, minus the number of goroutines that are
// alive and permanently marked as "system". But to make this count agree
@@ -966,7 +966,7 @@
tryRecordGoroutineProfile(gp1, Gosched)
})
- stopTheWorld("profile cleanup")
+ stopTheWorld(stwGoroutineProfileCleanup)
endOffset := goroutineProfile.offset.Swap(0)
goroutineProfile.active = false
goroutineProfile.records = nil
@@ -1101,7 +1101,7 @@
return gp1 != gp && readgstatus(gp1) != _Gdead && !isSystemGoroutine(gp1, false)
}
- stopTheWorld("profile")
+ stopTheWorld(stwGoroutineProfile)
// World is stopped, no locking required.
n = 1
@@ -1187,7 +1187,7 @@
// into buf after the trace for the current goroutine.
func Stack(buf []byte, all bool) int {
if all {
- stopTheWorld("stack trace")
+ stopTheWorld(stwAllGoroutinesStack)
}
n := 0
diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go
index 3a5273f..3c17c0b 100644
--- a/src/runtime/mstats.go
+++ b/src/runtime/mstats.go
@@ -347,7 +347,7 @@
// which is a snapshot as of the most recently completed garbage
// collection cycle.
func ReadMemStats(m *MemStats) {
- stopTheWorld("read mem stats")
+ stopTheWorld(stwReadMemStats)
systemstack(func() {
readmemstats_m(m)
diff --git a/src/runtime/os_linux.go b/src/runtime/os_linux.go
index b0246e5..f407e6a 100644
--- a/src/runtime/os_linux.go
+++ b/src/runtime/os_linux.go
@@ -739,7 +739,7 @@
// N.B. Internally, this function does not depend on STW to
// successfully change every thread. It is only needed for user
// expectations, per above.
- stopTheWorld("doAllThreadsSyscall")
+ stopTheWorld(stwAllThreadsSyscall)
// This function depends on several properties:
//
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 35aeb2d..845e25d 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -1157,6 +1157,59 @@
return gp.atomicstatus.CompareAndSwap(_Gpreempted, _Gwaiting)
}
+// stwReason is an enumeration of reasons the world is stopping.
+type stwReason uint8
+
+// Reasons to stop-the-world.
+//
+// Avoid reusing reasons and add new ones instead.
+const (
+ stwUnknown stwReason = iota // "unknown"
+ stwGCMarkTerm // "GC mark termination"
+ stwGCSweepTerm // "GC sweep termination"
+ stwWriteHeapDump // "write heap dump"
+ stwGoroutineProfile // "goroutine profile"
+ stwGoroutineProfileCleanup // "goroutine profile cleanup"
+ stwAllGoroutinesStack // "all goroutines stack trace"
+ stwReadMemStats // "read mem stats"
+ stwAllThreadsSyscall // "AllThreadsSyscall"
+ stwGOMAXPROCS // "GOMAXPROCS"
+ stwStartTrace // "start trace"
+ stwStopTrace // "stop trace"
+ stwForTestCountPagesInUse // "CountPagesInUse (test)"
+ stwForTestReadMetricsSlow // "ReadMetricsSlow (test)"
+ stwForTestReadMemStatsSlow // "ReadMemStatsSlow (test)"
+ stwForTestPageCachePagesLeaked // "PageCachePagesLeaked (test)"
+ stwForTestResetDebugLog // "ResetDebugLog (test)"
+)
+
+func (r stwReason) String() string {
+ return stwReasonStrings[r]
+}
+
+// If you add to this list, also add it to src/internal/trace/parser.go.
+// If you change the values of any of the stw* constants, bump the trace
+// version number and make a copy of this.
+var stwReasonStrings = [...]string{
+ stwUnknown: "unknown",
+ stwGCMarkTerm: "GC mark termination",
+ stwGCSweepTerm: "GC sweep termination",
+ stwWriteHeapDump: "write heap dump",
+ stwGoroutineProfile: "goroutine profile",
+ stwGoroutineProfileCleanup: "goroutine profile cleanup",
+ stwAllGoroutinesStack: "all goroutines stack trace",
+ stwReadMemStats: "read mem stats",
+ stwAllThreadsSyscall: "AllThreadsSyscall",
+ stwGOMAXPROCS: "GOMAXPROCS",
+ stwStartTrace: "start trace",
+ stwStopTrace: "stop trace",
+ stwForTestCountPagesInUse: "CountPagesInUse (test)",
+ stwForTestReadMetricsSlow: "ReadMetricsSlow (test)",
+ stwForTestReadMemStatsSlow: "ReadMemStatsSlow (test)",
+ stwForTestPageCachePagesLeaked: "PageCachePagesLeaked (test)",
+ stwForTestResetDebugLog: "ResetDebugLog (test)",
+}
+
// stopTheWorld stops all P's from executing goroutines, interrupting
// all goroutines at GC safe points and records reason as the reason
// for the stop. On return, only the current goroutine's P is running.
@@ -1171,10 +1224,10 @@
// This is also used by routines that do stack dumps. If the system is
// in panic or being exited, this may not reliably stop all
// goroutines.
-func stopTheWorld(reason string) {
+func stopTheWorld(reason stwReason) {
semacquire(&worldsema)
gp := getg()
- gp.m.preemptoff = reason
+ gp.m.preemptoff = reason.String()
systemstack(func() {
// Mark the goroutine which called stopTheWorld preemptible so its
// stack may be scanned.
@@ -1188,14 +1241,14 @@
// have already completed by the time we exit.
// Don't provide a wait reason because we're still executing.
casGToWaiting(gp, _Grunning, waitReasonStoppingTheWorld)
- stopTheWorldWithSema()
+ stopTheWorldWithSema(reason)
casgstatus(gp, _Gwaiting, _Grunning)
})
}
// startTheWorld undoes the effects of stopTheWorld.
func startTheWorld() {
- systemstack(func() { startTheWorldWithSema(false) })
+ systemstack(func() { startTheWorldWithSema() })
// worldsema must be held over startTheWorldWithSema to ensure
// gomaxprocs cannot change while worldsema is held.
@@ -1221,7 +1274,7 @@
// stopTheWorldGC has the same effect as stopTheWorld, but blocks
// until the GC is not running. It also blocks a GC from starting
// until startTheWorldGC is called.
-func stopTheWorldGC(reason string) {
+func stopTheWorldGC(reason stwReason) {
semacquire(&gcsema)
stopTheWorld(reason)
}
@@ -1265,7 +1318,10 @@
// startTheWorldWithSema and stopTheWorldWithSema.
// Holding worldsema causes any other goroutines invoking
// stopTheWorld to block.
-func stopTheWorldWithSema() {
+func stopTheWorldWithSema(reason stwReason) {
+ if traceEnabled() {
+ traceSTWStart(reason)
+ }
gp := getg()
// If we hold a lock, then we won't be able to stop another M
@@ -1344,7 +1400,7 @@
worldStopped()
}
-func startTheWorldWithSema(emitTraceEvent bool) int64 {
+func startTheWorldWithSema() int64 {
assertWorldStopped()
mp := acquirem() // disable preemption because it can be holding p in a local var
@@ -1388,8 +1444,8 @@
// Capture start-the-world time before doing clean-up tasks.
startTime := nanotime()
- if emitTraceEvent {
- traceGCSTWDone()
+ if traceEnabled() {
+ traceSTWDone()
}
// Wakeup an additional proc in case we have excessive runnable goroutines
diff --git a/src/runtime/trace.go b/src/runtime/trace.go
index 45a066e..2fe6d2d 100644
--- a/src/runtime/trace.go
+++ b/src/runtime/trace.go
@@ -31,8 +31,8 @@
traceEvProcStop = 6 // stop of P [timestamp]
traceEvGCStart = 7 // GC start [timestamp, seq, stack id]
traceEvGCDone = 8 // GC done [timestamp]
- traceEvGCSTWStart = 9 // GC STW start [timestamp, kind]
- traceEvGCSTWDone = 10 // GC STW done [timestamp]
+ traceEvSTWStart = 9 // STW start [timestamp, kind]
+ traceEvSTWDone = 10 // STW done [timestamp]
traceEvGCSweepStart = 11 // GC sweep start [timestamp, stack id]
traceEvGCSweepDone = 12 // GC sweep done [timestamp, swept, reclaimed]
traceEvGoCreate = 13 // goroutine creation [timestamp, new goroutine id, new stack id, stack id]
@@ -171,7 +171,8 @@
// mTraceState is per-M state for the tracer.
type mTraceState struct {
- startingTrace bool // this M is in TraceStart, potentially before traceEnabled is true
+ startingTrace bool // this M is in TraceStart, potentially before traceEnabled is true
+ tracedSTWStart bool // this M traced a STW start, so it should trace an end
}
// pTraceState is per-P state for the tracer.
@@ -247,7 +248,7 @@
// Do not stop the world during GC so we ensure we always see
// a consistent view of GC-related events (e.g. a start is always
// paired with an end).
- stopTheWorldGC("start tracing")
+ stopTheWorldGC(stwStartTrace)
// Prevent sysmon from running any code that could generate events.
lock(&sched.sysmonlock)
@@ -377,7 +378,7 @@
func StopTrace() {
// Stop the world so that we can collect the trace buffers from all p's below,
// and also to avoid races with traceEvent.
- stopTheWorldGC("stop tracing")
+ stopTheWorldGC(stwStopTrace)
// See the comment in StartTrace.
lock(&sched.sysmonlock)
@@ -560,7 +561,7 @@
trace.headerWritten = true
trace.lockOwner = nil
unlock(&trace.lock)
- return []byte("go 1.19 trace\x00\x00\x00"), false
+ return []byte("go 1.21 trace\x00\x00\x00"), false
}
// Optimistically look for CPU profile samples. This may write new stack
// records, and may write new tracing buffers.
@@ -1485,12 +1486,23 @@
traceEvent(traceEvGCDone, -1)
}
-func traceGCSTWStart(kind int) {
- traceEvent(traceEvGCSTWStart, -1, uint64(kind))
+func traceSTWStart(reason stwReason) {
+ // Don't trace if this STW is for trace start/stop, since traceEnabled
+ // switches during a STW.
+ if reason == stwStartTrace || reason == stwStopTrace {
+ return
+ }
+ getg().m.trace.tracedSTWStart = true
+ traceEvent(traceEvSTWStart, -1, uint64(reason))
}
-func traceGCSTWDone() {
- traceEvent(traceEvGCSTWDone, -1)
+func traceSTWDone() {
+ mp := getg().m
+ if !mp.trace.tracedSTWStart {
+ return
+ }
+ mp.trace.tracedSTWStart = false
+ traceEvent(traceEvSTWDone, -1)
}
// traceGCSweepStart prepares to trace a sweep loop. This does not
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.
1 comment:
Patchset:
Just wanted to say thanks for this CL - this is going to be very useful ❤️
To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.