Skip to content

rework std.atomic#18085

Merged
andrewrk merged 4 commits intomasterfrom
std-atomics
Nov 23, 2023
Merged

rework std.atomic#18085
andrewrk merged 4 commits intomasterfrom
std-atomics

Conversation

@andrewrk
Copy link
Member

  • remove std.atomic.Queue
  • remove std.atomic.Stack
  • remove std.atomic.Ordering
    • it is provided by the language with std.builtin.AtomicOrder.
  • remove std.atomic.fence
    • it is provided by the language with @fence.
  • remove std.atomic.compilerFence
    • if this is desired, it should be a language feature, not a standard
      library function with inline asm.
  • move std.atomic.Atomic to std.atomic.Value
  • fix incorrect argument order passed to testing.expectEqual
  • make the functions be a thin wrapper over the atomic builtins and
    stick to the naming conventions.
  • remove pointless functions loadUnchecked and storeUnchecked. Instead,
    name the field raw instead of value (which is redundant with the
    type name).
  • simplify the tests by not passing every possible combination. Many
    cases were iterating over every possible combinations but then not
    even using the for loop element value!
  • remove the redundant compile errors which are already implemented by
    the language itself.
  • remove dead x86 inline assembly. this should be implemented in the
    language if at all.

* remove `std.atomic.Ordering`
  - it is provided by the language with `std.builtin.AtomicOrder`.
* remove `std.atomic.fence`
  - it is provided by the language with `@fence`.
* remove `std.atomic.compilerFence`
  - if this is desired, it should be a language feature, not a standard
    library function with inline asm.
This was originally supposed to be a lock-free stack, but I gave up on
that and made it be a thread-safe stack which is implemented poorly
using spin locks. Nobody should use this data structure.

The alternative is a normal stack protected by a mutex.
This was originally supposed to be a lock-free queue, but I gave up on
that and made it be a thread-safe queue instead.

Putting the mutex directly inside the queue data structure makes it
non-composeable. Instead, the recommendation is to use a normal queue
protected by an external mutex.
* move std.atomic.Atomic to std.atomic.Value
* fix incorrect argument order passed to testing.expectEqual
* make the functions be a thin wrapper over the atomic builtins and
  stick to the naming conventions.
* remove pointless functions loadUnchecked and storeUnchecked. Instead,
  name the field `raw` instead of `value` (which is redundant with the
  type name).
* simplify the tests by not passing every possible combination. Many
  cases were iterating over every possible combinations but then not
  even using the for loop element value!
* remove the redundant compile errors which are already implemented by
  the language itself.
* remove dead x86 inline assembly. this should be implemented in the
  language if at all.
@andrewrk andrewrk added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. standard library This issue involves writing Zig code for the standard library. labels Nov 23, 2023
@andrewrk andrewrk requested a review from kprotty as a code owner November 23, 2023 02:16
@xdBronch
Copy link
Contributor

why remove Queue and Stack?

@andrewrk
Copy link
Member Author

Have a look at the respective commit messages

@xdBronch
Copy link
Contributor

sorry, didnt think to check that

@andrewrk andrewrk enabled auto-merge November 23, 2023 02:45
return @atomicRmw(T, &self.raw, .Xchg, operand, order);
}

pub inline fn cmpxchgWeak(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this was renamed to the builtin, why not rename swap as well?

@andrewrk andrewrk merged commit 2bffd81 into master Nov 23, 2023
@andrewrk andrewrk deleted the std-atomics branch November 23, 2023 09:55
llogick added a commit to zigtools/zls that referenced this pull request Nov 26, 2023
* zig std had removed meta.trait

ps: this change need set minimum zig build version to 0.12.0-dev.1686+d5e21a4f1

* std.fs: split Dir into IterableDir

ziglang/zig#12060

* rework std.atomic

ziglang/zig#18085

* set minimum build version to 0.12.0-dev.1710+2bffd8101

* Update binned_allocator.zig

* build.zig.zon: Update `known-folders`

---------

Co-authored-by: nullptrdevs <16590917+nullptrdevs@users.noreply.github.com>
@xdBronch
Copy link
Contributor

xdBronch commented Dec 1, 2023

for when the time comes, this broke std.event (more than it probably already was). there are 34 combined uses of atomic's Queue and Stack

@andrewrk
Copy link
Member Author

andrewrk commented Dec 1, 2023

yeah I know. the event loop needs to be completely rewritten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Implementing this issue could cause existing code to no longer compile or have different behavior. standard library This issue involves writing Zig code for the standard library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants