Rework synchronization primitives - Futex edition #8084
Conversation
- linux futex Mutex is still broken
| pub fn timestamp() i64 { | ||
| return @divFloor(milliTimestamp(), ms_per_s); | ||
| } | ||
|
|
||
| /// Get a calendar timestamp, in milliseconds, relative to UTC 1970-01-01. | ||
| /// Get a calendar timestamp, in nanoseconds, relative to UTC 1970-01-01. |
There was a problem hiding this comment.
milliTimestamp returns nanoseconds?
| pub usingnamespace if (@hasDecl(root, "sync")) | ||
| root.sync | ||
| else if (std.builtin.single_threaded) | ||
| primitives.debug |
There was a problem hiding this comment.
Why is this debug rather than core?
There was a problem hiding this comment.
core is for algorithms, the "Debug" impls are the "serial" ones. Unfortunate naming.
| .{generic.forFutex(os)}, | ||
| .{generic.forFutex(spin)}, | ||
| }) |futex| { | ||
| // @compileError("TODO: test wait/wake/nanotime"); |
There was a problem hiding this comment.
| // @compileError("TODO: test wait/wake/nanotime"); | |
| return error.SkipZigTest; // TODO: test wait/wake/nanotime |
|
|
||
| const builtin = std.builtin; | ||
| const assert = std.debug.assert; | ||
| const helgrind: ?type = if (builtin.valgrind_support) std.valgrind.helgrind else null; |
There was a problem hiding this comment.
You don't need this check, the valgrind module already does it.
| } | ||
|
|
||
| /// Returns true if the system's high performance timer implementation is actually monotonic. | ||
| /// For some platforms, this is not always the case: https://doc.rust-lang.org/src/std/time.rs.html#227 |
There was a problem hiding this comment.
| /// For some platforms, this is not always the case: https://doc.rust-lang.org/src/std/time.rs.html#227 | |
| /// For some platforms, this is not always the case: https://doc.rust-lang.org/1.50.0/src/std/time.rs.html#227 |
There was a problem hiding this comment.
future versions of rust may move that file/code, so it is better(IMO) to link to a permlink
| @@ -1737,3 +1737,56 @@ pub const IOCPARM_MASK = 0x1fff; | |||
| fn ior(inout: u32, group: usize, num: usize, len: usize) usize { | |||
| return (inout | ((len & IOCPARM_MASK) << 16) | ((group) << 8) | (num)); | |||
| } | |||
|
|
|||
| pub const KERN_SUCCESS = 0; | |||
There was a problem hiding this comment.
This looks like it should be an enum
| /// This uses the same method as seen in Amanieu's port of WTF::ParkingLot: | ||
| /// https://github.com/Amanieu/parking_lot/blob/master/core/src/parking_lot.rs | ||
| fn from(address: usize) *WaitBucket { | ||
| const seed = @truncate(usize, 0x9E3779B97F4A7C15); |
There was a problem hiding this comment.
huh... googling this number shows that it is 2^64 * phi and seems to have a long history of use in wait queues
| } | ||
|
|
||
| pub fn notifyOne(self: *Self) void { | ||
| if (helgrind) |hg| { |
There was a problem hiding this comment.
As mentioned in another comment: no need for this check.
|
@kprotty could you provide status update on this PR, i.e., do you plan on doing more work on this branch and hopefully merging this upstream in the near future? Or is there a blocker for it or something that I might have missed? I'd be very excited to see this merged. |
|
@kubkon Late response, but there are a few blockers:
|
Ah, gotcha. Thanks for the explanation! |
|
Is this going to be mergeable anytime soon? If not it woud probably be better to close it so that it isn't cluttering the pull requests. |
|
@kprotty you might need to strategize a bit so as to not get overwhelmed and figure out how to break this into smaller, independently mergeable pieces |
@Vexu Most likely no. Its ok to close this given:
@andrewrk I've been taking this advice. Introduced atomic stuff in #8866, adding Futex in #9070, sync primitives based on Futex after that, then |
See #7800 for initiative and reason for branching into new PR. Don't intend to really add more changes ATM but am willing to accept anymore that come in.