Metering middleware. #1839

nlewycky · 2020-11-23T23:25:41Z

Based on https://github.com/wasmerio/wasmer-reborn/pull/129 . Original description:

Necessary APIs for modifying module info.
Global transformation.
Per-function transformation.
Testing and examples.

…make it work on compiler-llvm.

webmaster128 · 2020-11-23T23:30:14Z

Thanks for bringing this over. Happy to test this work in an external project. Just ping me when you think this makes sense.

MarkMcCaskey

Looks pretty reasonable to me!

I'd like to see an example using metering too as the place where we explain how to use it, but that can be done separately after this merges

MarkMcCaskey · 2020-11-23T23:27:40Z

Cargo.toml

@@ -23,6 +23,7 @@ wasmer-wasi = { version = "1.0.0-alpha5", path = "lib/wasi", optional = true }
 wasmer-wast = { version = "1.0.0-alpha5", path = "tests/lib/wast", optional = true }
 wasmer-cache = { version = "1.0.0-alpha5", path = "lib/cache", optional = true }
 wasmer-types = { version = "1.0.0-alpha5", path = "lib/wasmer-types" }
+wasmer-middlewares = { version = "1.0.0-alpha.5", path = "lib/middlewares", optional = true }


Slight typo in the version, should drop the .

Done, thanks.

MarkMcCaskey · 2020-11-23T23:32:22Z

lib/compiler-singlepass/src/compiler.rs

+        let mut module = (*compile_info.module).clone();
+        self.config.middlewares.apply_on_module_info(&mut module);
+        compile_info.module = Arc::new(module);


This will leave all the old Arc<Module>s unaffected, this should be looked at closely for bugs

Indeed that was a source of the bug fixed in 363a28c of this PR. I'm trusting that the tests will catch any real problems here. (Also, there are no old Arc's? At least not in this function?)

compile_info.module before it gets overwritten is pointing to the old Arc, anything that it's shared with won't get the update

Yeah I'm not sure about its correctness in context, but it's possible that this could cause problems later

MarkMcCaskey · 2020-11-23T23:37:55Z

lib/middlewares/src/metering.rs

+                    state.push_operator(Operator::GlobalGet { global_index: self.remaining_points_index.as_u32() });
+                    state.push_operator(Operator::I64Const { value: self.accumulated_cost as i64 });
+                    state.push_operator(Operator::I64LtU);
+                    state.push_operator(Operator::If { ty: WpTypeOrFuncType::Type(WpType::EmptyBlockType) });
+                    state.push_operator(Operator::Unreachable); // FIXME: Signal the error properly.
+                    state.push_operator(Operator::End);
+
+                    // globals[remaining_points_index] -= self.accumulated_cost;
+                    state.push_operator(Operator::GlobalGet { global_index: self.remaining_points_index.as_u32() });
+                    state.push_operator(Operator::I64Const { value: self.accumulated_cost as i64 });
+                    state.push_operator(Operator::I64Sub);
+                    state.push_operator(Operator::GlobalSet { global_index: self.remaining_points_index.as_u32() });


Might be ergonomic to implement https://doc.rust-lang.org/std/iter/trait.Extend.html for this, so you can put a bunch of operators in a collection like &[] and push them all in

Done. Two new Extends.

webmaster128 · 2020-11-26T14:03:44Z

lib/middlewares/src/metering.rs

+/// An instance of `Metering` should not be shared among different modules, since it tracks
+/// module-specific information like the global index to store metering state. Attempts to use
+/// a `Metering` instance from multiple modules will result in a panic.


If an instance of Metering can be used for one module only, and it is installed via CompilerConfig::push_middleware, does this mean you need a new compiler (and engine and store) for each metered module?

That's right. The current structure makes easier reusing the modules in headless engines.

nlewycky · 2020-11-30T23:19:40Z

bors r+

1839: Metering middleware. r=nlewycky a=nlewycky Based on wasmerio/wasmer-reborn#129 . Original description: - [x] Necessary APIs for modifying module info. - [x] Global transformation. - [x] Per-function transformation. - [x] Testing and examples. Co-authored-by: losfair <[email protected]> Co-authored-by: Syrus <[email protected]> Co-authored-by: Nick Lewycky <[email protected]> Co-authored-by: nlewycky <[email protected]>

bors · 2020-12-01T00:53:33Z

Build failed:

Test cross-compile on linux

syrusakbary · 2020-12-01T17:57:13Z

bors r+

1839: Metering middleware. r=syrusakbary a=nlewycky Based on wasmerio/wasmer-reborn#129 . Original description: - [x] Necessary APIs for modifying module info. - [x] Global transformation. - [x] Per-function transformation. - [x] Testing and examples. Co-authored-by: losfair <[email protected]> Co-authored-by: Syrus <[email protected]> Co-authored-by: Nick Lewycky <[email protected]> Co-authored-by: nlewycky <[email protected]> Co-authored-by: Syrus Akbary <[email protected]>

syrusakbary · 2020-12-01T19:48:19Z

bors r-

bors · 2020-12-01T19:48:20Z

Canceled.

syrusakbary · 2020-12-01T19:48:28Z

bors r+

bors · 2020-12-01T21:55:25Z

Build succeeded:

losfair and others added 11 commits November 20, 2020 14:34

Add module info transformation method to ModuleMiddleware.

e7dd725

Add metering middleware.

6537720

Add metering middleware.

13a979a

Update module info in-place.

9d8f315

Add more metering tests.

17e7fea

Port a metering test from the old repo.

77b99a0

Short-circuit read_operator for empty chains

89762a6

Add support for middleware to experimental_native_compile_module and …

8ca3693

…make it work on compiler-llvm.

Fix middleware with singlepass.

363a28c

Remove shady "split the borrow" function that relied on UB.

970af1a

Clean up build warnings.

21369a2

MarkMcCaskey reviewed Nov 23, 2020

View reviewed changes

nlewycky added 2 commits November 23, 2020 15:40

Fix the version number for the new middlewares crate.

05d8970

Add a variant of Extend that works on borrowed arrays.

32b3dae

jubianchi added 1.0 Wasmer at 1.0 ⏱ metering 🎉 enhancement New feature! labels Nov 25, 2020

webmaster128 reviewed Nov 26, 2020

View reviewed changes

Merge branch 'master' into feature/middleware

f110c4d

nlewycky marked this pull request as ready for review November 30, 2020 23:12

nlewycky requested review from Hywan, losfair and syrusakbary as code owners November 30, 2020 23:12

syrusakbary approved these changes Nov 30, 2020

View reviewed changes

Merge branch 'master' into feature/middleware

097bfeb

webmaster128 mentioned this pull request Dec 1, 2020

Add metering CosmWasm/cosmwasm#642

Closed

bors bot merged commit 5371543 into master Dec 1, 2020

bors bot deleted the feature/middleware branch December 1, 2020 21:55

syrusakbary mentioned this pull request Dec 1, 2020

Wasm runtime tracing #1852

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metering middleware. #1839

Metering middleware. #1839

nlewycky commented Nov 23, 2020

webmaster128 commented Nov 23, 2020

MarkMcCaskey left a comment

MarkMcCaskey Nov 23, 2020

nlewycky Nov 24, 2020

MarkMcCaskey Nov 23, 2020

nlewycky Nov 23, 2020

MarkMcCaskey Nov 24, 2020

MarkMcCaskey Nov 24, 2020

MarkMcCaskey Nov 23, 2020

nlewycky Nov 24, 2020

webmaster128 Nov 26, 2020

syrusakbary Nov 30, 2020

nlewycky commented Nov 30, 2020

bors bot commented Dec 1, 2020

syrusakbary commented Dec 1, 2020

syrusakbary commented Dec 1, 2020

bors bot commented Dec 1, 2020

syrusakbary commented Dec 1, 2020

bors bot commented Dec 1, 2020

Metering middleware. #1839

Metering middleware. #1839

Conversation

nlewycky commented Nov 23, 2020

webmaster128 commented Nov 23, 2020

MarkMcCaskey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nlewycky commented Nov 30, 2020

bors bot commented Dec 1, 2020

syrusakbary commented Dec 1, 2020

syrusakbary commented Dec 1, 2020

bors bot commented Dec 1, 2020

syrusakbary commented Dec 1, 2020

bors bot commented Dec 1, 2020