Le 04/10/2011 16:33, Tommaso Cucinotta a écrit :
I have some doubts about how UndoElement instances are handled.
a) they're pushed into the stack *by value*
std::deque<UndoElement> c_;
so, more instances of UndoElement are created and copied wastefully:
void push(UndoElement const & v) {
c_.push_front(v);
I am not sure it is a big problem. Is the waste even measurable?
furthermore, if the internal representation of dequeue<> uses an
array, then
we waste more memory for the unused entries than we would, if it were
a dequeue<UndoElement*>.
I assume an UndoElement is not much more that 100 bytes. This makes a
total of 10kbytes. This is nothing to care about.
b) the UndoElement class contains pointers to MathData and ParagraphList
which are created by new
if (cell.inMathed()) {
[...]
undo.array = new MathData(ar.buffer(), ar.begin(), ar.end());
} else {
[...]
undo.pars = new ParagraphList(first, last);
}
stack.push(undo);
but apparently never deleted:
They are deleted when they are used (in textUndoOrRedo) but it seems
indeed that they are not deleted when an element is dropped out of the
UndoStack. I would suggest to use auto_ptr (or shared_ptr?) for these
elements, so that they are correctly handled. The current code is not
very clear to me, since in some cases the pointers are merely copied and
in other cases the object is cloned.
I think there is something here, triggered by the massive buffer
modifications done by advanced search (this code is really weird to me,
BTW).
JMarc
