4

I'm implementing my own queue which blocks on .pop(). This function also accepts additional argument which is a timeout. So at the moment I have such code:

template <class T>
class BlockingQueue {

private:
    std::queue<T> m_queue;
    std::mutex m_mutex;
    std::condition_variable m_condition;

public:
    T pop(uint64_t t_millis) {
        std::unique_lock<std::mutex> lock(m_mutex);
        auto status = m_condition.wait_for(
            lock,
            std::chrono::milliseconds(t_millis),
            [=] {
                return !m_queue.empty();
            }
        );
        if (!status) {
            throw exceptions::Timeout();
        }
        T next(std::move(m_queue.front()));
        m_queue.pop();
        return next;
    };
}

where exceptions::Timeout is my custom exception. Now I've been thinking about this exception throwing from the performance point of view. Would it be better to return some kind of return code from that function? How does that affect performance?

Also since .pop already returns something how would you implement additional return code? I suppose some new structure that holds both T and a return code would be needed. Is that increase in complexity really worth it?

Improve this question
14
  • 1
    [FYI] You may not want to return the queue item from pop: stackoverflow.com/questions/25035691/… Commented Jun 13, 2016 at 12:18
  • 1
    A pop() that returns a value and throws an exception? OK, then I guess. Commented Jun 13, 2016 at 12:18
  • 1
    @freakish I'd separate the functions into two bits: one that returns a bool called try_whatever (non-blocking), then your blocking pop() here. I don't think an exception is necessary here. Commented Jun 13, 2016 at 12:36
  • 1
    "How would you implement return code?" one way would be something like this: channel9.msdn.com/Shows/Going+Deep/… You can implement an expected<T> template class, which represents "either, successful construction of a value, or the error that prevented its creation" using e.g. boost::variant<T, error_code> if you want. It's just another style of error handling, it's efficient and doesn't add much complexity, although it might not mesh well with your existing code Commented Jun 13, 2016 at 12:37
  • 1
    "I've been thinking about this exception throwing from the performance point of view" -- that's a mistake. If you want to know about performance you need to measure it, not think about it. Quality of implementation can easily make an order-of-magnitude difference to the time taken to throw and catch an exception (or for that matter the time to acquire the lock in the successful case). Commented Jun 13, 2016 at 12:38

4 Answers 4

Reset to default
4

Throw exceptions when an expectation has not been met, return a status code when you're querying for status.

for example:

/// pops an object from the stack
/// @returns an object of type T
/// @pre there is an object on the stack
/// @exception std::logic_error if precondition not met
T pop();

/// queries how many objects are on the stack
/// @returns a count of objects on the stack
std::size_t object_count() const;

/// Queries the thing for the last transport error
/// @returns the most recent error or an empty error_code
std::error_code last_error() const;

and then there's the asio-style reactor route coupled with executor-based futures:

/// Asynchronously wait for an event to be available on the stack.
/// The handler will be called exactly once.
/// to cancel the wait, call the cancel() method
/// @param handler is the handler to call either on error or when
///        an item is available
/// @note Handler has the call signature void(const error_code&, T)
///
template<class Handler>
auto async_pop(Handler handler);

which could be called like this:

queue.async_pop(asio::use_future).then([](auto& f) {
  try {
    auto thing = f.get();
    // use the thing we just popped
  }
  catch(const system_error& e) {
    // e.code() indicates why the pop failed
  }
});
Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

If this is a single-consumer queue then logic_error is appropriate for calling it when empty. However, if it's a multi-consumer thread-safe queue then the caller has no means to guarantee that the queue is non-empty (TOCTOU), in which case it should probably throw something different.
Btw your proposed API still needs a timed wait function. Given what you have I'd suggest either void wait_non_empty(uint64_t t_millis); or std::size_t wait_non_empty(uint64_t t_millis); returning the object_count(). Or live dangerously and just call it wait instead of wait_non_empty. Hopefully it's obvious what condition you wait for on a queue!
@SteveJessop or go down the asio reactor route. template<Handler> auto async_pop(Handler&& handler);
1

One way to signal an error in a situation like this, without throwing an exception, would be to use something like Andrei Alexandrescu's expected<T> template.

He gave a nice talk about it a while back. The idea is, expected<T> either contains a T, or it contains an exception / error code object describing why the T couldn't be produced.

You can use his implementation, or easily adapt the idea for your own purposes. For instance you can build such a class on top of boost::variant<T, error_code> quite easily.

This is just another style of error handling, distinct from C-style integer error codes and C++ exceptions. Using a variant type does not imply any extra dynamic allocations -- such code can be efficient and doesn't add much complexity.

This is actually pretty close to how error handling is done in Rust idiomatically. c.f. 2 3

Improve this answer

Comments

1

Also since .pop already returns something how would you implement additional return code? I suppose some new structure that holds both T and a return code would be needed.

Going with this approach would put an extra requirement on the types that can be used with your BlockingQueue: they must be default constructible. It can be avoided if pop() returns the result through a std::unique_ptr (signaling the timeout with a nullptr), but that will introduce noticeable overhead.

I see no disadvantage of using exceptions here. If you are measuring your timeouts in milliseconds, then handling an exception in case of a timeout should be negligible.

Improve this answer

8 Comments

Compare throwing an exception to returning a bool. Both are negligible individually, but the difference will show up in high numbers.
But those "high" numbers will be nothing compared to the enormous numbers coming from timeouts.
@Leon: the best case with code that uses locks is no contention/blocking, in which case it doesn't matter how long the timeouts are set to, they're actually 0 and anything is big compared with 0. The worst case is that the locks are heavily contended and the app is CPU-bound, in which case anything saved will help. I agree that almost all C++ code needn't worry about the cost of exceptions, but comparing them against the timeout doesn't capture why: it has to be because they're cheap compared with other processing your app does.
Or to put it another way: suppose your timeout (on a socket, perhaps) is 900 seconds which is a typical TCP-level timeout. The server might actually respond much faster than that almost always, so assessing performance as "anything less than 15 minutes is irrelevant for internet apps" is incorrect. In the worst case, sure you have 15 minutes to get your house in order, but you might need to be more responsive than that in the common case.
@SteveJessop I think my reasoning is not fully understood. I am saying that if you could afford to wait for something for 15 minutes, then if it doesn't arrive after 15 minutes (no matter if that happens 1% or 99% of the calls) you can afford spending several seconds on the handling of that situation and shouldn't strive to be able to do it in a nanosecond.
|
0

An exception is not necessary here. A "timeout" is just as expected an outcome as getting an item from the queue. Without the timeout, the program is essentially equivalent to the halting problem. Let's say the client specified that they want an indefinite timeout. Would the exception ever throw? How would you handle such an exception (assuming you're still alive in this post-apocalyptic scenario?)

Instead I find these two design choices more logical (though they're not the only ones):

  • Block until an item is available. Create a function named wait that polls and returns false if it times out, or true when an item is available. The rest of your pop() function can remain unchanged.

  • Don't block. Instead return a status:

    • If the operation would block, return "busy"
    • If the queue is empty, return "empty"
    • Otherwise, you can "pop" and return "success"

Since you have a mutex, these options seem preferable to an i.e. non-waiting function.

Improve this answer

8 Comments

"Let's say the client specified that they want an indefinite timeout" - let's not exaggerate, the limit appears to be 584 million years
@SteveJessop Sounds enterprise-grade to me. I think most people would give up after 5 minutes.
Block until an item is available That would basically block the caller forever, I wouldn't be able to interrupt it. Consider this: I have a worker which pops an element from a queue. But some other thread might disable that worker. So I do it via is_running flag which has to be checked every now and then. This is were timeout comes in play. Also correct me if I'm wrong but won't both solutions fail due to TOCTOU in case of multiple consumers?
@freakish Block the caller forever On that current thread yes. Not sure how that's different from your current code. .. won't both solutions fail due to TOCTOU .. Outside the scope of the question. See C++0x has no semaphores? How to synchronize threads?
@sleeptightpupper: the current code does not block the thread forever, it only blocks it for t_millis milliseconds. If you're saying "never implement timed wait" then I think you're missing an important requirement of the question. But actually I think freakish has read "Block until an item is available" but ignored how in the next sentence you say, "returns false if it times out". So it doesn't block forever unless the caller asks it to
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.