fix bug and optimize fill_trace logic for gc (ethereum#104)

* fix bug and upgrade trace code

* minor

* refactor (ethereum#105)

* move

* add comments

* fix typo

* update comments

* rename extraData()

* update comments

Co-authored-by: HAOYUatHZ <[email protected]>
Co-authored-by: HAOYUatHZ <[email protected]>

Author: mask-pp

core/types/l2trace.go

@@ -1,10 +1,25 @@
 package types
 
 import (
+	"runtime"
+	"sync"
+
 	"github.com/scroll-tech/go-ethereum/common"
 	"github.com/scroll-tech/go-ethereum/common/hexutil"
 )
 
+var (
+	loggerResPool = sync.Pool{
+		New: func() interface{} {
+			// init arrays here; other types are inited with default values
+			return &StructLogRes{
+				Stack:  []string{},
+				Memory: []string{},
+			}
+		},
+	}
+)
+
 // BlockResult contains block execution traces and results required for rollers.
 type BlockResult struct {
 	BlockTrace       *BlockTrace        `json:"blockTrace"`
@@ -25,8 +40,8 @@ type ExecutionResult struct {
 	// It's exist only when tx is a contract call.
 	CodeHash *common.Hash `json:"codeHash,omitempty"`
 	// If it is a contract call, the contract code is returned.
-	ByteCode   string         `json:"byteCode,omitempty"`
-	StructLogs []StructLogRes `json:"structLogs"`
+	ByteCode   string          `json:"byteCode,omitempty"`
+	StructLogs []*StructLogRes `json:"structLogs"`
 }
 
 // StructLogRes stores a structured log emitted by the EVM while replaying a
@@ -37,14 +52,30 @@ type StructLogRes struct {
 	Gas           uint64            `json:"gas"`
 	GasCost       uint64            `json:"gasCost"`
 	Depth         int               `json:"depth"`
-	Error         string            `json:"error,omitempty"`
+	Error         error             `json:"error,omitempty"`
 	Stack         []string          `json:"stack,omitempty"`
 	Memory        []string          `json:"memory,omitempty"`
 	Storage       map[string]string `json:"storage,omitempty"`
 	RefundCounter uint64            `json:"refund,omitempty"`
 	ExtraData     *ExtraData        `json:"extraData,omitempty"`
 }
 
+// Basic StructLogRes skeleton, Stack&Memory&Storage&ExtraData are separated from it for GC optimization;
+// still need to fill in with Stack&Memory&Storage&ExtraData
+func NewStructLogResBasic(pc uint64, op string, gas, gasCost uint64, depth int, refundCounter uint64, err error) *StructLogRes {
+	logRes := loggerResPool.Get().(*StructLogRes)
+	logRes.Pc, logRes.Op, logRes.Gas, logRes.GasCost, logRes.Depth, logRes.RefundCounter = pc, op, gas, gasCost, depth, refundCounter
+	logRes.Error = err
+	runtime.SetFinalizer(logRes, func(logRes *StructLogRes) {
+		logRes.Stack = logRes.Stack[:0]
+		logRes.Memory = logRes.Memory[:0]
+		logRes.Storage = nil
+		logRes.ExtraData = nil
+		loggerResPool.Put(logRes)
+	})
+	return logRes
+}
+
 type ExtraData struct {
 	// CALL | CALLCODE | DELEGATECALL | STATICCALL: [tx.to address’s code, stack.nth_last(1) address’s code]
 	CodeList [][]byte `json:"codeList,omitempty"`
@@ -73,30 +104,3 @@ type StorageProofWrapper struct {
 	Value string   `json:"value,omitempty"`
 	Proof []string `json:"proof,omitempty"`
 }
-
-// NewExtraData create, init and return ExtraData
-func NewExtraData() *ExtraData {
-	return &ExtraData{
-		CodeList:  make([][]byte, 0),
-		ProofList: make([]*AccountProofWrapper, 0),
-	}
-}
-
-func (e *ExtraData) Clean() {
-	e.CodeList = e.CodeList[:0]
-	e.ProofList = e.ProofList[:0]
-}
-
-// SealExtraData doesn't show empty fields.
-func (e *ExtraData) SealExtraData() *ExtraData {
-	if len(e.CodeList) == 0 {
-		e.CodeList = nil
-	}
-	if len(e.ProofList) == 0 {
-		e.ProofList = nil
-	}
-	if e.CodeList == nil && e.ProofList == nil {
-		return nil
-	}
-	return e
-}

core/vm/logger.go

@@ -85,15 +85,14 @@ var (
 	loggerPool = sync.Pool{
 		New: func() interface{} {
 			return &StructLog{
-				Stack:     make([]uint256.Int, 0),
-				ExtraData: types.NewExtraData(),
+				// init arrays here; other types are inited with default values
+				Stack: make([]uint256.Int, 0),
 			}
 		},
 	}
 )
 
 func NewStructlog(pc uint64, op OpCode, gas, cost uint64, depth int) *StructLog {
-
 	structlog := loggerPool.Get().(*StructLog)
 	structlog.Pc, structlog.Op, structlog.Gas, structlog.GasCost, structlog.Depth = pc, op, gas, cost, depth
 
@@ -109,7 +108,14 @@ func (s *StructLog) clean() {
 	s.Stack = s.Stack[:0]
 	s.ReturnData.Reset()
 	s.Storage = nil
-	s.ExtraData.Clean()
+	s.ExtraData = nil
+}
+
+func (s *StructLog) getOrInitExtraData() *types.ExtraData {
+	if s.ExtraData == nil {
+		s.ExtraData = &types.ExtraData{}
+	}
+	return s.ExtraData
 }
 
 // overrides for gencodec
@@ -236,7 +242,7 @@ func (l *StructLogger) CaptureState(pc uint64, op OpCode, gas, cost uint64, scop
 		l.storage[contractAddress][storageKey] = storageValue
 		structlog.Storage = l.storage[contractAddress].Copy()
 
-		if err := traceStorageProof(l, scope, structlog.ExtraData); err != nil {
+		if err := traceStorageProof(l, scope, structlog.getOrInitExtraData()); err != nil {
 			log.Error("Failed to trace data", "opcode", op.String(), "err", err)
 		}
 	}
@@ -247,7 +253,7 @@ func (l *StructLogger) CaptureState(pc uint64, op OpCode, gas, cost uint64, scop
 	if ok {
 		// execute trace func list.
 		for _, exec := range execFuncList {
-			if err = exec(l, scope, structlog.ExtraData); err != nil {
+			if err = exec(l, scope, structlog.getOrInitExtraData()); err != nil {
 				log.Error("Failed to trace data", "opcode", op.String(), "err", err)
 			}
 		}
@@ -441,63 +447,28 @@ func (t *mdLogger) CaptureEnter(typ OpCode, from common.Address, to common.Addre
 
 func (t *mdLogger) CaptureExit(output []byte, gasUsed uint64, err error) {}
 
-var (
-	formatPool = sync.Pool{
-		New: func() interface{} {
-			return make([]types.StructLogRes, 0, 128)
-		},
-	}
-)
-
 // FormatLogs formats EVM returned structured logs for json output
-func FormatLogs(logs []StructLog) []types.StructLogRes {
-	formatted := formatPool.Get().([]types.StructLogRes)
-	runtime.SetFinalizer(&formatted, func(format *[]types.StructLogRes) {
-		for _, res := range *format {
-			res.ExtraData = nil
-			res.Storage = nil
-			res.Stack = res.Stack[:0]
-			res.Memory = res.Memory[:0]
-		}
-		formatPool.Put(*format)
-	})
+func FormatLogs(logs []StructLog) []*types.StructLogRes {
+	formatted := make([]*types.StructLogRes, 0, len(logs))
 
-	for index, trace := range logs {
-		formatted = append(formatted, types.StructLogRes{
-			Pc:            trace.Pc,
-			Op:            trace.Op.String(),
-			Gas:           trace.Gas,
-			GasCost:       trace.GasCost,
-			Depth:         trace.Depth,
-			RefundCounter: trace.RefundCounter,
-			Error:         trace.ErrorString(),
-		})
-		if len(trace.Stack) != 0 {
-			if formatted[index].Stack == nil {
-				formatted[index].Stack = make([]string, 0, len(trace.Stack))
-			}
-			for _, stackValue := range trace.Stack {
-				formatted[index].Stack = append(formatted[index].Stack, stackValue.Hex())
-			}
+	for _, trace := range logs {
+		logRes := types.NewStructLogResBasic(trace.Pc, trace.Op.String(), trace.Gas, trace.GasCost, trace.Depth, trace.RefundCounter, trace.Err)
+		for _, stackValue := range trace.Stack {
+			logRes.Stack = append(logRes.Stack, stackValue.Hex())
 		}
-		if trace.Memory.Len() != 0 {
-			if formatted[index].Memory == nil {
-				formatted[index].Memory = make([]string, 0, (trace.Memory.Len()+31)/32)
-			}
-			for i := 0; i+32 <= trace.Memory.Len(); i += 32 {
-				formatted[index].Memory = append(formatted[index].Memory, common.Bytes2Hex(trace.Memory.Bytes()[i:i+32]))
-			}
+		for i := 0; i+32 <= trace.Memory.Len(); i += 32 {
+			logRes.Memory = append(logRes.Memory, common.Bytes2Hex(trace.Memory.Bytes()[i:i+32]))
 		}
 		if len(trace.Storage) != 0 {
 			storage := make(map[string]string)
 			for i, storageValue := range trace.Storage {
 				storage[i.Hex()] = storageValue.Hex()
 			}
-			formatted[index].Storage = storage
-		}
-		if trace.ExtraData != nil {
-			formatted[index].ExtraData = trace.ExtraData.SealExtraData()
+			logRes.Storage = storage
 		}
+		logRes.ExtraData = trace.ExtraData
+
+		formatted = append(formatted, logRes)
 	}
 	return formatted
 }

eth/tracers/api_test.go

@@ -217,7 +217,7 @@ func TestTraceCall(t *testing.T) {
 				Gas:         params.TxGas,
 				Failed:      false,
 				ReturnValue: "",
-				StructLogs:  []types.StructLogRes{},
+				StructLogs:  []*types.StructLogRes{},
 			},
 		},
 		// Standard JSON trace upon the head, plain transfer.
@@ -234,7 +234,7 @@ func TestTraceCall(t *testing.T) {
 				Gas:         params.TxGas,
 				Failed:      false,
 				ReturnValue: "",
-				StructLogs:  []types.StructLogRes{},
+				StructLogs:  []*types.StructLogRes{},
 			},
 		},
 		// Standard JSON trace upon the non-existent block, error expects
@@ -263,7 +263,7 @@ func TestTraceCall(t *testing.T) {
 				Gas:         params.TxGas,
 				Failed:      false,
 				ReturnValue: "",
-				StructLogs:  []types.StructLogRes{},
+				StructLogs:  []*types.StructLogRes{},
 			},
 		},
 		// Standard JSON trace upon the pending block
@@ -280,7 +280,7 @@ func TestTraceCall(t *testing.T) {
 				Gas:         params.TxGas,
 				Failed:      false,
 				ReturnValue: "",
-				StructLogs:  []types.StructLogRes{},
+				StructLogs:  []*types.StructLogRes{},
 			},
 		},
 	}
@@ -333,7 +333,7 @@ func TestTraceTransaction(t *testing.T) {
 		Gas:         params.TxGas,
 		Failed:      false,
 		ReturnValue: "",
-		StructLogs:  []types.StructLogRes{},
+		StructLogs:  []*types.StructLogRes{},
 	}) {
 		t.Error("Transaction tracing result is different")
 	}