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

Add new gas metering method: mutable global + local gas function #34

Merged
merged 59 commits into from Nov 20, 2022

Conversation

agryaznov
Copy link
Contributor

Solves #11

In addition to method used beforehand (i.e. injection of calls to an imported gas function into metering blocks), this new method now available too, which:

  • Injects an exported mutable global into the module, which tracks the gas left.
  • Injects the local gas function which subtracts the gas spent from the global, and drops unreachable if out of gas.

This new method should make the resulting module more efficient in terms of execution time, as invoking an imported gas host function every metering block adds an overhead. Synchronization between the global and the runtime should be implemented on the execution engine side.

src/gas_metering/mod.rs Outdated Show resolved Hide resolved
src/gas_metering/mod.rs Outdated Show resolved Hide resolved
src/gas_metering/mod.rs Outdated Show resolved Hide resolved
@agryaznov agryaznov requested a review from athei October 11, 2022 12:00
@agryaznov
Copy link
Contributor Author

new benches is WIP (coming soon)

@agryaznov agryaznov changed the title Add improved gas metering method: mutable global instead of imported gas function Add the improved gas metering method: mutable global instead of imported gas function Oct 11, 2022
src/gas_metering/backend.rs Outdated Show resolved Hide resolved
Copy link
Member

@athei athei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do not perform more work in the gas function as necessary.

src/gas_metering/mod.rs Outdated Show resolved Hide resolved
@agryaznov
Copy link
Contributor Author

agryaznov commented Nov 17, 2022

With new wasmi 0.20 engine, the overheads of calling an imported function and a local function are roughly the same. So the performance wins of using the new gas metering method are in question, and should be sanity-checked for each particular execution engine. Benchmarks are added showing how this could be measured. Versus using an emulated host function they show up to 46% performance decrease for modules instrumented with the new gas metering method. However, these numbers should not be treated as the ultimate truth, as in a real world setup it deeply depends on the host function implementation and where the execution engine resides. Client-side execution engine is expected to have slower host functions, so that's could be a proper use case for the new gas metering method with a mutable global.

This problem:

Just figured out that applying current implementation of stack_limiter injector after gas_metering::mutable_global injector bloats the resulting module massively. The reason is that with current impl of stack_limiter its wraps every call to the local gas function with its stack cost metering instructions. For many_blocks.wasm the overhead is 10x times of original module size.

will be addressed in a follow-up PR.

@agryaznov agryaznov requested a review from athei November 17, 2022 10:08
@agryaznov agryaznov changed the title Add the improved gas metering method: mutable global instead of imported gas function Add new gas metering method: mutable global + local gas function Nov 17, 2022
@agryaznov
Copy link
Contributor Author

Benches

version tested: 6f43fbc

with rustc 1.65, wasmi 0.20.0

fixture host func mut global perf
coremark 20.81 s 20.20 s +3%
recursive_ok 367 µs 585 µs -37%
fibonacci_recursive 9.15 µs 13.56 µs -33%
factorial_recursive 1.50 µs 1.98 µs -24%
count_until 5.03 ms 8.13 ms -38%
memory_vec_add 6.21 ms 8.45 ms -27%
tiny_keccak 925 µs 1080 µs -14%
global_bump 3.79 ms 7.03 ms -46%

src/gas_metering/mod.rs Outdated Show resolved Hide resolved
src/gas_metering/mod.rs Outdated Show resolved Hide resolved
src/gas_metering/mod.rs Outdated Show resolved Hide resolved
src/gas_metering/mod.rs Outdated Show resolved Hide resolved
src/gas_metering/mod.rs Outdated Show resolved Hide resolved
tests/diff.rs Outdated Show resolved Hide resolved
@agryaznov agryaznov force-pushed the ag-gas-metering branch 2 times, most recently from db2d4e5 to 5084aa4 Compare November 18, 2022 19:44
@athei athei merged commit a4dde28 into master Nov 20, 2022
@athei athei deleted the ag-gas-metering branch November 20, 2022 14:00
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.

None yet

2 participants