Optimize for performance instead of code size #197
Conversation
birchmd
requested review from
artob,
mrLSD,
0x3bfc,
sept-en,
joshuajbouw and
mfornet
| @@ -26,6 +26,7 @@ jobs: | |||
| bully-build: | |||
| name: Bully build | |||
| runs-on: self-hosted | |||
| needs: test | |||
There was a problem hiding this comment.
This is unrelated to the opt-level change, but seems to make CI run better (prevents some kind of race causing a file to be deleted which is still needed by other tests).
There was a problem hiding this comment.
Hmm, that's interesting.
I suspect that, unfortunately, this will not solve the problem. Two or more concurrent "test" jobs can possibly produce the same issue.
Let me explain: each job is launched on random free runner (1 of 16), so each of them have it's own isolated workdir. Except only moment - they still share target/release and target/debug directories (you can see that I made them as symlinks), and this directories are shared for all runs and all jobs (even concurrent). I did a little research before doing it, it was stated that cargo target dir is OK for concurrent builds, but after seeing this - I doubt a lot. I mean, it's impossible to explain those races another way (because those are only files that are shared/concurrently used). I should probably redesign this part...
Do you know which particular file was under race? If it's under target/release or target/debug - it's definitely my fault here :)
|
Thats a huge improvement, but I think we should have a discussion about which path we should go if contract bloat is better to reduce cost of gas, which I do agree, is far better. We should also have quite a large reduction when we have the native precomps too. |
* JSON: fix bugs and add unit tests. (#141) * Add storage layout debug support for `EvmErc20.sol`. (#178) * ERC-20: forbid using invalid NEP-141 AccountID for mapping. (#179) * Add EIP-2930 support. (#181, #182) * Migrate all workflows to self-hosted runners. (#185) * Speed up the workflow using build caching. (#189) * Use the new math API host functions. (#190) * Fix `clippy::enum_variant_names` warning. (#192) * Add different networks to the Makefile. (#193) * Update the network status in the README. (#194) * Remove the toolchain installation step in workflows. (#195) * Run all tests for all networks in CI. (#196) * Optimize for performance instead of code size. (#197) * Parallelize the test suites. (#198) * Add build-caching to the testing workflow. (#201) * Refactor tests to use Signer. (#203) * Add options to the bench profile. (#204) * Remove a duplicate test. (#205) * Add a sanity test for access list handling. (#206) * Update nearcore to the latest branch. * Add feature gates to the SDK's new host functions. Co-authored-by: Ahmed Ali <ahmed@aurora.dev> Co-authored-by: Dmitry Strokov <dmitry@aurora.dev> Co-authored-by: Evgeny Ukhanov <evgeny@aurora.dev> Co-authored-by: Joshua J. Bouw <joshua@aurora.dev> Co-authored-by: Kirill <kirill@aurora.dev> Co-authored-by: Michael Birch <michael@aurora.dev>
Benchmarks show that this significantly reduces gas usage (around 47%) for non-trivial contracts (e.g. uniswap). We have a larger binary as a result (678K -> 865K on my machine), but I think it is worthwhile.