Avoid building all three wasm engines for production use

matklad commented 3 years ago

At the moment, we enable all three vms using default features in near-vm-runner:

https://github.com/near/nearcore/blob/07b4fd45f90570d9c4f306f724cf84499cc519c7/runtime/near-vm-runner/Cargo.toml#L48

The problem with default features is that they are impossible to disable. Our current setup is more or less equivalent to just always using all three vm. Indeed, if you remove this default feature, you'll notice that near-vm-runner doesn't even compile, as it misses some important cfgs.

At the same time, wasm runtimes are very heavy dependencies (take long time to compile, and often use global resources like #<span class="error">[no_mangle]</span> functions or signal handlers), so using conditional compilation here makes sense.

I think to fix this we need:

change nightly CI setup to run cargo test --workspace --features nightly_protocol,nightly_protocol_features,protocol_feature_evm,wasmer0_vm,wasmer1_vm,wasmtime_vm
change to default = <span class="error">["wasmer0_vm"]</span> only
fix compilation time errors

matklad commented 3 years ago

cc @olonho @ailisp

bowenwang1996 commented 3 years ago

@matklad does the contract runtime team want to work on this issue or do you want someone else to work on it?

ailisp commented 3 years ago

we haven't reach consensus on it at last meeting, @olonho @matklad let's make a conclusion on this today

matklad commented 3 years ago

Discussed in today's meeting. Summary:

for contract runtime team, it's more convenient if all vms are enabled by default
for other teams, this creates extra compilation time tax (b/c vm deps are heavy)
a nice middleground is to ensure that we test all vms on CI, but ship only one

Specific proposal:

remove wasmer1_vm and wasmtime_vm from the set of default features in Cargo.toml
add those features to the nightly tests instead.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

matklad commented 2 years ago

cc @akhi3030

near / nearcore

Avoid building all three wasm engines for production use #4358