Change the return type of step_inner function.

[aoli-al]

This PR simplifies the return result of step_inner function.

Using Result<Never, InterpreterResult> as return result will cause
unnecessary overhead (mostly additional memory operations). Since
step_inner is a hot function, I propose to simplify the return type of
step_inner function and handle the error results manually.

Some benchmark results I have:

Without Optimization (Original Version):
test rng_u256 ... bench: 68,395,922 ns/iter (+/- 19,520,348)
test rng_usize ... bench: 46,822,703 ns/iter (+/- 19,869,026)
test simple_loop_u256 ... bench: 18,807,019 ns/iter (+/- 5,683,907)
test simple_loop_usize ... bench: 13,812,568 ns/iter (+/- 3,310,862)

With Optimization:
test rng_u256 ... bench: 53,654,255 ns/iter (+/- 12,985,727)
test rng_usize ... bench: 36,481,886 ns/iter (+/- 13,044,046)
test simple_loop_u256 ... bench: 15,010,101 ns/iter (+/- 5,306,132)
test simple_loop_usize ... bench: 10,584,103 ns/iter (+/- 2,439,375)

Some perf results I have:

[parity-cla-bot]

It looks like @Leeleo3x hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

[aoli-al]

[clabot:check]

[parity-cla-bot]

It looks like @Leeleo3x signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

[dvdplm]

Do you mind including the benchmark code in the PR?

Removing the error conversion and all uses of ? makes the code less readable. Can you elaborate on why the return type change necessitates changing the error handling like this?

[aoli-al]

The benchmark I'm using is included in current Parity code base. To run benchmarks:

cargo bench --package evmbin

I do understand ? operator makes the code more readable. However, the ? operator calls into_result function and wraps the result into Result type which causes additional memory operations. This is also because the size of InterpreterResult is large and need to be stored in memory to be moved around.

[ordian]

Do you mind including the benchmark code in the PR?

I think the benches are from evmbin/benches/mod.rs.

Removing the error conversion and all uses of ? makes the code less readable. Can you elaborate on why the return type change necessitates changing the error handling like this?

That's a known issue rust-lang/rust#37939

[dvdplm]

That's a known issue rust-lang/rust#37939

I had no idea, thank you for educating me.