QVector one-line deep copy?
-
I have a (class member)
QVector
holdingstruct
elements with simpl-ish-type members:struct MyStruct { int memberA; enum MemberB; QString memberC; };
I need to retain a saved (class member) copy of its state/content, as that will change. Not only will elements be added/deleted, but the content of a given element's
struct
may be altered.QVector<MyStruct> current; QVector<MyStruct> saved(current);
won't work: although it gets deep copied on adding/removing elements to
current
, it doesn't oncurrent[i].structMember = ...
: the element insaved[i]
now has the current, altered value. (At least, that was my finding, unless you think I am going mad....)So I go:
saved.clear(); for (const MyStruct &elm : current) saved.append(elm);
and this is good for deep copy.
But, being a pedant, I wonder if there is a "faster" way in which I can squeeze out the best internal copying perfomance, and reduce these 3 lines to 1?! Is there any Qt/C++ "copy" statement I can use here to produce a deep copy?
-
QVector<MyStruct> current; QVector<MyStruct> saved(current); saved.detach();
No idea if it will work, but at least in principle it should.
-
@JonB said in QVector one-line deep copy?:
won't work: although it gets deep copied on adding/removing elements to current, it doesn't on current[i].structMember = ...: the element in saved[i] now has the current, altered value. (At least, that was my finding, unless you think I am going mad....)
No, your finding is wrong. Please show us your code - a simple main.cpp with 10 lines of what you're doing should be fine.
Qt containers are copy-on-write, see https://doc.qt.io/qt-5/implicit-sharing.html
But, being a pedant, I wonder if there is a "faster" way in which I can squeeze out the best internal copying perfomance
The vector copy does basically the same as you wrote. See https://code.woboq.org/qt5/qtbase/src/corelib/tools/qvector.h.html#_ZN7QVector7reallocEi6QFlagsIN10QArrayData16AllocationOptionEE
int main(int argc, char **argv) { QVector<int> myVec = {1, 2, 3 ,4}; QVector<int> copy(myVec); copy[3] = 1; qDebug() << myVec; qDebug() << copy; }
output as expected:
QVector(1, 2, 3, 4)
QVector(1, 2, 3, 1)Do you maybe store your struct as a pointer in the vector?
-
QVector<MyStruct> current; QVector<MyStruct> saved(current); saved.detach();
No idea if it will work, but at least in principle it should.
-
@J-Hilk
Oohh. I like the look of this, but don't see how I can make it work for my case...?Your
const QVector<MyStruct> saved(current)
only works at construction time. That is not good enough for my situation. The class instance holding these two vector members persists. From time to time I need to re-copycurrent
tosaved
during its lifetime, so I need a method statements for that. And attempting:saved = current;
gives me a
error: no viable overloaded '=' error: passing ‘const QVector<MyStruct>’ as ‘this’ argument discards qualifiers [-fpermissive]
?
@sierdzio
I didn't try yours for same reason. Do you wish to re-enter the fray with a new proposal in light of the foregoing? -
@Christian-Ehrlicher
Sorry, I have only just seen your reply. I did wonder about my finding.OK, I need to repro, when I have a second. Just to save me: note that I wish to go
myVec[3].member = value
(yes, writing into originalstruct
, not treating it as an immutable and re-assigning the wholestruct
), will that make any difference, so that the vector does not know to deep-copy on this statement?In a less-likely possibility: the mutual-change I happened to notice was on the
QString
member, I don't suppose there should be any issue because of that? -
@JonB said in QVector one-line deep copy?:
will that make any difference, so that the vector does not know to deep-copy on this statement?
It should not since operator[] is non-const and forces a detach()
/edit:
struct s { int one = 1; int two = 2; }; int main(int argc, char **argv) { QVector<s> myVec = {{1, 2}, {3, 4}}; QVector<s> copy(myVec); copy[1].one = 5; qDebug() << myVec[1].one; qDebug() << copy[1].one;; }
-->
3
5 -
It should not since operator[] is non-const and forces a detach()
Ahhhh. What if I am not actually using a
[]
? :) My changes tocurrent
will happen via a reference or a pointer, not an index, like:for (MyStruct &cur : current) cur.member = newValue;
And I want that to change an element's struct-member-value in
current
without affecting the "copied" one insaved
. Is that pushing me down a rabbit-hole here? :) -
@JonB said in QVector one-line deep copy?:
for (MyStruct &cur : current)
This will also force a detach.
/edit:
int main(int argc, char **argv) { QVector<s> myVec = {{1, 2}, {3 ,4}}; QVector<s> copy(myVec); for (auto &v : myVec) v.one = 42; qDebug() << myVec[0].one << myVec[1].one; qDebug() << copy[0].one << copy[1].one;; }
-->
42 42
1 3 -
@Christian-Ehrlicher
Sigh :) OK, I'll have to produce a repro... Because code works withsaved.append()
but not withsaved = current
.Hang on, one more. Same to you as to @J-Hilk previously. I can't use
QVector<int> copy(myVec);
. Copying is not done during constructor.current
&saved
are persistent member variables. It is called explicitly at various points. So it has to be something likesaved = current
. Does that make a difference to what you are telling me should work? -
@JonB said in QVector one-line deep copy?:
Does that make a difference to what you are telling me should work?
No, it does exactly the same, our version calls the copy operator, yours the operator=() but they do the same (if not it would be wrong by design ;) ). Just check it out with my little repro - it does not change anything.
-
@Christian-Ehrlicher
Hmm, now just wait a cotton-picking moment! @sierdzio appears to sort me out, after all!QVector<MyStruct> current, saved; // on its own, this does *not* work; // when I alter something in `current` it gets changed in `saved` saved = current; // as soon as I put this line next, it *does* work, I see changes when I later alter something in `current` saved.detach();
So..... ?
-
@JonB
obviously the const only works on construction 😉that said a small test example:
struct MyStruct { int memberA; QString memberC; MyStruct() : memberA{0}, memberC{QLatin1String()} {} }; #include <QDebug> #include <vector> int main (int argc, char *argv[]) { QVector<MyStruct> current{1}; QVector<MyStruct> saved(current); std::vector<MyStruct> deepCopy; for(auto s : current) deepCopy.push_back(s); current.first().memberA = 10; qDebug() << current.first().memberA << saved.first().memberA << deepCopy.at(0).memberA; }
results in
10 0 0
so it detaches automatically.
It doesn't for you?
-
@Christian-Ehrlicher , or @jsulm [or indeed any of you helpful experts, it's just that this is getting tricky...]
I wonder if you would care to help. Because I think we need your deep understanding of C++ to see if you can understand what is going on!As I said, in my code it definitely does not work without an explicit
detach()
, and (I think) it will be to do with pointers.Just to resume: if I break just prior to the assignment into the
struct
of an element incurrent
I can see that element in bothcurrent
&saved
, and they are indeed showing the same address. After the assignment the address remains the same in both of them, hence my observed behaviour. There is no detachment!If in the debugger I put a watch point on the element in question. I do that via
current[13]
&saved[13]
(debugger won't allow.at(13)
). If I do that, at that instant the addresses change --- i.e. detachment occurs, because of the[]
. (I think it is thecurrent[13]
which changes whilesaved[13]
stays the same, if that's what you would expect.)Like you, I don't think I saw that in a test program. However, my real code has a nasty wrinkle going on. And I'm wondering if it's all @jsulm's fault!
In https://forum.qt.io/topic/120395/return-pointer-to-member-in-const-method/8 he proposed the following code (you'd have to read the whole thread to understand why), which I now use:
const int *pointerToMember() const { return &member; } int *pointerToMember() { const MyClass *_this = this; return const_cast<int*>(_this->pointerToMember()); } // Now compiler knows that you want to call const pointerToMember
In my code for the first overload, I am actually marching through the
QVector current
returning a reference to the found element as a pointer, ornullptr
if not found.The way I get the pointer to the element I need to change (in
current
) is via the second, "writeable" overload there.Now then: I have a suspicion this is the cause? I am getting a writeable element in a (shared)
QVector
via aconst_cast<>
of aconst
method. Does this cheat by any chance disable/break the "detach-on-write" we are relying on to make sure element incurrent
gets changed but not insaved
. I have a feeling.... :) -
At least this could be a reason although I don't see your actual code to say it for sure.
I would have returned &member directly also in the non-const version - then this should not happen for sure. -
@Christian-Ehrlicher
:) OK, here's the lookup code:const Class::MyStruct *Class::find(int arg) const { for (const MyStruct &ms : current) if (ms.arg== arg) return &ms; return nullptr; } Class::MyStruct *Class::find(int arg) { const Class *_this = this; return const_cast<MyStruct *>(_this->find(arg)); } QVector<MyStruct> current, saved; // member variables current.append(...); // this can be called at various times saved = current; // this can be called at various times MyStruct *ms = find(something); // this will be found in current if (ms != nullptr) ms->someMember = newValue; // want to change in current, only
On that last line the element in
current
gets its content changed, but the element insaved
continues to point to the same, now changed, element, with no "detachment".It's difficult at present for me to break this up, as code presently relies elsewhere on these two definitions.
I am suggesting that the
const_cast<>
in the second overload is what "breaks" any detachment, is that possible? I'm thinking that it leads to the code/Qt thinking that no detachment is necessary for an element, because it thinks that must have already happened given the non-const
return result, something like that?If so, how would you like me to rewrite it? Bear in mind the whole of the referenced thread, in which I steadfastly refuse to return an index from my lookup
find()
, as that is so inefficient compared to a pointer :)Finally, assuming it is @jsulm's solution to that thread which causes the problem, can I sue him? :-:
-
Correct - this code kills the copy on write behavior since current is not detached in find() because it's a const operation there.
-
@Christian-Ehrlicher
And? :) That solves the mystery here (I can use @sierdzio's explicitdetach()
for now), but leaves me with the other thread still not correctly answered, now :( You didn't say if I could sue @jsulm there....Thanks to you, and all, for answering. I didn't even recall when I raised this "deep copy" question that it was going via/could be related to the
const_cast<>
method.