Added til::spsc, a lock-free, single-producer/-consumer FIFO queue

[lhecker]

Summary of the Pull Request

This PR adds the til::spsc namespace, which implements a lock-free, single-producer, single-consumer FIFO queue ("channel"). The queue efficiently blocks the caller using Futexes if no data can be written to / read from the queue (e.g. using WaitOnAddress on Windows). Furthermore it allows batching of data and contains logic to signal the caller if the other side has been dropped/destructed.

PR Checklist

• Closes #xxx
• CLA signed. If not, go over here and sign the CLA
• Tests added/passed
• Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
• Schema updated.
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

til::spsc::details::arc<T> contains most of the queue's logic and as such has the relevant documentation for its design.

Validation Steps Performed

The queue was tested on Windows, Linux and macOS using MSVC, gcc and llvm and each of their available runtime introspection utilities in order to ensure no race conditions or memory leaks occur.

[beviu]

I have some questions about some things that are probably not very important. Feel free to ignore.

[lhecker]

BTW If you have a hard time understanding any part of the code, please let me know. I fully realize that the bit fiddling and atomics and stuff are kinda "meh" and this would allow me to improve the clarity of the code.
Also thanks again for your kind review @greg904! 🙂

[DHowett]

This is incredibly clever, and I love it. I only understood roughly 80% of it, but it seems like it fits the brief and it comes with tests, and it's a great and concise and cross-platform piece of code. I'd be exceptionally proud to have this in our codebase. 😄

[zadjii-msft]

@msftbot make sure @miniksa signs off on this one

[ghost]

Hello @zadjii-msft!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

• I'll only merge this pull request if it's approved by @miniksa

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

[zadjii-msft]

I'm 99% sure we never covered this in my data structures or operating systems classes, so I think I'm not totally comprehending how this works. This seems sufficiently like magic to me.

That being said, reading through the arc code wasn't as painful as it could have been, because there's more than enough comments. The only thing I want is more comments in the tests, and maybe more tests in general, as examples of how to use this.

This is great stuff, and I'm gonna sign off, but let's make sure @miniksa signs off too.

[miniksa]

I'm generally good with this.

The only thing I'm left not understanding here is the memory orders. I read it here and I read it on the usual suspect website (cplusplus.com) and I still don't understand it. If you have a way of simplifying it for memory-ordering-idiots like me, please document it where each one is chosen why it works. Right now, I just trust that you (and @greg904) do understand it and you've worked out the optimal choices.

I'll hit approve after you clean up the final round of comments so as to not set off the AutoMerge bot.

[miniksa]

Some notes from attempting to use this (that may turn into follow ons, not necessarily needed to merge this):

1. I'd love to push_n a std::wstring_view or std::string_view directly if the type is wchar_t or char.
2. When I do pop_n into a buffer, if it didn't fill the whole buffer... how much of it was used? std::distance of the output iterator returned (first half of pair) to what I originally gave in?

[DHowett]

Maybe that's just a push_n that takes anything that can .begin() and .end()

[lhecker]

@miniksa Yeah you’re right about your second point. The first value of the pair is the resulting iterator and std::distance will return the number of elements. If the second value of the pair is true (the sender hasn’t been dropped yet), pop_n will always return as many items as requested.
If you‘re using a std::string or similar container I suggest using a std::back_inserter as the first argument to pop_n.
I can add a method move() which accepts a single container argument with .begin/.end methods and uses that for sending items.

[miniksa]

What if I just want as many as are ready even if the sender hasn't dropped yet (only waiting if there are none?)

[miniksa]

Okay, I tried using it here but with strings/wstrings instead of characters because I couldn't just "consume what was left". So I consumed one string at a time.

https://github.com/microsoft/terminal/tree/dev/miniksa/gotta_go_fast_spsc

It's doing pretty well. It's obvious that with a bounded queue, there's a backup that happens so it's not "as fast" as my completely unbounded one. But I didn't measure the memory usage on the unbounded one and it was assuredly bad.

It is super simple to use, though. I used it very poorly in that branch and need to do better, but it proves the concept.

If we can get a "consume whatever you've got even if the producer still exists" mode to pop, I think we'll be pretty good.

And then my next phase will be identifying "simple text" on writing into the buffer and making that go faster as that's the new bottleneck...

[miniksa]

Just one last interesting thing before I leave for this evening... RtlpWakeByAddress seems to take a bit when it is called a bunch. I wonder if we need a throttle on the wake events like we've done throttling for other eventing?

[miniksa]

Just one last interesting thing before I leave for this evening... RtlpWakeByAddress seems to take a bit when it is called a bunch. I wonder if we need a throttle on the wake events like we've done throttling for other eventing?

I thought about it in the car this morning, and I'm guessing this is just a symptom of a consumer that doesn't have enough to do. The producer spends a lot of time waking it back up again because it keeps consuming the whole queue and going to sleep. Probably not something that the spsc_queue needs to do anything about.

[DHowett]

This documentation is excellent. Thank you so much

[DHowett]

Audit mode, however, hates it.

[miniksa]

Exactly what I was hoping for. Thank you.

[ghost]

Hello @miniksa!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.