From 08134552a49da645b3c08f8f748be37ed6891dcb Mon Sep 17 00:00:00 2001 From: vyzo Date: Fri, 24 Mar 2023 15:48:58 +0200 Subject: [PATCH] address review comments --- chain/consensus/compute_state.go | 1 + chain/gen/genesis/miners.go | 1 + chain/stmgr/call.go | 1 + chain/vm/execution.go | 14 ++++++++++---- chain/vm/vmi.go | 2 +- metrics/metrics.go | 6 +++--- 6 files changed, 17 insertions(+), 8 deletions(-) diff --git a/chain/consensus/compute_state.go b/chain/consensus/compute_state.go index cf05d612d..3c1bab9ca 100644 --- a/chain/consensus/compute_state.go +++ b/chain/consensus/compute_state.go @@ -196,6 +196,7 @@ func (t *TipSetExecutor) ApplyBlocks(ctx context.Context, cronGas = 0 partDone = metrics.Timer(ctx, metrics.VMApplyMessages) + // TODO reorg the code to minimize the execution critical section vmi, err := makeVm(pstate, epoch, ts.MinTimestamp()) if err != nil { return cid.Undef, cid.Undef, xerrors.Errorf("making vm: %w", err) diff --git a/chain/gen/genesis/miners.go b/chain/gen/genesis/miners.go index 900389f56..09b46a6e7 100644 --- a/chain/gen/genesis/miners.go +++ b/chain/gen/genesis/miners.go @@ -108,6 +108,7 @@ func SetupStorageMiners(ctx context.Context, cs *store.ChainStore, sys vm.Syscal if err != nil { return cid.Undef, fmt.Errorf("creating vm: %w", err) } + // Note: genesisVm is mutated, so this has to happen in a deferred func; go horror show. defer func() { genesisVm.Done() }() if len(miners) == 0 { diff --git a/chain/stmgr/call.go b/chain/stmgr/call.go index 9633d6417..8e18c25df 100644 --- a/chain/stmgr/call.go +++ b/chain/stmgr/call.go @@ -159,6 +159,7 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr if err != nil { return nil, xerrors.Errorf("failed to set up vm: %w", err) } + // Note: vmi is mutated, so this has to happen in a deferred func; go horror show. defer func() { vmi.Done() }() for i, m := range priorMsgs { diff --git a/chain/vm/execution.go b/chain/vm/execution.go index d984d0095..0edb13884 100644 --- a/chain/vm/execution.go +++ b/chain/vm/execution.go @@ -17,8 +17,14 @@ import ( ) const ( + // DefaultAvailableExecutionLanes is the number of available execution lanes; it is the bound of + // concurrent active executions. + // This is the default value in filecoin-ffi DefaultAvailableExecutionLanes = 4 - DefaultPriorityExecutionLanes = 2 + // DefaultPriorityExecutionLanes is the number of reserved execution lanes for priority computations. + // This is purely userspace, but we believe it is a reasonable default, even with more available + // lanes. + DefaultPriorityExecutionLanes = 2 ) var ErrExecutorDone = errors.New("executor has been released") @@ -118,7 +124,7 @@ func (e *executionEnv) getToken(lane ExecutionLane) *executionToken { e.available-- - metricsUp(metrics.VMExecutionActive, lane) + metricsUp(metrics.VMExecutionRunning, lane) return &executionToken{lane: lane, reserved: 0} case ExecutionLanePriority: @@ -134,7 +140,7 @@ func (e *executionEnv) getToken(lane ExecutionLane) *executionToken { reserving = 1 } - metricsUp(metrics.VMExecutionActive, lane) + metricsUp(metrics.VMExecutionRunning, lane) return &executionToken{lane: lane, reserved: reserving} default: @@ -152,7 +158,7 @@ func (e *executionEnv) putToken(token *executionToken) { e.cond.Broadcast() - metricsDown(metrics.VMExecutionActive, token.lane) + metricsDown(metrics.VMExecutionRunning, token.lane) } func metricsUp(metric *stats.Int64Measure, lane ExecutionLane) { diff --git a/chain/vm/vmi.go b/chain/vm/vmi.go index e5d5daff8..7aa52b585 100644 --- a/chain/vm/vmi.go +++ b/chain/vm/vmi.go @@ -37,7 +37,7 @@ type Interface interface { Flush(ctx context.Context) (cid.Cid, error) } -// Executor is the general vm execution interface, which is prioritized according to execution langes. +// Executor is the general vm execution interface, which is prioritized according to execution lanes. // User must call Done when it is done with this executor to release resource holds by the execution // environment type Executor interface { diff --git a/metrics/metrics.go b/metrics/metrics.go index 61bd86fbd..bd1295d17 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -125,7 +125,7 @@ var ( VMSends = stats.Int64("vm/sends", "Counter for sends processed by the VM", stats.UnitDimensionless) VMApplied = stats.Int64("vm/applied", "Counter for messages (including internal messages) processed by the VM", stats.UnitDimensionless) VMExecutionWaiting = stats.Int64("vm/execution_waiting", "Counter for VM executions waiting to be assigned to a lane", stats.UnitDimensionless) - VMExecutionActive = stats.Int64("vm/execution_default", "Counter for active VM executions", stats.UnitDimensionless) + VMExecutionRunning = stats.Int64("vm/execution_running", "Counter for running VM executions", stats.UnitDimensionless) // miner WorkerCallsStarted = stats.Int64("sealing/worker_calls_started", "Counter of started worker tasks", stats.UnitDimensionless) @@ -373,8 +373,8 @@ var ( Aggregation: view.LastValue(), TagKeys: []tag.Key{ExecutionLane}, } - VMExecutionActiveView = &view.View{ - Measure: VMExecutionActive, + VMExecutionRunningView = &view.View{ + Measure: VMExecutionRunning, Aggregation: view.LastValue(), TagKeys: []tag.Key{ExecutionLane}, }