Vyper: State leakage across test runs when using parrellization

[pipermerriam]

What is wrong.

The tests at tests/examples/safe_remote_purchase/test_safe_remote_purchase.py fail when run using pytest-xdist to parallelize test runs.

    def test_abort(w3, assert_tx_failed, check_balance, get_contract, contract_code):
        a0, a1, a2 = w3.eth.accounts[:3]
        c = get_contract(contract_code, value=2)
        # Only sender can trigger refund
        assert_tx_failed(lambda: c.abort(transact={'from': a2}))
        # Refund works correctly
        c.abort(transact={'from': a0, 'gasPrice': 0})
>       assert check_balance() == (INIT_BAL_a0 - w3.toWei(2, 'ether'), INIT_BAL_a1)
E       assert (100000000000...0000000000000) == (9999980000000...0000000000000)
E         At index 0 diff: 1000000000000000000000000 != 999998000000000000000000
E         Use -v to get the full diff

tests/examples/safe_remote_purchase/test_safe_remote_purchase.py:62: AssertionError

replicate by installing pytest-xdist and running with

pytest tests/examples/safe_remote_purchase/test_safe_remote_purchase.py -n 2

It's likely this isn't deterministic and you may need to run the full suite.

How can it be fixed.

Figure out where statefulness is leaking across test runs and fix it.

[pipermerriam]

Updating the w3 and tester fixtures in tests/conftest.py did not remedy this.

[fubuloubu]

patch_log_filter_remove?

[jacqueswww]

Whilst we are optimising tests, is there a way to make eth tester mine blocks without doing PoW (PoA maybe?). I think this is a huge bottle neck atm. I adjusted the difficulty lower but hardly had any effect. I just know for alot of the example tests forwarding blocks takes most of the tests time. Parrallelisation should be the last optimization we make.

[pipermerriam]

@jacqueswww yes. It's likely that eth-tester isn't taking advantage of this since it's relatively new. We have an API for doing this (docs are broken so I'm linking you to the test suite where it's used)

https://github.com/ethereum/py-evm/blob/master/tests/core/builder-tools/test_chain_construction.py#L133

the eth-tester chain that the PyEVMBackend uses should be updated to use some version of this API to disable the POW check.

[pipermerriam]

Parrallelisation should be the last optimization we make.

Can you explain the reasoning behind this? In most CI environments I've been able to gain an easy 2-3x speed increase simply by parallelizing with pytest-xdist.

[jacqueswww]

@pipermeriam By last resort, I mean it should be the last step we switch on after all other optimisations have been made.

So in any program I write, I see parrallelisation as the last resort, because it's cheating right? You are hiding the aggregate work that gets done. :-) Yeah I am a old riser and believe in the adage of if you want fast programs, give a programmer a slow computer :-P

[jacqueswww]

@pipermeriam Thinking of it some more, lately I have realized that I actually get far more consistent metrics on single core performance accross all processors and platforms. So it make it so much easier to have a quick yardstick guage. This is because the actual performance increase per core is tiny across processor generations.

[pipermerriam]

@jacqueswww I don't see the reasoning in either of those to not enable paralellization in the CI environment.

• Parallelization isn't required and won't be the default. Locally, tests will still run single core unless you provide extra flags
• CI is a shared hosting environment making it an environment unsuitable for measuring performance.
• I agree with your logic in the context of writing an actual application, but I don't see how that logic extends to tests. Fast test suite runs speed up the iteration cycle for development which has very tangible productivity boosts.

[fubuloubu]

Agreed on parallelizing all the tests. It's not that big a deal, easy win!

[jacqueswww]

Sure no problem, let's dot it.

Also, you are wrong that vyper isn't an Application actually you might be right as it's a MetaApplication :P the test run decreased by 400 seconds when I fixed a bug on ABI handling, pyetherum test suite was 2x faster than switching to eth tester - this is super useful. And I bet you if you guys enhance pyevm significantly one can pick it up. That's also why we leverage get_with_gas_estimation (will become get_contract eventually).

[fubuloubu]

In my mind, the ideal structure of the compiler is a series of functions that moves one data structure form to another. If each layer is a pure function, there should be no issue testing each one in parallel, as the data in and out of each layer is what's important (pre/post conditions for a test)

[jacqueswww]

That's not the truth though, it's about what you produce - because you could still be producing a long list of jumps and store codes with your pure functions, or reusing constants ineffectively that are directly correlated to actual time on evm, thereby test time.

That level of testing is very primitive and you definitely need both functional and end to end tests.

[jacqueswww]

So all I want to leave here as food for thought, is there is and could be so much more to tests than just testing functions and coverage :-) {or I would like to think so}.

[pipermerriam]

you are wrong that vyper isn't an Application

I wasn't intending to say Vyper isn't an application. I was saying the tests aren't the application and thus I don't apply the same standards for performance gains (thus being fine with gaining speed through concurrency). Sorry for any misunderstanding.

[jacqueswww]

Ah yes, sorry I misread that ;)

As for tests, I am onboard to make them as fast as possible on the CI :rocket:

[pipermerriam]

And I bet you if you guys enhance pyevm significantly one can pick it up

Yeah, we're aware, and it's very likely that the core EVM performance will get some :heart: in the near future.

[jacqueswww]

I am busy cleaning up conftest a little and bumping our test versions. Will look into the state leaking as part of it.

[jacqueswww]

@pipermerriam running in parallel, the tests run ~5x faster :smile: 104.98 seconds

[pipermerriam]

That's sexy

[fubuloubu]

That's awesome! So... you found the problem? (or somehow avoided it by upgrading? lol)

Edit: I see what you did there

[jacqueswww]

Yes, if you check the PR. The tests were failing because of wrong assumptions in the tests regarding balances & pytest scoping.