PDA

View Full Version : QDomNode delete children



trallallero
9th December 2013, 09:38
I have a memory problem using QDomNode as the XML file is too big so I'm trying to create a row in a QDomDocument, write it to a file and delete it. I'm creating an Excel file so every row has a lot of cells. If I create e cell and then delete it, every thing is ok and the memory problem is solved but if I create a row, fill it with many cells, write all to a file and then delete the row, the row and all its cells are not deleted.

I have debugged this line:


oldRow.parentNode().removeChild(oldRow);

and I've seen that in the QDomNodePrivate* QDomNodePrivate::removeChild(QDomNodePrivate* oldChild) method, the child is actually removed from the list, but it's not deleted.
How can delete it ?

I've also tried oldRow.clear() but nothing happens.

zaphod.b
9th December 2013, 14:27
From the QDomDocument doc:

The parsed XML is represented internally by a tree of objects that can be accessed using the various QDom classes. All QDom classes only reference objects in the internal tree. The internal objects in the DOM tree will get deleted once the last QDom object referencing them and the QDomDocument itself are deleted.

If your document is too large for DOM, go with SAX.

trallallero
9th December 2013, 14:42
I know but I just need to apply a patch as switching to SAX would take too much time now.

zaphod.b
9th December 2013, 14:58
I suppose "cells" are child nodes of "row" in your DOM? removeChild() is not recursive, you'd have to to this yourself, or else "cells" remain referenced.

trallallero
10th December 2013, 08:40
Good point thanks, but I tried this:



if (node.isElement())
{
QDomElement e = node.toElement();
QDomNodeList nodeList = e.childNodes();
for( int i = 0; i < nodeList.count(); ++i )
if( nodeList.at(i).isElement() )
nodeList.at(i).parentNode().removeChild(nodeList.a t(i));
}
node.parentNode().removeChild(node);


and it doesn't work.
Node is the row and nodeList will contain all the "cells".
I've debugged it and removeChild is actually called.

If I delete each cell immediately after having created it (but it's something I cannot do), the used RAM goes up to 200MB, if I, instead, delete each row and all its children (the cells), the used RAM goes up to 900MB.

Something is removed because if I don't call any removeChild method, the used RAM goes up to 1.8GB and the program crashes.

zaphod.b
10th December 2013, 12:19
Are your rows and cells represented by element nodes only?

If so, why do you still ask for isElement()?
If not, why do you only remove the element nodes?
What about attributes and text?

trallallero
10th December 2013, 12:26
I've removed the isElement and added a clear() but it doesn't help.



QDomNodeList nodeList = node.childNodes();
for( int i = 0; i < nodeList.count(); ++i )
{
nodeList.at(i).clear();
nodeList.at(i).parentNode().removeChild(nodeList.a t(i));
}
node.clear();
node.parentNode().removeChild(node);


clear() should remove all the attributes, no ?

Anyway, we decided to switch to QXmlStreamWriter/QXmlStreamReader but it would be interesting to understand why this happens. I've also asked Qt/Digia customer support but still no answer.

zaphod.b
10th December 2013, 12:43
clear() won't delete the node itself, just make null node of it.

The code snippet you provided will now remove immediate child nodes, "cell elements" that is. I'd guess in general these cell elements will have child nodes themselves, e.g. text. As I wrote before, you'd have to make sure you traverse all descendant nodes, no matter what sublevel or type.

trallallero
10th December 2013, 20:46
clear() won't delete the node itself, just make null node of it.

The code snippet you provided will now remove immediate child nodes, "cell elements" that is. I'd guess in general these cell elements will have child nodes themselves, e.g. text. As I wrote before, you'd have to make sure you traverse all descendant nodes, no matter what sublevel or type.

It's exactly the other way round as explained by the Qt support via email, received today.


When you call removeChild() then all it is going to do is remove it from the current tree, you still get a reference to the QDomNode that you have removed which is why it still exists in memory because it hasn't been actually deleted. If you want to ensure that the contents of the node are also deleted then you need to call clear() on that node. This will cause it to be deleted and point to a null node and therefore the memory should be freed up for you. See:

http://qt-project.org/doc/qt-4.8/qdomnode.html#clear

But I will answer them tomorrow because this is not true.
If I only call clear(), nothing is deleted. The call to clear() seems to have no consequences.
Or, probably, for clear() it's as you said about removeChild(), call it recursively.

zaphod.b
11th December 2013, 01:13
Sorry for the confusion. I remembered incorrectly from the distant past.

Meanwhile I had a look at the sources (5.1 but shouldn't have changed).
As you'll have seen from your debugging, clear() simply calls the impl dtor :


void QDomNode::clear()
{
if (impl && !impl->ref.deref())
delete impl;
impl = 0;
}

The dtor indeed iterates the children, thus traversing the tree :


QDomNodePrivate::~QDomNodePrivate()
{
QDomNodePrivate* p = first;
QDomNodePrivate* n;

while (p) {
n = p->next;
if (!p->ref.deref())
delete p;
else
p->setNoParent();
p = n;
}
first = 0;
last = 0;
}

I'm not sure what exactly deref() does, but if I interpret the asm correctly, it simply decrements the ref count and returns false if there are no references left, true otherwise. In the latter case deletion traversal terminates, and the node's subtree will dangle in the sense that it remains owned by the doc but is child of this node no more.

My conclusion is, if not all of your "row" is deleted, there must be reference(s) to its descendant(s). This is in accordance with the doc cited in post #2 (http://www.qtcentre.org/threads/57184-QDomNode-delete-children?p=255437#post255437).

If you provided relevant parts of your code there might be a chance to locate the culprit.