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.

Sign up for GitHub

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

Jump to bottom

[mono][interp] Fix concurrency issues with publishing transfomed imethod #87555

Merged
merged 3 commits into from
Jun 16, 2023
Merged

[mono][interp] Fix concurrency issues with publishing transfomed imethod #87555

merged 3 commits into from
Jun 16, 2023

Commits on Jun 14, 2023

  1. [mono][utils] Move all membar code to single file

    @BrzVlad
    BrzVlad committed Jun 14, 2023
    Configuration menu
    Copy the full SHA
    5ed0703 View commit details
    Browse the repository at this point in the history

  2. [mono][utils] Redefine write and read memory barriers 

    Before this change they were doing a full memory barrier, regardless of architecture. We have a beginning of more precise implementation via the *_FENCE defines. Implement mono_memory_write_barrier and mono_memory_read_barrier reusing these defines instead.

The only consequence of this change is that, on x86 and amd64, `mono_memory_write_barrier` and `mono_memory_read_barrier` become a compiler barrier instead of a full mfence.
    @BrzVlad
    BrzVlad committed Jun 14, 2023
    Configuration menu
    Copy the full SHA
    6ad0764 View commit details
    Browse the repository at this point in the history

  3. [mono][interp] Fix concurrency issues with publishing transfomed imethod 

    When publishing a transformed InterpMethod*, we first set all relevant fields (like `code`, `alloca_size` etc), we execute a write barrier and finally we set the `transformed` flag. On relaxed memory arches we need to have a read barrier on the consumer, since there is no data dependency between `transformed` and the other fields of `InterpMethod`.

On arm this change does a full barrier (we could get away with just a load acquire but we haven't yet added support for emitting this in the runtime). Still, this doesn't seem to introduce a heavy perf penalty (on my arm64 M1) but we can revisit if necessary. On x86/amd64 this is a compiler barrier so it should have no impact. WASM is single threaded for now.
    @BrzVlad
    BrzVlad committed Jun 14, 2023
    Configuration menu
    Copy the full SHA
    160301b View commit details
    Browse the repository at this point in the history