max0x7ba / atomic_queue

C++ lockless queue.
MIT License
1.47k stars 176 forks source link

Add unsafe AtomicQueueB2::data() accessor for bulk reads #52

Closed KitsuneAlex closed 1 year ago

KitsuneAlex commented 1 year ago

As the title mentions, this allows accessing the internal data pointer of a dynamic atomic queue for bulk read operations. Making sure the call is safe should be up to the implementor in my opinion to reduce overhead as much as possible.

max0x7ba commented 1 year ago

As the title mentions, this allows accessing the internal data pointer of a dynamic atomic queue for bulk read operations. Making sure the call is safe should be up to the implementor in my opinion to reduce overhead as much as possible.

What is the intended use case for this, please?

KitsuneAlex commented 1 year ago

The specific reason i'd love to see this merged is since i am currently writing a collection stream library, i have to construct a given container type C as the result of a function, where one of the aformentioned queues is used as an intermediary buffer. However, i'd like to fill the resulting container from said queue using std::memcpy as a little optimization.

KitsuneAlex commented 1 year ago

Instead of directly exposing a pointer to the data, i could also add a simple function which copies the queue into a given container type instead. That would also be a possibility. (Edit for example)

template<template<typename, typename...> typename C>
[[nodiscard]] C<T> copy_as() const noexcept {
    C<T> result;
    std::memcpy(result.data(), elements_, was_size());
    return result;
}

Of course this example doesn't take into account concepts if you're into those, but you get the point.

max0x7ba commented 1 year ago

Instead of directly exposing a pointer to the data, i could also add a simple function which copies the queue into a given container type instead. That would also be a possibility. (Edit for example)

template<template<typename, typename...> typename C>
[[nodiscard]] C<T> copy_as() const noexcept {
    C<T> result;
    std::memcpy(result.data(), elements_, was_size());
    return result;
}

Of course this example doesn't take into account concepts if you're into those, but you get the point.

Your contribution would need to provide such a member function for the other 3 containers out 4, along with documentation stating the semantics, layout and guarantees.

The elements of the queue are not guaranteed to be stored in contiguous linear order, so that exposing a pointer to the ring-buffer array is just an invitation for the users to shoot themselves in the foot, as the above code snippet does.

You don't need a special shotgun from this library to shoot your entire leg off, the "standard" shotgun should be good enough for you:

#define class struct
#define private public
#define protected public
#include <atomic_queue/atomic_queue.h>
max0x7ba commented 1 year ago

An thread-unsafe iterator range to existing queue elements would be the right solution and the standard containers have iterator range constructors.

KitsuneAlex commented 1 year ago

Fair point for this specific example case, but i honestly think using iterator range constructors everytime can be an unneeded level of indirection in some situations, especially if the target is NOT a standard container but a raw memory buffer. I don't want to allocate a temporary standard container everytime i wanna access data via pointer. Most standard containers have a data accessor too.

max0x7ba commented 1 year ago

Fair point for this specific example case, but i honestly think using iterator range constructors everytime can be an unneeded level of indirection in some situations, especially if the target is NOT a standard container but a raw memory buffer.

You may like to provide a well-formed example which proves your point.

I don't want to allocate a temporary standard container everytime i wanna access data via pointer.

To copy elements from an iterator range you need to use std::copy or std::uninitialized_copy. No need to invoke a range constructor of any standard container.

Most standard containers have a data accessor too.

Out of 15+ standard library containers (not container adapters) only 3 provide data() member function to access the underlying array along with the guarantee of a specific order of elements. These queues cannot provide such a guarantee.

All standard containers provide iterator range access, on the other hand.

You are clutching at the straws here.

KitsuneAlex commented 1 year ago

Okay, you're right. Consider what i said irrelevant.