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

perf: use ArrayVec for Stack #14

Closed
wants to merge 1 commit into from
Closed

Conversation

mooori
Copy link

@mooori mooori commented Jul 18, 2022

Changes the type of field Stack.data from Vec<U256> to ArrayVec<U256, 1024>.
This optimization was proposed in #448 and is based on the limit of 1024
items for the EVM stack.

Change of a public interface

Stack::data() returns &[U256] instead of &Vec<U256>.

NEAR gas usage

There a few tests in aurora-engine which fail due to gas usage lower than
expected:

---- tests::repro::repro_8ru7VEA stdout ----
thread 'tests::repro::repro_8ru7VEA' panicked at 'assertion failed: `(left == right)`
  left: `242`,
 right: `240`', engine-tests/src/tests/repro.rs:146:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- tests::repro::repro_D98vwmi stdout ----
thread 'tests::repro::repro_D98vwmi' panicked at 'assertion failed: `(left == right)`
  left: `199`,
 right: `198`', engine-tests/src/tests/repro.rs:146:5

---- tests::repro::repro_5bEgfRQ stdout ----
thread 'tests::repro::repro_5bEgfRQ' panicked at 'assertion failed: `(left == right)`
  left: `706`,
 right: `705`', engine-tests/src/tests/repro.rs:146:5

---- tests::repro::repro_FRcorNv stdout ----
thread 'tests::repro::repro_FRcorNv' panicked at 'assertion failed: `(left == right)`
  left: `197`,
 right: `196`', engine-tests/src/tests/repro.rs:146:5

---- tests::repro::repro_GdASJ3KESs stdout ----
thread 'tests::repro::repro_GdASJ3KESs' panicked at 'assertion failed: `(left == right)`
  left: `133`,
 right: `132`', engine-tests/src/tests/repro.rs:146:5

---- tests::uniswap::test_uniswap_input_multihop stdout ----
thread 'tests::uniswap::test_uniswap_input_multihop' panicked at 'assertion failed: `(left == right)`
  left: `123`,
 right: `122`', engine-tests/src/tests/uniswap.rs:41:5

Issue with jsontests

This change causes a stack overflow in evm-tests/jsontests. Identifying
the cause of a stack overflow in Rust can be tricky. Therefore, before
trying to get to the bottom of this, it might make sense to evaluate if this
change could/would be merged at all?

Note: It seems like the latest jsontests commit suitable for Aurora's EVM fork
is c66b562.

@birchmd
Copy link
Member

birchmd commented Jul 18, 2022

Thanks for looking into this @mooori . In my opinion, it is probably not worth trying to figure out what goes wrong in the jsontests for a less than 1% gas savings. However, I do think it would be worth getting our fork up to date with the latest changes on the upstream sputnikvm project so that we can use the latest jsontests as well.

@mooori
Copy link
Author

mooori commented Jul 18, 2022

I will look into getting the fork up to date with upstream.

@mooori mooori closed this Jul 27, 2022
aleksuss added a commit to aleksuss/sputnikvm that referenced this pull request Jul 3, 2024
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