yurydelendik opened PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompileModuleholdsCodeMemoryandGdbJitImageRegistration- Preliminary marked
CompileModuleasSend+Syncfor "frame_info.rs" -- probably needs to be changedStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompileModuleholdsCodeMemoryandGdbJitImageRegistration- Preliminary marked
CompileModuleasSend+Syncfor "frame_info.rs" -- probably needs to be changedStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik requested alexcrichton for a review on PR #1761.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompileModuleholdsCodeMemoryandGdbJitImageRegistration- Preliminary marked
CompileModuleasSend+Syncfor "frame_info.rs" -- probably needs to be changedStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompileModuleholdsCodeMemoryandGdbJitImageRegistration- Preliminary marked
CompileModuleasSend+Syncfor "frame_info.rs" -- probably needs to be changedStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompileModuleholdsCodeMemoryandGdbJitImageRegistration- Preliminary marked
CompileModuleasSend+Syncfor "frame_info.rs" -- probably needs to be changedStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompileModuleholdsCodeMemoryandGdbJitImageRegistration- Preliminary marked
CompileModuleasSend+Syncfor "frame_info.rs" -- probably needs to be changedStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik edited PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistration- Preliminary marked
CompiledModuleasSend+Syncfor "frame_info.rs" -- probably needs to be changedStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistration- Preliminary marked
CompiledModuleasSend+Syncfor "frame_info.rs" -- probably needs to be changedStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik edited PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
The
Storehere internally looks to be the only reason that this is non-Send, right? If that's the case can we move theStoreout, or better yet only take anEngineto compile amodule?This would require taking a
Storeargument to create anInstancebut I think that's ok sinceLinkeris the "ergonomic" way to create instances.Additionally this would require, on instantiation, registration of JIT code ranges within a
Storeif a module hasn't previously been registered there, but I think that's ok?
alexcrichton created PR Review Comment:
FWIW this is breaking on Windows, but I think it's fine to just skip this example and compile it as a no-op on Windows
alexcrichton created PR Review Comment:
I'm personaly pretty wary of
Derefunless it's used for smart pointers, so could this be removed to instead useself.modulewhere necessary?
alexcrichton created PR Review Comment:
This makes me a little uncomfortable using
Dereflike this.The only reason for this, I believe, is keeping the
CodeMemoryalive, right? Since the memory management ofInstanceHandleis detached from the handle itself (it happens inStorefor thewasmtimecrate), could the managment ofCodeMemorymove intoStoreas well?For example could each
CompiledModulestore anArc<CodeMemory>, and then we push a strong reference of that into aStorewhenever a module is instantiated within aStore?
alexcrichton created PR Review Comment:
.find(...).is_some()can be replaced with.any(...)as well
alexcrichton created PR Review Comment:
IIRC
SignatureRegistryhas a lock internally, could that be removed now in favor of just using theRefCellhere for mutability?I think that there's no need for
SignatureRegistryto be synchronized, even in a shared memory world I think instances are still disconnected in that they can have different signature registries on each thread.
alexcrichton created PR Review Comment:
I think in general this probably isn't necessary if we do this registration at instantiation time instead of compile time for a
Module. Once aModuleis instantiated into aStorewe never drop theInstanceHandleanyway, so it's permanently registered within a store, which means there's no need to remove.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik submitted PR Review.
yurydelendik created PR Review Comment:
I replaced
Deref<Target = Module>withInstanceContext. I'm not convinced thatStoreis good location forCodeMemory. I understand that the end goal is to be capable to ditch entireModule/CodeModuleeven ifInstanceor its exports are present, but keepCodeMemoryalive. Can we do it in follow up PRs?
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik has marked PR #1761 as ready for review.
yurydelendik requested alexcrichton for a review on PR #1761.
yurydelendik submitted PR Review.
yurydelendik created PR Review Comment:
I redesigned API to remove
StorefromModule. @alexcrichton can you take a look at this change before I proceed any further?
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Since the
storeis now passed int toInstance::newI think this check for same-store-ness can be removed?
alexcrichton created PR Review Comment:
One of the worries I have about this is that while an instance is alive it retains the entire
CompiledModule. Not everything inCompiledModule, however, is used throughout the liftime of the instance. For example I think a number of internal maps here are also copied to theInstanceHandle. I think it'd be best if we could split this into two parts:
- Parts needed for the lifetime of the
InstanceHandle. Some of this is required by the handle itself (likeModule), others are simply required to stay alive for the duration of the instance (e.g.CodeMemoryandGdbJitImageRegistration- Other parts are only needed for introspection and instantiation. For example I think the
trampolinesmap andfinished_functionslists here are only needed temporarily, since their relevant bits are copied into theInstanceHandle.This is a relatively niche concern though and it's not like it's a radical change from what we have here. Could a FIXME and/or issue be left though for this future refactoring?
alexcrichton created PR Review Comment:
I think this can probably be removed since it's only used for the one check in
wasm_instance_newwhich I think we can also remove now too.
alexcrichton created PR Review Comment:
Could the deallocation that happens inside here happen outside of the logkc?
alexcrichton created PR Review Comment:
IIRC
let _ = foois a special idiom in Rust where it will run the dtor forfooimmediately, rather than defer it to the end of the block. I think that means that the lock isn't held for the duration of this block?
alexcrichton created PR Review Comment:
Could this allocation happen before the synchronization of the lock?
alexcrichton created PR Review Comment:
I don't think that the
u32here is used, right?Also could this have documentation as to its purpose?
alexcrichton created PR Review Comment:
Could an instance of
Compilerbe stored within theEngineitself, so the same instance is reused across all compilations? I thinkCompilershould beSendandSyncnow right?
alexcrichton created PR Review Comment:
How come this changed to
CompiledModule? That's retaining a lot more information here thanModulepreviously was
alexcrichton created PR Review Comment:
Also as another note, this technically is still unsafe since we're not synchronized with anything else in the process that may be frobbing these symbols. Is there really no way we can rely on a gdb thing to do the synchronization for us?
yurydelendik submitted PR Review.
yurydelendik created PR Review Comment:
No. This process shall control access to __jit_debug_descriptor -- added comment
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik submitted PR Review.
yurydelendik created PR Review Comment:
The gdb only known about
__jit_debug_descriptorand sets breakpoint at__jit_debug_register_code. Locking access to __jit_debug_descriptor is the process chore -- gdb will traverse its data once process in the __jit_debug_register_code.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik created PR Review Comment:
Theoretically, yes, we can remove. It will drive wasmtime_instance_new signature change too, and renders wasm_module_share / wasm_module_obtain useless. I would like to defer this change for future work.
yurydelendik submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
This might be best as a private method on
Config?
alexcrichton created PR Review Comment:
Could these two fields be wrapped up in one
Arcallocation?
alexcrichton created PR Review Comment:
Could
Default for Storedelegate to this now that it exists?
alexcrichton created PR Review Comment:
FWIW now that we're doubling down on
Rcinside ofStore, I think we should remove these methods. It's too easy, at callsites, to forget these methods are related toRefCelland can introduce panics.
alexcrichton created PR Review Comment:
Or, alternatively, all accesses to these fields would be scoped inside of methods on this type, where each method doesn't leak borrowed
RefCells through returns
alexcrichton created PR Review Comment:
Could this have a comment for what's going on here? Without that it's a bit cryptic what this check is doing
alexcrichton created PR Review Comment:
I think changing signatures is fine, but if you'd prefer to defer that's ok too. Can you be sure an issue is filed about this?
alexcrichton created PR Review Comment:
(removed in favor of direct field access at callsites, that is)
alexcrichton created PR Review Comment:
Er sorry I mean thte literal call to malloc, not the reading/writing of the global.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik submitted PR Review.
yurydelendik created PR Review Comment:
Returned back to store only Module, and added ModuleCode (which is only CodeMemory + GdbJitRegistration)
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik submitted PR Review.
yurydelendik created PR Review Comment:
Not sure how to do it:
Engine::newandStore::newhave some additional logic that needs to be run.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik requested alexcrichton for a review on PR #1761.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Long-term it feels weird to have two
dyn Anyin thisInstance, instead ideally we'd just have oneBox<dyn Any>and shove everything in there. I don't want to iterate too much on this, it's pretty unimportant at this time. Just something to keep in mind for the future, this is more than ok for now.
alexcrichton created PR Review Comment:
Instead of embedding the
.unwrap()here, could this returnOption<&mut Module>? It's only used in one location right now which is fine, but I'd prefer to avoid proliferating users of this API
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik updated PR #1761 from mv-sig-registry to master:
To make wasmtime more threads friendly, this PR:
- Moves
CodeMemory,VMInterruptsandSignatureRegistryfromCompilerCompiledModuleholdsCodeMemoryandGdbJitImageRegistrationStorekeeps track of its JIT code- Makes "jit_int.rs" stuff
Send+Sync- Adds the threads example.
yurydelendik merged PR #1761.
Last updated: Dec 06 2025 at 06:05 UTC