internal/trace: reduce event size by packing goroutine statuses

The trace parser was using an otherwise-unused event argument to hold an
extra goroutine state argument for the GoStatus & GoStatusStack events.
This is needed because the execution tracer just records the "after" for
state transitions, but we want to have both the "before" and "after"
states available in the StateTransition info for the parsed event. When
GoStatusStack was added, the size of the argument array was increased to
still have room for the extra status. However, statuses are currently
only 1 byte, and the status argument is 8 bytes, so there is plenty of
room to pack the "before" and "after" statuses in a single argument. Do
that instead to avoid the need for an extra argument.

Change-Id: I6886eeb14fb8e5e046b6afcc5b19e04218bcacd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/601455
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/internal/trace/base.go b/src/internal/trace/base.go
index 4cbd3e64f1007cfe83c3bbf24a106fff8a89dfd5..4f4ce486305434deb3be100f4cecbcba00c4ea79 100644 (file)
--- a/src/internal/trace/base.go
+++ b/src/internal/trace/base.go
@@ -19,11 +19,7 @@ import (
 
 // maxArgs is the maximum number of arguments for "plain" events,
 // i.e. anything that could reasonably be represented as a baseEvent.
-//
-// TODO(mknyszek): This is only 6 instead of 5 because GoStatusStack
-// has 5 arguments and needs to smuggle in a 6th. Figure out a way to
-// shrink this in the future.
-const maxArgs = 6
+const maxArgs = 5
 
 // timedEventArgs is an array that is able to hold the arguments for any
 // timed event.

diff --git a/src/internal/trace/event.go b/src/internal/trace/event.go
index a5d5637e60da063afebfd22a6defa66087629757..4c80a7e5ec4fd7d17b84589449e1396b6f88a428 100644 (file)
--- a/src/internal/trace/event.go
+++ b/src/internal/trace/event.go
@@ -647,8 +647,9 @@ func (e Event) StateTransition() StateTransition {
                s = goStateTransition(e.ctx.G, GoSyscall, GoRunnable)
                s.Stack = e.Stack() // This event references the resource the event happened on.
        case go122.EvGoStatus, go122.EvGoStatusStack:
-               // N.B. ordering.advance populates e.base.extra.
-               s = goStateTransition(GoID(e.base.args[0]), GoState(e.base.extra(version.Go122)[0]), go122GoStatus2GoState[e.base.args[2]])
+               packedStatus := e.base.args[2]
+               from, to := packedStatus>>32, packedStatus&((1<<32)-1)
+               s = goStateTransition(GoID(e.base.args[0]), GoState(from), go122GoStatus2GoState[to])
        default:
                panic(fmt.Sprintf("internal error: unexpected event type for StateTransition kind: %s", go122.EventString(e.base.typ)))
        }

diff --git a/src/internal/trace/order.go b/src/internal/trace/order.go
index 4b3b8029fd293a63189434b8fb87a8a7cbecee82..d0818a500c03c8b7b14bb7d6242996cc2f8320ed 100644 (file)
--- a/src/internal/trace/order.go
+++ b/src/internal/trace/order.go
@@ -377,7 +377,7 @@ func (o *ordering) advanceGoStatus(ev *baseEvent, evt *evTable, m ThreadID, gen
        } else {
                return curCtx, false, fmt.Errorf("found goroutine status for new goroutine after the first generation: id=%v status=%v", gid, status)
        }
-       ev.extra(version.Go122)[0] = uint64(oldState) // Smuggle in the old state for StateTransition.
+       ev.args[2] = uint64(oldState)<<32 | uint64(status) // Smuggle in the old state for StateTransition.
 
        newCtx := curCtx
        switch status {