add new_with_user_state #2282

zhiqiangxu · 2020-10-08T11:59:33Z

Allow user to specify custom user state when instantiate a wasm instance.

bjorn3

How do you get access to the user state?

bjorn3 · 2020-10-08T12:02:30Z

crates/wasmtime/src/instance.rs

@@ -156,7 +163,12 @@ impl Instance {
 /// [inst]: https://webassembly.github.io/spec/core/exec/modules.html#exec-instantiation
 /// [issue]: https://github.com/bytecodealliance/wasmtime/issues/727
 /// [`ExternType`]: crate::ExternType
- pub fn new(store: &Store, module: &Module, imports: &[Extern]) -> Result<Instance, Error> {
+ fn _new(


Suggested change

fn _new(

pub fn new_with_user_state(

bjorn3 · 2020-10-08T12:03:35Z

crates/wasmtime/src/instance.rs

+ pub fn new(store: &Store, module: &Module, imports: &[Extern]) -> Result<Instance, Error> {
+ return _new(store, module, imports, None);
+ }
+
+ pub fn new_with_user_state(
+ store: &Store,
+ module: &Module,
+ imports: &[Extern],
+ user_state: Option<dyn Any>,
+ ) -> Result<Instance, Error> {
+ return _new(store, module, imports, user_state);
+ }


Suggested change

pub fn new(store: &Store, module: &Module, imports: &[Extern]) -> Result<Instance, Error> {

return _new(store, module, imports, None);

}

pub fn new_with_user_state(

store: &Store,

module: &Module,

imports: &[Extern],

user_state: Option<dyn Any>,

) -> Result<Instance, Error> {

return _new(store, module, imports, user_state);

}

pub fn new(store: &Store, module: &Module, imports: &[Extern]) -> Result<Instance, Error> {

Instance::new_with_user_state(store, module, imports, None)

}

This function is also missing documentation.

Fixed, _new is removed.

One can access the user state by instance.host_state() then cast it to HostState

bjorn3 · 2020-10-08T12:11:56Z

crates/wasmtime/src/instance.rs

@@ -179,6 +195,10 @@ impl Instance {
 })
 }

+ pub fn new(store: &Store, module: &Module, imports: &[Extern]) -> Result<Instance, Error> {
+ return new_with_user_state(store, module, imports, None);


Suggested change

return new_with_user_state(store, module, imports, None);

Instance::new_with_user_state(store, module, imports, None)

You can't use new_with_user_state directly as it isn't imported in the current module.

Please prefer implicit return at the end of a block/function.

bjorn3 · 2020-10-08T12:30:35Z

crates/wasmtime/src/instance.rs

@@ -104,14 +104,15 @@ pub struct Instance {
 module: Module,
 }

+#[allow(dead_code)]


This shouldn't be necessary.

Otherwise it will report: warning: field is never read: frame_info_registration.

bjorn3 · 2020-10-08T12:41:22Z

crates/wasmtime/src/instance.rs

@@ -102,7 +104,47 @@ pub struct Instance {
 module: Module,
 }

+#[allow(dead_code)]
+pub struct HostState {
+ frame_info_registration: Option<Arc<GlobalFrameInfoRegistration>>,


Suggested change

frame_info_registration: Option<Arc<GlobalFrameInfoRegistration>>,

_frame_info_registration: Option<Arc<GlobalFrameInfoRegistration>>,

This suppresses the dead code warning for this field only.

I moved #[allow(dead_code)] to above the frame_info_registration field

github-actions · 2020-10-08T13:17:12Z

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

alexcrichton · 2020-10-08T21:13:21Z

Thanks for the PR! Can you describe in a bit more detail how you see this being used though? As-is this seems like it's only useful in that you can get a destructor called when the instance itself is deallocated, but I'm not sure if that's the intention or if there's another use case you'd like to unlock. (I'm also not entirely sure how that ability would be used itself)

zhiqiangxu · 2020-10-08T23:56:29Z

Thanks for the PR! Can you describe in a bit more detail how you see this being used though? As-is this seems like it's only useful in that you can get a destructor called when the instance itself is deallocated, but I'm not sure if that's the intention or if there's another use case you'd like to unlock. (I'm also not entirely sure how that ability would be used itself)

This is useful if you want to add extra logic such as gas limit for blockchain, the limit can be stored in user state, and checked in FunctionEnvironment::before_translate_operator(extra built in function is required to do that).I will try to submit code for customize that in another PR.

alexcrichton · 2020-10-11T17:26:19Z

Hm ok, although the wasmtime crate is not intended to be used where you're also using lots of cranelift internals. At that point it may be best to add a first-class feature to Wasmtime.

zhiqiangxu · 2020-10-11T17:35:16Z

I see. Gas limiting is a fundamental feature for blockchain,so such feature will be very handy for whoever implements vm layer based on wasmtime.

alexcrichton · 2020-10-11T18:29:35Z

Oh sure yeah I don't mean to imply that we're not interested in this feature, just that this probably isn't the best way to implement it in Wasmtime.

zhiqiangxu · 2020-10-12T00:41:25Z

What way do you recommend ? I don’t mind if there is a better solution:)

alexcrichton · 2020-10-12T21:27:29Z

This sort of limiting execution seems like it would probably be implemented similar to how timeouts are implemented right now, which is to say that there's a configuration option in Config which allows you to tweak an object from Store. The configuration option would then get plumbed through to the codegen backend to affect the generated code.

bjorn3 · 2021-06-12T11:29:57Z

Does this still make sense after #2897?

alexcrichton · 2021-06-24T18:39:52Z

I'm going to close this since this has been inactive for some time and I agree that after #2897 this is less relevant than before. If you'd still like to pursue this though feel free to send a PR!

add new_with_user_state

65c1133

bjorn3 suggested changes Oct 8, 2020

View reviewed changes

zhiqiangxu added 2 commits October 8, 2020 20:07

fix according to review

c1310f9

add missing pub

9d74488

bjorn3 reviewed Oct 8, 2020

View reviewed changes

zhiqiangxu added 4 commits October 8, 2020 20:17

prefer implicit return

c6a1dfc

fix

3db1e7e

fix

372ed92

fix comment

076c0db

bjorn3 reviewed Oct 8, 2020

View reviewed changes

github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Oct 8, 2020

add missing commit

4b65169

alexcrichton closed this Jun 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add new_with_user_state #2282

add new_with_user_state #2282

zhiqiangxu commented Oct 8, 2020 •

edited

Loading

bjorn3 left a comment

bjorn3 Oct 8, 2020

bjorn3 Oct 8, 2020

zhiqiangxu Oct 8, 2020

bjorn3 Oct 8, 2020

zhiqiangxu Oct 8, 2020

bjorn3 Oct 8, 2020

zhiqiangxu Oct 8, 2020

bjorn3 Oct 8, 2020 •

edited

Loading

zhiqiangxu Oct 8, 2020

github-actions bot commented Oct 8, 2020

alexcrichton commented Oct 8, 2020

zhiqiangxu commented Oct 8, 2020 •

edited

Loading

alexcrichton commented Oct 11, 2020

zhiqiangxu commented Oct 11, 2020

alexcrichton commented Oct 11, 2020

zhiqiangxu commented Oct 12, 2020

alexcrichton commented Oct 12, 2020

bjorn3 commented Jun 12, 2021

alexcrichton commented Jun 24, 2021

	return new_with_user_state(store, module, imports, None);
	Instance::new_with_user_state(store, module, imports, None)

	frame_info_registration: Option<Arc<GlobalFrameInfoRegistration>>,
	_frame_info_registration: Option<Arc<GlobalFrameInfoRegistration>>,

add new_with_user_state #2282

add new_with_user_state #2282

Conversation

zhiqiangxu commented Oct 8, 2020 • edited Loading

bjorn3 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

bjorn3 Oct 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 8, 2020

Subscribe to Label Action

alexcrichton commented Oct 8, 2020

zhiqiangxu commented Oct 8, 2020 • edited Loading

alexcrichton commented Oct 11, 2020

zhiqiangxu commented Oct 11, 2020

alexcrichton commented Oct 11, 2020

zhiqiangxu commented Oct 12, 2020

alexcrichton commented Oct 12, 2020

bjorn3 commented Jun 12, 2021

alexcrichton commented Jun 24, 2021

zhiqiangxu commented Oct 8, 2020 •

edited

Loading

bjorn3 Oct 8, 2020 •

edited

Loading

zhiqiangxu commented Oct 8, 2020 •

edited

Loading