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.

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

Atomic operations. #10610

Merged
merged 28 commits into from
Nov 24, 2022
Merged

Conversation

Apprentice-Alchemist
Copy link
Contributor

@Apprentice-Alchemist Apprentice-Alchemist commented Mar 1, 2022

Even JavaScript has atomics now, no reason why Haxe shouldn't.

Supported targets:

The hashlink unit tests fail in TestException when setting hl_ver to 1.12.0, because of 98df217

Currently haxe.Atomic<T> is only implemented for Int, but it should be possible to implement atomic operations for "all" types, at least on Hxcpp/Hashlink and probably Java and C# too.
haxe.atomic.AtomicObject is implemented for Hashlink, Java and C#.

@Simn
Copy link
Member

Simn commented Mar 1, 2022

That cas_loop stuff looks wild... how can that be atomic without any synchronization?

@Apprentice-Alchemist
Copy link
Contributor Author

That cas_loop stuff is an attempt at reproducing what Clang/GCC generate for their atomic intrinsics

        mov     eax, dword ptr [rdi]
.LBB3_1:                                # =>This Inner Loop Header: Depth=1
        mov     ecx, eax
        or      ecx, esi
        lock            cmpxchg dword ptr [rdi], ecx
        jne     .LBB3_1
        ret

@skial skial mentioned this pull request Mar 2, 2022
1 task
@Simn
Copy link
Member

Simn commented Mar 26, 2022

I read a bit about CAS loops and I think this is fine. The function application freaks me out a bit, but I suppose inlining is going to take care of everything here.

However, that loop really looks like it wants to become a do ... while loop.

@Apprentice-Alchemist
Copy link
Contributor Author

Apprentice-Alchemist commented Mar 26, 2022

Alright, the loop has been changed to a do .. while loop.
I also changed the Java compareExchange implementation to also use a CAS loop, otherwise the original value can't be returned without a race condition.

The only problem remaining is the TestExceptions.testExceptions test breaking when using -D hl-ver=1.12.0.
(I assume this is due to 98df217)

The exact error:

unit.TestExceptions
  testExceptionStack: FAILURE .FFFF.FFFFF
    line: 255, _with_ throws: {file : /usr/local/share/haxe/std/hl/_std/haxe/Exception.hx, line : 24, method : __constructor__} is expected, but got {file : /usr/local/share/haxe/std/haxe/ValueException.hx, line : 24, method : __constructor__}
    line: 255, _with_ throws: {file : /usr/local/share/haxe/std/hl/_std/haxe/Exception.hx, line : 6, method : __constructor__} is expected, but got {file : unit/TestExceptions.hx, line : 46, method : __constructor__}
    line: 255, _with_ throws: {file : /usr/local/share/haxe/std/hl/_std/haxe/Exception.hx, line : -12, method : __constructor__} is expected, but got {file : unit/TestExceptions.hx, line : 42, method : __constructor__}
    line: 255, _with_ throws: {file : /usr/local/share/haxe/std/hl/_std/haxe/Exception.hx, line : -30, method : __constructor__} is expected, but got {file : /usr/local/share/haxe/std/hl/_std/haxe/Exception.hx, line : 29, method : thrown}
    line: 255, _without_ throws: {file : /usr/local/share/haxe/std/haxe/CallStack.hx, line : 42, method : callStack} is expected, but got {file : /usr/local/share/haxe/std/hl/_std/haxe/Exception.hx, line : 42, method : __constructor__}
    line: 255, _without_ throws: {file : /usr/local/share/haxe/std/haxe/CallStack.hx, line : 32, method : callStack} is expected, but got {file : /usr/local/share/haxe/std/haxe/ValueException.hx, line : 24, method : __constructor__}
    line: 255, _without_ throws: {file : /usr/local/share/haxe/std/haxe/CallStack.hx, line : 22, method : callStack} is expected, but got {file : unit/TestExceptions.hx, line : 46, method : __constructor__}
    line: 255, _without_ throws: {file : /usr/local/share/haxe/std/haxe/CallStack.hx, line : 12, method : callStack} is expected, but got {file : unit/TestExceptions.hx, line : 42, method : __constructor__}
    line: 255, _without_ throws: {file : /usr/local/share/haxe/std/haxe/CallStack.hx, line : 2, method : callStack} is expected, but got {file : /usr/local/share/haxe/std/hl/_std/haxe/Exception.hx, line : 29, method : thrown}

EDIT: I think I've fixed it? It seems to behave like other targets again, and the test passes.

@Apprentice-Alchemist
Copy link
Contributor Author

I've added an AtomicBool type (this is just a wrapper around AtomicInt, except on Java).
I've also implemented atomics for C# again (this time simply using __cs__).

@Simn
Copy link
Member

Simn commented Nov 21, 2022

Could you resolve the conflict?

@Apprentice-Alchemist
Copy link
Contributor Author

Resolved the conflict, everything should be working now.

Hashlink test failure is due to the new native Int64 impl.
(Which was not tested previously due to hl-ver being set to 1.11.0)

@Apprentice-Alchemist
Copy link
Contributor Author

Fixed the issues with the new Int64 stuff in #10860 and HaxeFoundation/hashlink#574.

@Simn Simn merged commit 7c7f284 into HaxeFoundation:development Nov 24, 2022
@Simn
Copy link
Member

Simn commented Nov 24, 2022

Thank you for your work and patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants