riteshghorse commented on a change in pull request #15657:
URL: https://github.com/apache/beam/pull/15657#discussion_r731375722
##########
File path: sdks/go/pkg/beam/core/metrics/store.go
##########
@@ -174,36 +174,35 @@ type executionTracker struct {
numberOfTransitions int64
millisSinceLastTransition time.Duration
transitionsAtLastSample int64
-}
-
-// executionState is used to store as atomic.Value in Store.
-type pTransformState struct {
- pid string
- state bundleProcState
-}
-
-func newPTransformState(pid string, state bundleProcState) *pTransformState {
- return &pTransformState{pid: pid, state: state}
+ currentState bundleProcState
+ pid string
}
func SetPTransformState(ctx context.Context, state bundleProcState) {
if bctx, ok := ctx.(*beamCtx); ok {
- ps := newPTransformState(bctx.ptransformID, state)
- bctx.pStore.Store(ps)
- atomic.AddInt64(&bctx.transitions, 1)
+ pid := bctx.ptransformID
+ bctx.store.mu.Lock()
+ bctx.store.executionStore.pid = pid
+ bctx.store.executionStore.currentState = state
+ bctx.store.mu.Unlock()
+ atomic.AddInt64(&bctx.store.executionStore.numberOfTransitions,
1)
Review comment:
`allocs/op` changed significantly (from 2 to 12) when I used
atomic.Value to store and load BundleState into it. Also, the time per loop
changed from 2566 ns/op to 3376 ns/op.
Did this benchmarking without pprof. Now I'll do it with ppro.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
