QByteArray data sharing and ownership
-
Thanks for your answers, I will answer and be more clear about my use cases.
But first, I have investigated a little further on the strange modification of the const QByteArray that get's modified when it is copied (it had absolutely nothing to do with the moc or metaobject construction). When a QByteArray is copied from one object to another is is shared. At least up to the point when you modify it in any way. This is acceptable of course, but one doesn't immediately realize that taking the pointer from the original array after it has been shared, will immediately cause a detach() (providing the pointer is shared) and a realloc of the object, loosing any earlier reserve (capacity) settings on it (well, it COULD in principle have preserved the capacity there). So, even if this is understandable the behavior is somewhat subtle.
Now my use case is to keep a certain buffer size of the array to be filled as I hand out the QByteArray's pointer to a libusb outstanding request. As the request returns (with the ACTUAL size) i modify the size of the QByteArray to the real one (this is where the resize function was not trustful, it would resize but it would reallocate sometime and also change the capacity of the array. Now further, within this libusb execution context the array data is to be sent to another thread for queuing and later processing, making a copy is ok on the emit call, the signal signature looks like this:
void bytesReceived(const EndPoint& ep, const QByteArray& data);
And after that emit, immediately resubmit the QByteArray's buffer by handing out the pointer the libusb request structure.
To my surprise, the emit would cause the QByteArray (which was passed to the signal as a const ref) become modified right after the data() call is made on the original array that was passed as const (and this is because of the copy being made from it). Here is where the sharing of the array becomes strange in my opinion, something being copied from it while it was given as a const has suddenly got its contents modified, and this is because I later take out the pointer from it (because the array has been shared now, but is being detached and reallocated).
Ironically, if I change the signature to pass the array by value, it is completely unchanged (even same capacity) when I afterwards take out the data pointer. This is totally explainable, it is however from a user point of view sort of weird.So, even though I do understand the mechanisms going on here and why they are there, I feel there is little or no use at all for the capacity/reserve support , at when it comes to the ability to use the data pointer for buffer management. All this IS documented, well, sort of, except for the total sudden loss of capacity setting upon data() access after an implicit share due to copy.
Personally I don't find the use case very strange at all, but I will for sure move away from QByteArrays when using them in more complex flows than rather closed scope data manipulations. All else becomes too complex and hard to track. In that case I'd rather be better off with regular C arrays allocated manually on the heap.I am not mad over this, I think QByteArray has its advantages and place, but it just made me a little disappointed though because I had hopes on arrays being used as buffer containers this way and in more advanced ways.
-
one doesn't immediately realize that taking the pointer from the original array after it has been shared, will immediately cause a detach()
Only calling non-const methods on the container causes detach. If you only want to read you can use
constData()
,constBegin()
etc. and that would not detach.I feel there is little or no use at all for the capacity/reserve support
Here's a popular case:
QByteArray b; b.reserve(100); b += "bla bla bla bla"; b += someData; b += someOtherData;
at when it comes to the ability to use the data pointer for buffer management
Keeping around pointers to implicitly shared data is a bad idea in general, not only with QByteArray. Same goes for QString, QVector etc.
-
@Larswad said in SLOT/SIGNAL safety with QByteArray &references:
So, even if this is understandable the behavior is somewhat subtle.
I disagree, it's quite clearly stated in fact:
The container classes are implicitly shared, they are reentrant, and they are optimized for speed, low memory consumption, and minimal inline code expansion, resulting in smaller executables.
http://doc.qt.io/qt-5/containers.html
http://doc.qt.io/qt-5/implicit-sharing.htmlNow my use case is to keep a certain buffer size of the array to be filled as I hand out the QByteArray's pointer to a libusb outstanding request. As the request returns (with the ACTUAL size) i modify the size of the QByteArray to the real one (this is where the resize function was not trustful, it would resize but it would reallocate sometime and also change the capacity of the array.
You could either use your own allocated buffer that you later attach/copy into a byte array or simply
resize()
the byte array beforehand and then shrink it back if necessary. Also the docs state that behavior:To obtain a pointer to the actual character data, call data() or constData(). These functions return a pointer to the beginning of the data. The pointer is guaranteed to remain valid until a non-const function is called on the QByteArray.
-
Also, are you sure that the signal and slot paradigm is the right in this case ?
Using a Q_INVOKABLE method with QMetaObject::invokeMethod might make your intent clearer.
-
I am not going to argue myself to death over this, it is a case where I was led into false expectations even though I knew QByteArray was implicitly shared. It is the implications of being implicitly shared that can cause surprising behaviors, e.g. indirectly breaking const promise as a result of sharing.
@kshegunov That statement tells only the implicit sharing is used on the containers, which of course is good and more or less obvious even without that statement. But it leaves a lot of interpretation in every use case and detail what the implications of that will be. It's a narrow and dangerous walk if not knowing the complete ruleset here. The devil is absolutely in the details here.
@Chris-Kawa Yes, that is a common case in the aim of improving the performance of it, as a result of it being a convenience container that has no idea about what to preallocate. Normally not a problem when NOT using a container class because then you have to take care of both the size and performance issue anyway, albeit with maybe a little more work and careful consideration. So I would still have expected some other more functional use of the reserve feature other than improving the performance of the container itself (which often in the end still becomes a very rough estimate). Heap thrashing and deep copies are still very hard to predict when things start to work "under the hood" like they do in complex container classes. One single code statement can explode in complexity and expand to serious cycle consumption.
Keeping around pointers to implicitly shared data is a bad idea in general, not only with QByteArray. Same goes for QString, QVector etc.
Yes, obviously and sadly it is and I will refrain from it in future. In this case the implicit sharing killed my whole purpose,.I would never use the const version to get the data for modification, even if I knew the would not cause a detach() and that it keeps the data pointer and reservations stable, because the whole semantic of those is to use the data only for reading and not to be handed out to some other uses.
I have walked through larger part of the QByteArray implementation these days, so I have a pretty good picture now what it does internally and I regard it now as something that you start putting into use once you already HAVE the data produced from other API levels, after that the data can be contained at some point using QByteArray and THEN conveniently get shifted around inside code strictly built on Qt framework. All other uses as a "buffer-container-manager-exposer-allocator" outside that becomes a gamble of life and death.@SGaist Thanks. But you mean that Q_INVOKABLE declaration indicate that a metacall is needed here? I have mostly used that in the cases of exposing to declarative.
-
No, not needed. It indicates that you can use it through a meta call in addition to use it like a normal function.
-
Things get even more useless here.
How in the world would anyone take ownership of the data from a QByteArray (e.g. calling something with the intent like takeRawAndDetach()) like saying to the array "The data is now MINE, I have ownership now, if you are shared I want a copy of you, otherwise return your data and drop off". Because as I interpret the detech() implementation, a sequence of data(), detach() would only deallocate that data that was returned to data() leaving me a dangling pointer.
Wow, it can't be that user-unfriendly and performance ugly that I would have to do something like:auto myData(new char[array.size()]);
memcpy(myData, array.constData(), array.size());Just to get ownership of the contained data.
I would have preferred if I could take over the data through a single call that would take care of actually making and returning a raw copy if the data is shared? There is not such method or feature in QByteArray?Meaning (if that is the case): You can't create a method in your code that takes arguments as a QByteArray reference and the let that function take over the byte array data without actually in there make a NEW buffer on the heap and deep copy of the whole QByteArray using regular new and memcopy?
If this actually IS the case I'm starting to think the shared containers suck, feature wise and performance wise. The cost is too big. -
I hope you don't mind that I forked this discussion as a separate topic. It seemed appropriate.
@Larswad said in QByteArray data sharing and ownership:
How in the world would anyone take ownership of the data from a QByteArray (e.g. calling something with the intent like takeRawAndDetach()) like saying to the array "The data is now MINE, I have ownership now, if you are shared I want a copy of you, otherwise return your data and drop off".
You can't do that directly, as you correctly observed.
Wow, it can't be that user-unfriendly and performance ugly that I would have to do something like:
auto myData(new char[array.size()]);
memcpy(myData, array.constData(), array.size());Just to get ownership of the contained data.
That's the correct way currently. I could imagine a method to be added that does that however, so my advice is to post it as a suggestion on the bugtracker. I'd even be willing to implement it when time permits.
I would have preferred if I could take over the data through a single call that would take care of actually making and returning a raw copy if the data is shared? There is not such method or feature in QByteArray?
Not at this time as far as I'm aware.
Meaning (if that is the case): You can't create a method in your code that takes arguments as a QByteArray reference and the let that function take over the byte array data without actually in there make a NEW buffer on the heap and deep copy of the whole QByteArray using regular new and memcopy?
Usually one goes the other way around - allocating your own buffer and attaching a
QByteArray
"interface" to it (e.g. throughQByteArray::fromRawData
) wherever it's needed.If this actually IS the case I'm starting to think the shared containers suck, feature wise and performance wise. The cost is too big.
You are entitled to your opinion, I personally like them and use shared data classes extensively, but in any case they're not going away any time soon. The whole signal-slot mechanism would fall apart if implicit sharing were to be removed.
-
@kshegunov Thanks for the answer!
Please disregard my last sentence, it was written in frustration on the situation at hand and was clearly uncalled for. -
@Larswad said in QByteArray data sharing and ownership:
Please disregard my last sentence, it was written in frustration on the situation at hand and was clearly uncalled for.
Happens to all of us, so I (usually) try to not nitpick about it. I was serious about posting this as a suggestion in the bugtracker though, you raised a good point. If you decide to proceed this way, please post also the link to the tracker issue here.
-
@kshegunov Yes, I think I just might file that feature request on the tracker. I came to think about quite a few years ago when I used COM's CComSafeArray and CComBSTR that had the detach() method as a way to leave their guts so to say, they on the other hand didn't support sharing which of course makes things simpler.