Stream: git-wasmtime

Topic: wasmtime / PR #1227 Thread-safe compilation API

Wasmtime GitHub notifications bot (Mar 04 2020 at 14:07):

arkpar opened PR #1227 from a-thread-safe-api to master:

This makes Module implement Send and Sync, allowing instances to be created and executed in different threads.

Closes #777

Wasmtime GitHub notifications bot (Mar 04 2020 at 14:22):

arkpar updated PR #1227 from a-thread-safe-api to master:

This makes Module implement Send and Sync, allowing instances to be created and executed in different threads.

Closes #777

Wasmtime GitHub notifications bot (Mar 04 2020 at 15:03):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Mar 04 2020 at 15:03):

alexcrichton created PR Review Comment:

I personally think that this probably isn't the way we want to parallelize Module and Store and such. This means that compilation is still entirely serialized and you can't compile multiple modules in parallel, one of the main usages of enabling parallelism. I think we need to work to continue to refactor and reimplement the guts of Compiler to make them Send and Sync and use &self.

For example the trampoline_park field is going away in https://github.com/bytecodealliance/wasmtime/pull/957, and I don't think that the switch to AtomicPtr<T> here is correct in that it's not ever modified, it's just a pointer.

The main next change is to move CodeMemory to per-module instances, and then mutability shouldn't be needed on most of the fields here.

Wasmtime GitHub notifications bot (Mar 04 2020 at 15:03):

alexcrichton created PR Review Comment:

Instead of unsafely asserting send/sync for FinishedFunctions, could a newtype wrapper around *const [VMFunctionBody] be made to narrow-the scope of send/sync? That way we're not unsafely asserting thread safety about DefinedFuncIndex or BoxedSlice

Wasmtime GitHub notifications bot (Mar 04 2020 at 15:03):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Mar 04 2020 at 15:03):

alexcrichton created PR Review Comment:

I've worked pretty hard to remove lazy_static from most crates because it often behaves in susprising ways. I don't understand much about the GDB integration here, but I think it would be better to have an instanced lock and/or atomics to manage this.

Wasmtime GitHub notifications bot (Mar 05 2020 at 16:05):

yurydelendik submitted PR Review.

Wasmtime GitHub notifications bot (Mar 05 2020 at 16:05):

yurydelendik created PR Review Comment:

gdb will use __jit_debug_descriptor (see static below) on this module startup and during reactiving on __jit_debug_register_code breakpoint AFAIK. The __jit_debug_descriptor is a global symbol for entire module.

Wasmtime GitHub notifications bot (Mar 05 2020 at 17:18):

arkpar submitted PR Review.

Wasmtime GitHub notifications bot (Mar 05 2020 at 17:18):

arkpar created PR Review Comment:

Is it safe to assume there won't be any concurrent access here?

Wasmtime GitHub notifications bot (Mar 16 2020 at 15:03):

pepyakin closed without merge PR #1227.

Last updated: Nov 22 2024 at 17:03 UTC