Most efficient method of copying std::deque contents to byte-array

Question

Is there a better way to copy the contents of a std::deque into a byte-array? It seems like there should be something in the STL for doing this.

// Generate byte-array to transmit
uint8_t * i2c_message = new uint8_t[_tx.size()];
if ( !i2c_message ) {
    errno = ENOMEM;
    ::perror("ERROR: FirmataI2c::endTransmission - Failed to allocate memory!");
} else {
    size_t i = 0;

    // Load byte-array
    for ( const auto & data_byte : _tx ) {
        i2c_message[i++] = data_byte;
    }

    // Transmit data
    _marshaller.sendSysex(firmata::I2C_REQUEST, _tx.size(), i2c_message);
    _stream.flush();

    delete[] i2c_message;
}

I'm looking for suggestions for either space or speed or both...

EDIT: It should be noted that _marshaller.sendSysex() cannot throw.

FOLLOW UP:

I thought it would be worth recapping everything, because the comments are pretty illuminating (except for the flame war). :-P

The answer to the question as asked...

Use std::copy

The bigger picture:

Instead of simply increasing the raw performance of the code, it was worth considering adding robustness and longevity to the code base.

I had overlooked RAII - Resource Acquisition is Initialization. By going in the other direction and taking a slight performance hit, I could get big gains in resiliency (as pointed out by @PaulMcKenzie and @WhozCraig). In fact, I could even insulate my code from changes in a dependency!

Final Solution:

In this case, I actually have access to (and the ability to change) the larger code base - often not the case. I reevaluated* the benefit I was gaining from using a std::deque and I swapped the entire underlying container to a std::vector. Thus saving the performance hit of container swapping, and gaining the benefits of contiguous data and RAII.

*I chose a std::deque because I always have to push_front two bytes to finalize my byte-array before sending. However, since it is always two bytes, I was able to pad the vector with two dummy bytes and replace them by random access - O(n) time.

I've only seen `std::copy` used to copy from array to container, not the opposite. — Zak, Jul 22 '17 at 23:49
First, i'd use a `std::vector`. second, I'd use the iterator constructor to build said-same. That eliminates most of this code, including giving you [reliable RAII](http://en.cppreference.com/w/cpp/language/raii). the `data()` member of the vector, or address-of-first-element, would grant you the pointer to contiguous data you seem to be seeking. — WhozCraig, Jul 22 '17 at 23:50
Supposed that `std::deque` is based on a `std::vector`you can use that equally well. — user0042, Jul 22 '17 at 23:51
@user0042 `std::deque` is not mandated by the standard to be `std::vector` based. And it would make little sense that it would be, as the two container types excel at several tasks very differently. Both are random accessible containers, but deques are specialized for fast end (front or back) insertion and removal, while vectors offer guaranteed continuity; something not offered by a deque, and somewhat the point of why the OP is doing this. — WhozCraig, Jul 22 '17 at 23:55
@WhozCraig _"std::deque is not mandated by the standard to be `std::vector` based."_ Thank you, I well know that, that's why I said _Supposed ..._. — user0042, Jul 22 '17 at 23:58
@Zak The RAII is important, since you didn't mention whether `_marshaller.sendSysex` can throw an exception. If it does `throw`, then you have a memory leak due to the `delete []` call never being reached. Using RAII prevents that from happening. — PaulMcKenzie, Jul 23 '17 at 00:07
@Zak *'ve only seen std::copy used to copy from array to container, not the opposite* -- You mean you haven't seen [something like this](http://coliru.stacked-crooked.com/a/15ecd72cd27ac986)? — PaulMcKenzie, Jul 23 '17 at 00:08
@user0042 good you know, because I'd be curious what implementation ever used a std::vector-based deque, and if they did, how on earth they managed the O(1) constant time front-end insertion, which *is* mandated by the standard. If you ever run across such an implementation, I'd definitely be interested in seeing how they did it. — WhozCraig, Jul 23 '17 at 00:09
@WhozCraig @user0042 please PM to settle who was right. `_marshaller.sendSysex` cannot throw, I'll make a note in the post. — Zak, Jul 23 '17 at 00:13
@PaulMcKenzie Yes. I haven't seen that as a matter of fact. That is great! I'll probably use it. Thank you. Also, thanks @user0042 for mentioning `std::copy`. — Zak, Jul 23 '17 at 00:15
@Zak -- Right now, that function doesn't throw, but if it is ever updated to `throw`, or you introduce code before the `delete` that throws, then you're back to square one. Might as well use RAII now -- it won't harm and will prevent any future surprises. — PaulMcKenzie, Jul 23 '17 at 00:16
A deque is not really an efficient way of storing bytes. I bet you vector is 2x faster. — Michaël Roy, Jul 23 '17 at 00:19

score 7 · Accepted Answer · edited Jul 23 '17 at 05:31

Embrace the C++ standard library. Assuming _tx is really a std::deque<uint8_t>, one way to do this is simply:

std::vector<uint8_t> msg(_tx.cbegin(), _tx.cend());
_marshaller.sendSysex(firmata::I2C_REQUEST, msg.size(), msg.data());

This allocates the proper size contiguous buffer, copies the contents from the source iterator pair, and then invokes your send operation. The vector will be automatically cleaned up on scope-exit, and an exception will be thrown if the allocation for building it somehow fails.

The standard library provides a plethora of ways to toss data around, especially given iterators that mark where to start, and where to stop. Might as well use that to your advantage. Additionally, letting RAII handle the ownership and cleanup of entities like this rather than manual memory management is nearly always a better approach, and should be encouraged.

In all, if you need continuity (and judging by the looks of that send-call, that's exactly why you're doing this), then copying from non-contiguous to contiguous space is pretty much your only option, and that takes space and copy-time. Not much you can do to avoid that. I suppose peeking into the implementation specifics of std::deque and possibly doing something like stacking send-calls would be possible, but I seriously doubt there would be any reward, and the only savings would likely evaporate in the multi-send invokes.

Finally, there is another option that may be worth considering. Look at the source of all of this. Is a std::deque really warranted? For example, certainly your building that container somewhere else. If you can do that build operation as-efficient, or nearly-so, using a std::vector, then this entire problem goes away, as you can just send that.

For example, if you knew (provably) that your std::deque would never be larger than some size N, you could, pre-size a std::vector or similar continuous RAII-protected allocation, to be 2*N in size, start both a fore and aft iterator pair in the middle and either prepend data by walking the fore iterator backward, or append data by walking the aft iterator forward. In the end, your data will be contiguous between fore and aft, and the send is all that remains. no copies would be required, though added-space is still required. This all hinges on knowing with certainty the maximum message size. If that is available to you, it may be an idea worth profiling.

RAII is a great point. I didn't like the allocation, and that was what prompted me to ask the question. However, I know this is not more space efficient and I doubt it is more speed efficient, so while it is good advice, it doesn't really answer the question. — Zak, Jul 23 '17 at 00:20
@Zak if you need continuity (and judging by the looks of that send-call, that's exactly why you're doing this), then copying from non-contiguous to contiguous space is pretty much your only option, and that takes space and copy-time. Not much you can do to avoid that. I suppose peeking into the implementation specifics of `std::deque` and possibly doing something like stacking send-calls would be possible, but I seriously doubt there would be any reward, and the only savings would likely evaporate in the multi-send invokes. — WhozCraig, Jul 23 '17 at 00:26
Paste that into your answer and I'll give you the checkmark. Also thanks for the great, complete and well-thought out conversation. — Zak, Jul 23 '17 at 00:27
@Zak Done, along with an interesting mind-code you may find of use. Best of luck. — WhozCraig, Jul 23 '17 at 00:36

Most efficient method of copying std::deque contents to byte-array

1 Answers1