From 66eddc1b1b357f7919be471dfedb5f985bf5bfa7 Mon Sep 17 00:00:00 2001 From: Manav Darji Date: Wed, 15 Feb 2023 12:32:58 +0530 Subject: [PATCH] Revert "Event based pprof (#732)" This reverts commit 22fa4033e8fabb51c44e8d2a8c6bb695a6e9285e. --- common/context.go | 32 --------- common/context_test.go | 107 ---------------------------- common/set/slice.go | 17 ----- eth/tracers/api.go | 2 +- internal/ethapi/api.go | 91 +---------------------- internal/ethapi/transaction_args.go | 2 +- 6 files changed, 4 insertions(+), 247 deletions(-) delete mode 100644 common/context.go delete mode 100644 common/context_test.go diff --git a/common/context.go b/common/context.go deleted file mode 100644 index 1f44cf97ae..0000000000 --- a/common/context.go +++ /dev/null @@ -1,32 +0,0 @@ -package common - -import ( - "context" - - unique "github.com/ethereum/go-ethereum/common/set" -) - -type key struct{} - -var ( - labelsKey key -) - -func WithLabels(ctx context.Context, labels ...string) context.Context { - if len(labels) == 0 { - return ctx - } - - labels = append(labels, Labels(ctx)...) - - return context.WithValue(ctx, labelsKey, unique.Deduplicate(labels)) -} - -func Labels(ctx context.Context) []string { - labels, ok := ctx.Value(labelsKey).([]string) - if !ok { - return nil - } - - return labels -} diff --git a/common/context_test.go b/common/context_test.go deleted file mode 100644 index bc093a3dca..0000000000 --- a/common/context_test.go +++ /dev/null @@ -1,107 +0,0 @@ -package common - -import ( - "context" - "reflect" - "sort" - "testing" -) - -func TestWithLabels(t *testing.T) { - t.Parallel() - - cases := []struct { - name string - initial []string - new []string - expected []string - }{ - { - "nil-nil", - nil, - nil, - nil, - }, - - { - "nil-something", - nil, - []string{"one", "two"}, - []string{"one", "two"}, - }, - - { - "something-nil", - []string{"one", "two"}, - nil, - []string{"one", "two"}, - }, - - { - "something-something", - []string{"one", "two"}, - []string{"three", "four"}, - []string{"one", "two", "three", "four"}, - }, - - // deduplication - { - "with duplicates nil-something", - nil, - []string{"one", "two", "one"}, - []string{"one", "two"}, - }, - - { - "with duplicates something-nil", - []string{"one", "two", "one"}, - nil, - []string{"one", "two"}, - }, - - { - "with duplicates something-something", - []string{"one", "two"}, - []string{"three", "one"}, - []string{"one", "two", "three"}, - }, - - { - "with duplicates something-something", - []string{"one", "two", "three"}, - []string{"three", "four", "two"}, - []string{"one", "two", "three", "four"}, - }, - } - - for _, c := range cases { - c := c - - t.Run(c.name, func(t *testing.T) { - t.Parallel() - - ctx := context.Background() - - ctx = WithLabels(ctx, c.initial...) - ctx = WithLabels(ctx, c.new...) - - got := Labels(ctx) - - if len(got) != len(c.expected) { - t.Errorf("case %s. expected %v, got %v", c.name, c.expected, got) - - return - } - - gotSorted := sort.StringSlice(got) - gotSorted.Sort() - - expectedSorted := sort.StringSlice(c.expected) - expectedSorted.Sort() - - if !reflect.DeepEqual(gotSorted, expectedSorted) { - t.Errorf("case %s. expected %v, got %v", c.name, expectedSorted, gotSorted) - } - }) - } -} diff --git a/common/set/slice.go b/common/set/slice.go index eda4dda23b..36f11e67fe 100644 --- a/common/set/slice.go +++ b/common/set/slice.go @@ -9,20 +9,3 @@ func New[T comparable](slice []T) map[T]struct{} { return m } - -func ToSlice[T comparable](m map[T]struct{}) []T { - slice := make([]T, len(m)) - - var i int - - for k := range m { - slice[i] = k - i++ - } - - return slice -} - -func Deduplicate[T comparable](slice []T) []T { - return ToSlice(New(slice)) -} diff --git a/eth/tracers/api.go b/eth/tracers/api.go index ce7b36b906..13f5c627cd 100644 --- a/eth/tracers/api.go +++ b/eth/tracers/api.go @@ -1052,7 +1052,7 @@ func (api *API) TraceCall(ctx context.Context, args ethapi.TransactionArgs, bloc } } // Execute the trace - msg, err := args.ToMessage(ctx, api.backend.RPCGasCap(), block.BaseFee()) + msg, err := args.ToMessage(api.backend.RPCGasCap(), block.BaseFee()) if err != nil { return nil, err } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index f5953f59c3..49b1610987 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -21,11 +21,7 @@ import ( "errors" "fmt" "math/big" - "os" - "path/filepath" - "runtime/pprof" "strings" - "sync" "time" "github.com/davecgh/go-spew/spew" @@ -1013,7 +1009,7 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash defer cancel() // Get a new instance of the EVM. - msg, err := args.ToMessage(ctx, globalGasCap, header.BaseFee) + msg, err := args.ToMessage(globalGasCap, header.BaseFee) if err != nil { return nil, err } @@ -1036,83 +1032,15 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash } // If the timer caused an abort, return an appropriate error message - timeoutMu.Lock() if evm.Cancelled() { - timeoutErrors++ - - if timeoutErrors >= pprofThreshold { - timeoutNoErrors = 0 - - if !isRunning { - runProfile() - } - - log.Warn("[eth_call] timeout", - "timeoutErrors", timeoutErrors, - "timeoutNoErrors", timeoutNoErrors, - "args", args, - "blockNrOrHash", blockNrOrHash, - "overrides", overrides, - "timeout", timeout, - "globalGasCap", globalGasCap) - } - - timeoutMu.Unlock() - return nil, fmt.Errorf("execution aborted (timeout = %v)", timeout) - } else { - if timeoutErrors >= pprofStopThreshold { - timeoutErrors = 0 - timeoutNoErrors = 0 - - if isRunning { - pprof.StopCPUProfile() - isRunning = false - } - } - } - - if isRunning && time.Since(pprofTime) >= pprofDuration { - timeoutErrors = 0 - timeoutNoErrors = 0 - - pprof.StopCPUProfile() - - isRunning = false } - - timeoutMu.Unlock() - if err != nil { return result, fmt.Errorf("err: %w (supplied gas %d)", err, msg.Gas()) } - return result, nil } -func runProfile() { - pprofTime = time.Now() - - name := fmt.Sprintf("profile_eth_call-count-%d-time-%s.prof", - number, pprofTime.Format("2006-01-02-15-04-05")) - - name = filepath.Join(os.TempDir(), name) - - f, err := os.Create(name) - if err != nil { - log.Error("[eth_call] can't create profile file", "name", name, "err", err) - return - } - - if err = pprof.StartCPUProfile(f); err != nil { - log.Error("[eth_call] can't start profiling", "name", name, "err", err) - return - } - - isRunning = true - number++ -} - func newRevertError(result *core.ExecutionResult) *revertError { reason, errUnpack := abi.UnpackRevert(result.Revert()) err := errors.New("execution reverted") @@ -1143,21 +1071,6 @@ func (e *revertError) ErrorData() interface{} { return e.reason } -var ( - number int - timeoutErrors int // count for timeout errors - timeoutNoErrors int - timeoutMu sync.Mutex - isRunning bool - pprofTime time.Time -) - -const ( - pprofThreshold = 3 - pprofStopThreshold = 3 - pprofDuration = time.Minute -) - // Call executes the given transaction on the state for the given block number. // // Additionally, the caller can specify a batch of contract for fields overriding. @@ -1664,7 +1577,7 @@ func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrH statedb := db.Copy() // Set the accesslist to the last al args.AccessList = &accessList - msg, err := args.ToMessage(ctx, b.RPCGasCap(), header.BaseFee) + msg, err := args.ToMessage(b.RPCGasCap(), header.BaseFee) if err != nil { return nil, 0, nil, err } diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index a8f0b2cde9..aa2596fe81 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -197,7 +197,7 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { // ToMessage converts the transaction arguments to the Message type used by the // core evm. This method is used in calls and traces that do not require a real // live transaction. -func (args *TransactionArgs) ToMessage(_ context.Context, globalGasCap uint64, baseFee *big.Int) (types.Message, error) { +func (args *TransactionArgs) ToMessage(globalGasCap uint64, baseFee *big.Int) (types.Message, error) { // Reject invalid combinations of pre- and post-1559 fee styles if args.GasPrice != nil && (args.MaxFeePerGas != nil || args.MaxPriorityFeePerGas != nil) { return types.Message{}, errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified")