userver icon indicating copy to clipboard operation
userver copied to clipboard

Consider renaming NonFifoQueues

Open itrofimow opened this issue 3 years ago • 3 comments

I feel like NonFifo is very unclear in terms of what a queue actually represents and leaves too wide of a room for an interpretation: stack is NonFifo, dequeing random element is NonFifo etc. Maybe something in the lines of CasualOrder/NonLinearizable would fit better?

To add to that, NonFifoSp[s|m]cQueue is actually fifo, because it serializes writes through a single producer token, as suggested in moodycamel documentation:

However, if for some reason you do extra explicit synchronization between the two producer threads yourself, thus defining a total order between enqueue operations, you might expect that the elements would come out in the same total order, which is a guarantee my queue does not offer. At that point, though, there semantically aren't really two separate producers, but rather one that happens to be spread across multiple threads. In this case, you can still establish a total ordering with my queue by creating a single producer token, and using that from both threads to enqueue (taking care to synchronize access to the token, of course, but there was already extra synchronization involved anyway).

There also exists https://github.com/cameron314/readerwriterqueue, which might be a better fit for spsc queue version

itrofimow avatar Aug 02 '22 03:08 itrofimow

Hi! IMO, it's acceptable to leave NonFifo Queue name as-is. It follows from their name that they generally aim for queue (FIFO) semantics, but sometimes shuffle the elements in a difficult-to-predict order.

It may still worth it to document, in which situations FIFO order can be violated, namely that items from different producers may be consumed in unspecified order, but items from the same producer are always consumed in the order of Push.

NonFifo suffixes have just been dropped where possible, see this commit.

Could you please split the optimized SPSC queue into a separate issue?

Anton3 avatar Aug 02 '22 12:08 Anton3

Hm, i'm pretty sure that it was still NonFifoSpscQueue when i created the issue, however your commit seems to be pushed 10 hours earlier – is that an artifact of the internal->external github synchronization you guys maintain?

Thinking more about NonFifo part - i see your point and maybe it's indeed better to just clean up a confusion in documentation, leaving name as is.

I'll file a separate issue for optimized Spsc version (edit: #71)

itrofimow avatar Aug 02 '22 12:08 itrofimow

is that an artifact of the internal->external github synchronization you guys maintain?

Yes, some work is still needed to complete the migration from the internal repo.

Anton3 avatar Aug 02 '22 13:08 Anton3

Gkia.785

swswo avatar Sep 08 '22 06:09 swswo