Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize evmloop #19203

Closed
wants to merge 6 commits into from
Closed

Optimize evmloop #19203

wants to merge 6 commits into from

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Mar 4, 2019

This PR contains four separate optimizations to the evm runloop. For review, it's probably simplest to verify each commit separately, since they are completely standalone, and some larger than others. For those, I've tried to make them as easy to verify as possible, by making the changes verifiable side-by-side with the original code.

The changes are:

1. Remove function call for stack validation

Previously, we always called operation.validateStack. Since operation.validateStack is a pointer, this call cannot be inlined by the compiler. It has been replaced with minStack and maxStack on the operation, which can be checked immediately in the runloop.

Additionally, I removed the call to enforceRestrictions, by inlining it.

2. Separate gas calculation into constant and dynamic

Previously, we always called operation.gasCost(in.gasTable, in.evm, contract, stack, mem, memorySize) (again, cannot be inlined).

For a lot of operations, this method only returned a constant. I instead added the constant gas values to those operations, which can be checked immediately. This makes a difference particularly for all cheap operations which have no dynamic part, which are the majority of the operations.

3 Optimize push1

This adds a custom opPush1 method for push1, where we can convert a byte directly into a uint64, and don't have to do padding which the generic method does.

4 Reuse pooled bigints for ADDRESS, ORIGIN, and CALLER

This adds a convenience-method to Address to convert into an existing bigint, and the ops above can thus use the int pool instead of allocating new bigints.

5 Avoid string interpolation for jump errors

On a trial run with metrics, it was found that the second largest time-sink was 7m30s in JUMPI (1m31s for JUMP). Now, since solidity used to do intentional invalid jumps for abort all the time, most of that time is probably spent on fmt.Errorf("invalid jump destination (%v) %v", nop, pos).

This PR replaces the string interpolation with a generic errInvalidJump error.

@holiman holiman requested a review from karalabe as a code owner March 4, 2019 09:04
@holiman holiman force-pushed the optimize_evmloop branch from b7ef92b to e5945d5 Compare March 4, 2019 09:51
}
// Static portion of gas
if !contract.UseGas(operation.constantGas) {
return nil, ErrOutOfGas
}

var memorySize uint64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comment saying memory check must be before gas check to prevent overflow in dynamic gas calculation.

@@ -886,6 +886,21 @@ func opSuicide(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memo
return nil, nil
}

// opPush1 is a specialized version of pushN
func opPush1(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move next to makePush

@@ -405,7 +405,7 @@ func opSha3(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory
}

func opAddress(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
stack.push(contract.Address().Big())
stack.push(contract.Address().SetBig(interpreter.intPool.get()))
Copy link
Contributor

@fjl fjl Mar 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternative: stack.push(interpreter.intPool.get().SetBytes(contract.Address().Bytes()))

@@ -205,6 +205,9 @@ func (a Address) Bytes() []byte { return a[:] }
// Big converts an address to a big integer.
func (a Address) Big() *big.Int { return new(big.Int).SetBytes(a[:]) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove Big and SetBig

@holiman
Copy link
Contributor Author

holiman commented Mar 11, 2019

Superseded by #19210 , fixes applied there

@holiman holiman closed this Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants