PDA

View Full Version : Doubt about deep copy using QByteArray::append() and QByteArray::fromRawData()



Momergil
8th May 2014, 20:26
Hello!

Simple question: are the next two codes equal?

1:


QByteArray bytesReceived;
char bytesReceivedTemp[SOCKET_BUFFER_SIZE];

//fill bytesReceivedTemp

bytesReceived.append(QByteArray::fromRawData(bytes ReceivedTemp,SOCKET_BUFFER_SIZE));


2:


QByteArray bytesReceived;
char bytesReceivedTemp[SOCKET_BUFFER_SIZE];

//fill bytesReceivedTemp

bytesReceived.append(bytesReceivedTemp,SOCKET_BUFF ER_SIZE);


This question is related to that problem of doing a deep copy or having two objects sharing the same data. The QByteArray::fromRawData() states that it performs a "shallow copy" of bytesReceivedTemp's data to the returned QByteArray, needing the first one to be valid while using the second and only for reading (if a modification is attempted, Qt will make a deep copy of the data). Qt Assistant also says that when append() is used against a empty QByteArray (the above situation, different from QByteArray bytesReceived = "";), it also performs a "shallow copy" of the appended data instead of a deep one. It would seem, therefore, that the first code is quite redundant if it is performed only once, since it's performing a "shallow copy" of another "shallow copy", while the second is not (in a second iteration with a persistent (e.g. static) bytesReceived which isn't empty anymore would make the first code perform only one "shallow copy" while the second would make a deep one).

Am I correct in all what I sad? Which would be the smartest way of handling this in a embedded development situation, when "shallow copies" are preferred over deep ones, and if bytesReceived is handled as mentioned in the last parenthesis?


Thanks,

Momergil

ChrisW67
9th May 2014, 02:20
No the two are not equal. If bytesReceived is empty then the first option merely points at the existing buffer and the second immediately makes a copy of the buffer into its own storage. If bytesReceived is not empty at the time of the append() call then both copy the data out of the existing buffer into their own storage.


Which would be the smartest way of handling this in a embedded development situation,
That depends entirely on what you intend to do with the bytesReceived QByteArray. You don't tell us, but in your code it is non-const and therefore we assume it will be modified. In that case there will be a deep copy of the original data anyway.

Momergil
9th May 2014, 13:39
Thanks Chris for the reply.


... and the second immediately makes a copy of the buffer into its own storage.

Really? Well, that certainly was not what I understood from Qt Assistant, which seems to hold that the second would also merely points at the existing buffer if bytesReceived is empty:


Note: QByteArray is an implicitly shared class. Consequently, if this is an empty QByteArray, then this will just share the data held in ba. In this case, no copying of data is done, taking constant time. If a shared instance is modified, it will be copied (copy-on-write), taking linear time.

If this is not an empty QByteArray, a deep copy of the data is performed, taking linear time.

I guess Qt Assistant is not much clear in this case... (Note: using Qt 4.8)


If bytesReceived is not empty at the time of the append() call then both copy the data

In other words, if bytesReceived is not empty, than calling QByteArray::fromRawData() is useless? o.O Is there a way to "correct" this situation?


in your code it is non-const and therefore we assume it will be modified.

Actually it's non-constant to avoid instantiating a new QByteArray each time new data arrives, but the bytesReceived is not supposed to be modified with the exception to a call of QByteArray::clear(), which AFAIK wouldn't make a call to the copy-on-write since it's not editing the data inside the QByteArray, but only the QByteArray itself (please correct me if I'm wrong!).

ChrisW67
10th May 2014, 22:42
OK, I misread your original options as:


QByteArray bytesReceived;
char bytesReceivedTemp[SOCKET_BUFFER_SIZE];

bytesReceived.append(QByteArray::fromRawData(bytes ReceivedTemp,SOCKET_BUFFER_SIZE)); // 1
bytesReceived.append(QByteArray(bytesReceivedTemp, SOCKET_BUFFER_SIZE)); // 2

In this case your two options use two different methods of constructing the QByteArray that you append to the array you declared. These two methods differ: option 2 copies immediately the other does not. By the time append() runs on the empty target array the copy has already been made.

Your second option was actually:


bytesReceived.append(bytesReceivedTemp,SOCKET_BUFF ER_SIZE); // 2

This version of append() never constructs a QByteArray to append, so you never get the implicit sharing of QByteArray buffers. It simply reallocates the internal buffer of the target QByteArray, reallocating space if required, and copies the specified bytes onto the end of the buffer.


You cannot "correct" the situation. If the target array already contains data then append() must copy the content of its argument onto the end of its buffer (allocating or reallocating memory as required). There is no magical way to make two independent buffers contiguous in memory to "append" without copying.

Momergil
12th May 2014, 22:59
Hello Chris and thanks again for the reply!

So let me see if I understood your now corrected explanation with a example (particularly the situation I want now to apply all this).

I have a socket connection from which data arrives into a char[]-buffer. I want to construct a temporary QByteArray that will never be modified (constant), only to manipulate these data as a QByteArray instead of a char[]. For this, I want to do a shallow copy, so I may use both options



char bytesReceivedTemp[SOCKET_BUFFER_SIZE];

const QByteArray bytesReceived = QByteArray::fromRawData(bytesReceivedTemp,SOCKET_B UFFER_SIZE)); //Option 1

const QByteArray bytesReceived(bytesReceivedTemp,SOCKET_BUFFER_SIZE ); //Option 2


, for both o them use QByteArray's implicit sharing of data. For this, I simply can't use append(), for in doing it I'll necessarily make a deep copy (what still seems to go against Qt Assistant's append() instructions, but that's what I understood of your explanations



(...)option 2 copies immediately(...)
(...)copies immediately the other does not(...) //which means, it copies later with append()
(...) and copies the specified bytes onto the end of the buffer. // For bytesReceived.append(bytesReceivedTemp,SOCKET_BUFF ER_SIZE)


). But what may happen is that the data package received in the socket system may come in fractions, so I'll need to store the already received data in a different QByteArray (now a deep copy) and wait for the new data pack from the socket. When it arrives, I should create a new QByteArray with a shallow copy to the first part of the pack plus a shallow copy of the new data recently received. The resulting template code:



QByteArray previousData;
char bytesReceivedTemp[SOCKET_BUFFER_SIZE];
memset(&bytesReceivedTemp,'\0',SOCKET_BUFFER_SIZE);

while(true)
{
socket_read(bytesReceivedTemp). //simplified

const QByteArray bytesReceived(previousData + QByteArray::fromRawData(bytesReceivedTemp,SOCKET_B UFFER_SIZE)); //bytesReceived with two shallow copies

switch(bytesReceived.at(0))
{
case PACK_HEADER_1: //identifies the pack
{
if (bytesReceived.size() < PACK_HEADER_1_SIZE) //verify if pack is fragmented
{
previousData = bytesReceived.mid(0,bytesReceived.size()); //Performs a deep copy of the current fraction of the pack
continue;
}

//Process...
}

//...
}
}


Note: obviously the algorithm is simplified.


Thanks,

Momergil

ChrisW67
14th May 2014, 10:44
Hello Chris and thanks again for the reply!

So let me see if I understood your now corrected explanation with a example (particularly the situation I want now to apply all this).

I have a socket connection from which data arrives into a char[]-buffer. I want to construct a temporary QByteArray that will never be modified (constant), only to manipulate these data as a QByteArray instead of a char[]. For this, I want to do a shallow copy, so I may use both options



char bytesReceivedTemp[SOCKET_BUFFER_SIZE];

const QByteArray bytesReceived = QByteArray::fromRawData(bytesReceivedTemp,SOCKET_B UFFER_SIZE)); //Option 1

const QByteArray bytesReceived(bytesReceivedTemp,SOCKET_BUFFER_SIZE ); //Option 2


No, the constructor you are using in Option 2 makes a deep copy outright. You want option 1.



But what may happen is that the data package received in the socket system may come in fractions, so I'll need to store the already received data in a different QByteArray (now a deep copy) and wait for the new data pack from the socket. When it arrives, I should create a new QByteArray with a shallow copy to the first part of the pack plus a shallow copy of the new data recently received. The resulting template code:



QByteArray previousData;
char bytesReceivedTemp[SOCKET_BUFFER_SIZE];
memset(&bytesReceivedTemp,'\0',SOCKET_BUFFER_SIZE);

while(true)
{
socket_read(bytesReceivedTemp). //simplified

const QByteArray bytesReceived(previousData + QByteArray::fromRawData(bytesReceivedTemp,SOCKET_B UFFER_SIZE)); //bytesReceived with two shallow copies


You are constructing a QByteArray bytesReceived that shares its internals structure (implicit sharing) with a temporary QByteArray (that is promptly destroyed). The temporary byte array is the one returned by QByteArray::operator+() after it creates an internal buffer containing the content of previousData and bytesReceivedTemp. You have not avoided a deep copy (and cannot). You might as well just:


previousData.append(QByteArray::fromRawData(bytesR eceivedTemp,SOCKET_BUFFER_SIZE));

Momergil
14th May 2014, 13:14
You have not avoided a deep copy (and cannot).

Really? :( Is there no other way of creating const QByteArray bytesReceived pointing to previousData and QByteArray::fromRawData(...) that avoids deep copy? :(

Well, than all this effort was for almost nothing \o/ (almost because, the way it was, it seems I had some memory problems that seems to have been solved with this conversation).


Well, life is hard :P

Thanks, then, Chris for all.


Momergil