[go] runtime: emit STW events for all pauses, not just those for the GC

22 views
Skip to first unread message

Michael Knyszek (Gerrit)

unread,
May 11, 2023, 5:15:21 PM5/11/23
to goph...@pubsubhelper.golang.org, Michael Pratt, golang-co...@googlegroups.com

Attention is currently required from: Michael Pratt.

Patch set 1:Run-TryBot +1

View Change

    To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
    Gerrit-Change-Number: 494495
    Gerrit-PatchSet: 1
    Gerrit-Owner: Michael Knyszek <mkny...@google.com>
    Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
    Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
    Gerrit-Attention: Michael Pratt <mpr...@google.com>
    Gerrit-Comment-Date: Thu, 11 May 2023 21:15:17 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes

    Michael Pratt (Gerrit)

    unread,
    May 11, 2023, 5:35:22 PM5/11/23
    to Michael Knyszek, goph...@pubsubhelper.golang.org, Gopher Robot, Michael Pratt, golang-co...@googlegroups.com

    Attention is currently required from: Michael Knyszek.

    View Change

    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:

    To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
    Gerrit-Change-Number: 494495
    Gerrit-PatchSet: 1
    Gerrit-Owner: Michael Knyszek <mkny...@google.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
    Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
    Gerrit-Attention: Michael Knyszek <mkny...@google.com>
    Gerrit-Comment-Date: Thu, 11 May 2023 21:35:17 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No

    Michael Knyszek (Gerrit)

    unread,
    May 12, 2023, 11:31:01 AM5/12/23
    to goph...@pubsubhelper.golang.org, Gopher Robot, Michael Pratt, golang-co...@googlegroups.com

    Attention is currently required from: Michael Pratt.

    View Change

    1 comment:

    • File src/internal/trace/parser.go:

      • 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.

    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
    Gerrit-Change-Number: 494495
    Gerrit-PatchSet: 1
    Gerrit-Owner: Michael Knyszek <mkny...@google.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
    Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
    Gerrit-Attention: Michael Pratt <mpr...@google.com>
    Gerrit-Comment-Date: Fri, 12 May 2023 15:30:59 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Michael Pratt <mpr...@google.com>

    Michael Knyszek (Gerrit)

    unread,
    May 12, 2023, 11:43:42 AM5/12/23
    to goph...@pubsubhelper.golang.org, Gopher Robot, Michael Pratt, golang-co...@googlegroups.com

    Attention is currently required from: Michael Pratt.

    View Change

    1 comment:

    • File src/runtime/proc.go:

    To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
    Gerrit-Change-Number: 494495
    Gerrit-PatchSet: 1
    Gerrit-Owner: Michael Knyszek <mkny...@google.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
    Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
    Gerrit-Attention: Michael Pratt <mpr...@google.com>
    Gerrit-Comment-Date: Fri, 12 May 2023 15:43:39 +0000

    Michael Knyszek (Gerrit)

    unread,
    May 12, 2023, 12:05:41 PM5/12/23
    to goph...@pubsubhelper.golang.org, Gopher Robot, Michael Pratt, golang-co...@googlegroups.com

    Attention is currently required from: Michael Pratt.

    Patch set 2:Run-TryBot +1

    View Change

      To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
      Gerrit-Change-Number: 494495
      Gerrit-PatchSet: 2
      Gerrit-Owner: Michael Knyszek <mkny...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
      Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
      Gerrit-Attention: Michael Pratt <mpr...@google.com>
      Gerrit-Comment-Date: Fri, 12 May 2023 16:05:38 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes

      Michael Knyszek (Gerrit)

      unread,
      May 12, 2023, 12:06:58 PM5/12/23
      to goph...@pubsubhelper.golang.org, Gopher Robot, Michael Pratt, golang-co...@googlegroups.com

      Attention is currently required from: Michael Pratt.

      View Change

      2 comments:

      • File src/internal/trace/parser.go:

        • 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:

        • :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.

      Gerrit-MessageType: comment
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
      Gerrit-Change-Number: 494495
      Gerrit-PatchSet: 2
      Gerrit-Owner: Michael Knyszek <mkny...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
      Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
      Gerrit-Attention: Michael Pratt <mpr...@google.com>
      Gerrit-Comment-Date: Fri, 12 May 2023 16:06:55 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Michael Knyszek <mkny...@google.com>
      Comment-In-Reply-To: Michael Pratt <mpr...@google.com>

      Michael Knyszek (Gerrit)

      unread,
      May 12, 2023, 12:13:28 PM5/12/23
      to goph...@pubsubhelper.golang.org, Gopher Robot, Michael Pratt, golang-co...@googlegroups.com

      Attention is currently required from: Michael Pratt.

      Patch set 3:Run-TryBot +1

      View Change

        To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-MessageType: comment
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
        Gerrit-Change-Number: 494495
        Gerrit-PatchSet: 3
        Gerrit-Owner: Michael Knyszek <mkny...@google.com>
        Gerrit-Reviewer: Gopher Robot <go...@golang.org>
        Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
        Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
        Gerrit-Attention: Michael Pratt <mpr...@google.com>
        Gerrit-Comment-Date: Fri, 12 May 2023 16:13:25 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: Yes

        Michael Knyszek (Gerrit)

        unread,
        May 12, 2023, 12:54:47 PM5/12/23
        to goph...@pubsubhelper.golang.org, Gopher Robot, Michael Pratt, golang-co...@googlegroups.com

        Attention is currently required from: Michael Pratt.

        Patch set 4:Run-TryBot +1

        View Change

          To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-MessageType: comment
          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
          Gerrit-Change-Number: 494495
          Gerrit-PatchSet: 4
          Gerrit-Owner: Michael Knyszek <mkny...@google.com>
          Gerrit-Reviewer: Gopher Robot <go...@golang.org>
          Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
          Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
          Gerrit-Attention: Michael Pratt <mpr...@google.com>
          Gerrit-Comment-Date: Fri, 12 May 2023 16:54:43 +0000
          Gerrit-HasComments: No
          Gerrit-Has-Labels: Yes

          Michael Knyszek (Gerrit)

          unread,
          May 12, 2023, 4:40:07 PM5/12/23
          to goph...@pubsubhelper.golang.org, Gopher Robot, Michael Pratt, golang-co...@googlegroups.com

          Attention is currently required from: Michael Pratt.

          Patch set 7:Run-TryBot +1

          View Change

            To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.

            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
            Gerrit-Change-Number: 494495
            Gerrit-PatchSet: 7
            Gerrit-Owner: Michael Knyszek <mkny...@google.com>
            Gerrit-Reviewer: Gopher Robot <go...@golang.org>
            Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
            Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
            Gerrit-Attention: Michael Pratt <mpr...@google.com>
            Gerrit-Comment-Date: Fri, 12 May 2023 20:40:04 +0000
            Gerrit-HasComments: No
            Gerrit-Has-Labels: Yes

            Michael Knyszek (Gerrit)

            unread,
            May 18, 2023, 4:45:48 PM5/18/23
            to goph...@pubsubhelper.golang.org, Felix Geisendörfer, Nick Ripley, Gopher Robot, Michael Pratt, golang-co...@googlegroups.com

            Attention is currently required from: Michael Pratt.

            Patch set 12:Run-TryBot +1

            View Change

            1 comment:

            • Patchset:

              • Patch Set #12:

                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.

            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
            Gerrit-Change-Number: 494495
            Gerrit-PatchSet: 12
            Gerrit-Owner: Michael Knyszek <mkny...@google.com>
            Gerrit-Reviewer: Gopher Robot <go...@golang.org>
            Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
            Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
            Gerrit-CC: Felix Geisendörfer <felix.gei...@datadoghq.com>
            Gerrit-CC: Nick Ripley <nick....@datadoghq.com>
            Gerrit-Attention: Michael Pratt <mpr...@google.com>
            Gerrit-Comment-Date: Thu, 18 May 2023 20:45:45 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: Yes

            Michael Pratt (Gerrit)

            unread,
            May 18, 2023, 6:02:42 PM5/18/23
            to Michael Knyszek, goph...@pubsubhelper.golang.org, Michael Pratt, Gopher Robot, Felix Geisendörfer, Nick Ripley, golang-co...@googlegroups.com

            Attention is currently required from: Michael Knyszek.

            Patch set 12:Code-Review +2

            View Change

            1 comment:

            • File src/internal/trace/parser.go:

              • 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.

            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
            Gerrit-Change-Number: 494495
            Gerrit-PatchSet: 12
            Gerrit-Owner: Michael Knyszek <mkny...@google.com>
            Gerrit-Reviewer: Gopher Robot <go...@golang.org>
            Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
            Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
            Gerrit-CC: Felix Geisendörfer <felix.gei...@datadoghq.com>
            Gerrit-CC: Nick Ripley <nick....@datadoghq.com>
            Gerrit-Attention: Michael Knyszek <mkny...@google.com>
            Gerrit-Comment-Date: Thu, 18 May 2023 22:02:39 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: Yes

            Michael Knyszek (Gerrit)

            unread,
            May 19, 2023, 12:50:32 PM5/19/23
            to goph...@pubsubhelper.golang.org, Michael Pratt, Gopher Robot, Felix Geisendörfer, Nick Ripley, golang-co...@googlegroups.com

            Patch set 12:Run-TryBot +1

            View Change

            1 comment:

            • File src/internal/trace/parser.go:

              • 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.

            Gerrit-MessageType: comment
            Gerrit-Project: go
            Gerrit-Branch: master
            Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
            Gerrit-Change-Number: 494495
            Gerrit-PatchSet: 12
            Gerrit-Owner: Michael Knyszek <mkny...@google.com>
            Gerrit-Reviewer: Gopher Robot <go...@golang.org>
            Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
            Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
            Gerrit-CC: Felix Geisendörfer <felix.gei...@datadoghq.com>
            Gerrit-CC: Nick Ripley <nick....@datadoghq.com>
            Gerrit-Comment-Date: Fri, 19 May 2023 16:50:29 +0000

            Michael Knyszek (Gerrit)

            unread,
            May 19, 2023, 12:50:36 PM5/19/23
            to goph...@pubsubhelper.golang.org, Michael Pratt, Gopher Robot, Felix Geisendörfer, Nick Ripley, golang-co...@googlegroups.com

            Patch set 13:Run-TryBot +1

            View Change

              To view, visit change 494495. To unsubscribe, or for help writing mail filters, visit settings.

              Gerrit-MessageType: comment
              Gerrit-Project: go
              Gerrit-Branch: master
              Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
              Gerrit-Change-Number: 494495
              Gerrit-PatchSet: 13
              Gerrit-Owner: Michael Knyszek <mkny...@google.com>
              Gerrit-Reviewer: Gopher Robot <go...@golang.org>
              Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
              Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
              Gerrit-CC: Felix Geisendörfer <felix.gei...@datadoghq.com>
              Gerrit-CC: Nick Ripley <nick....@datadoghq.com>
              Gerrit-Comment-Date: Fri, 19 May 2023 16:50:33 +0000
              Gerrit-HasComments: No
              Gerrit-Has-Labels: Yes

              Michael Knyszek (Gerrit)

              unread,
              May 19, 2023, 1:06:50 PM5/19/23
              to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gopher Robot, Michael Pratt, Felix Geisendörfer, Nick Ripley, golang-co...@googlegroups.com

              Michael Knyszek submitted this change.

              View 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:
              ```

              Approvals: Michael Knyszek: Run TryBots Michael Pratt: Looks good to me, approved Gopher Robot: TryBots succeeded
              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.

              Gerrit-MessageType: merged
              Gerrit-Project: go
              Gerrit-Branch: master
              Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
              Gerrit-Change-Number: 494495
              Gerrit-PatchSet: 14

              Felix Geisendörfer (Gerrit)

              unread,
              May 24, 2023, 4:49:48 AM5/24/23
              to goph...@pubsubhelper.golang.org, Gopher Robot, Michael Pratt, Nick Ripley, golang-co...@googlegroups.com

              View Change

              1 comment:

              • Patchset:

                • Patch Set #14:

                  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.

              Gerrit-MessageType: comment
              Gerrit-Project: go
              Gerrit-Branch: master
              Gerrit-Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
              Gerrit-Change-Number: 494495
              Gerrit-PatchSet: 14
              Gerrit-Owner: Michael Knyszek <mkny...@google.com>
              Gerrit-Reviewer: Gopher Robot <go...@golang.org>
              Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>
              Gerrit-Reviewer: Michael Pratt <mpr...@google.com>
              Gerrit-CC: Felix Geisendörfer <felix.gei...@datadoghq.com>
              Gerrit-CC: Nick Ripley <nick....@datadoghq.com>
              Gerrit-Comment-Date: Wed, 24 May 2023 08:49:43 +0000
              Gerrit-HasComments: Yes
              Gerrit-Has-Labels: No
              Reply all
              Reply to author
              Forward
              0 new messages