Commit 587e4b4d authored by Delyan Kratunov's avatar Delyan Kratunov Committed by facebook-github-bot-4

Expose move result from LockFreeRingBuffer::Cursor

Summary: Without feedback that a `moveForward` or `moveBackward` did nothing, a user
cannot implement this idiom safely:

```
while(rb.tryRead(entry, cursor)) {
  doSomething(entry);
  cursor.moveBackward();
}
```

In the case where the ring buffer has not overflowed and slot 0 is still
available, the reader will get stuck at it (read it continuously) until
the buffer overflows.

This diff allows the above example to become:

```
while(rb.tryRead(entry, cursor)) {
  doSomething(etnry);
  if (!cursor.moveBackward()) {
    break;
  }
}
```

Reviewed By: bryceredd

Differential Revision: D2674975

fb-gh-sync-id: a0c5857daf186ef19e203f90acc2145590f85c3b
parent 78022b6e
...@@ -70,16 +70,24 @@ public: ...@@ -70,16 +70,24 @@ public:
struct Cursor { struct Cursor {
explicit Cursor(uint64_t initialTicket) noexcept : ticket(initialTicket) {} explicit Cursor(uint64_t initialTicket) noexcept : ticket(initialTicket) {}
void moveForward(uint64_t steps = 1) noexcept { /// Returns true if this cursor now points to a different
/// write, false otherwise.
bool moveForward(uint64_t steps = 1) noexcept {
uint64_t prevTicket = ticket;
ticket += steps; ticket += steps;
return prevTicket != ticket;
} }
void moveBackward(uint64_t steps = 1) noexcept { /// Returns true if this cursor now points to a previous
/// write, false otherwise.
bool moveBackward(uint64_t steps = 1) noexcept {
uint64_t prevTicket = ticket;
if (steps > ticket) { if (steps > ticket) {
ticket = 0; ticket = 0;
} else { } else {
ticket -= steps; ticket -= steps;
} }
return prevTicket != ticket;
} }
protected: // for test visibility reasons protected: // for test visibility reasons
......
...@@ -95,18 +95,16 @@ TEST(LockFreeRingBuffer, readsCanBlock) { ...@@ -95,18 +95,16 @@ TEST(LockFreeRingBuffer, readsCanBlock) {
// expose the cursor raw value via a wrapper type // expose the cursor raw value via a wrapper type
template<typename T, template<typename> class Atom> template<typename T, template<typename> class Atom>
uint64_t value(const typename LockFreeRingBuffer<T, Atom>::Cursor&& rbcursor) { uint64_t value(const typename LockFreeRingBuffer<T, Atom>::Cursor& rbcursor) {
typedef typename LockFreeRingBuffer<T,Atom>::Cursor RBCursor; typedef typename LockFreeRingBuffer<T,Atom>::Cursor RBCursor;
RBCursor cursor = std::move(rbcursor);
struct ExposedCursor : RBCursor { struct ExposedCursor : RBCursor {
ExposedCursor(const RBCursor& cursor): RBCursor(cursor) {} ExposedCursor(const RBCursor& cursor): RBCursor(cursor) {}
uint64_t value(){ uint64_t value(){
return this->ticket; return this->ticket;
} }
}; };
return ExposedCursor(cursor).value(); return ExposedCursor(rbcursor).value();
} }
template<template<typename> class Atom> template<template<typename> class Atom>
...@@ -243,4 +241,22 @@ TEST(LockFreeRingBuffer, cursorFromWrites) { ...@@ -243,4 +241,22 @@ TEST(LockFreeRingBuffer, cursorFromWrites) {
EXPECT_EQ(3, cursorValue(rb.writeAndGetCursor(val))); EXPECT_EQ(3, cursorValue(rb.writeAndGetCursor(val)));
} }
TEST(LockFreeRingBuffer, moveBackwardsCanFail) {
const int capacity = 3;
LockFreeRingBuffer<int> rb(capacity);
// Workaround for template deduction failure
auto (&cursorValue)(value<int, std::atomic>);
int val = 0xfaceb00c;
rb.write(val);
rb.write(val);
auto cursor = rb.currentHead(); // points to 2
EXPECT_EQ(2, cursorValue(cursor));
EXPECT_TRUE(cursor.moveBackward());
EXPECT_TRUE(cursor.moveBackward()); // now at 0
EXPECT_FALSE(cursor.moveBackward()); // moving back does nothing
}
} // namespace folly } // namespace folly
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment