QGraphicsAnchorLayout: remove a misguided use of QLinkedList

QSimplexConstraints are held in QList everywhere, yet one
single function, getGraphParts(), used a temporary
QLinkedList. It did so because the function repeatedly
walks the list, erasing elements from it until no more
elements have been removed.

Thus, in O-terms, QLinkedList is the correct choice here.
Sadly, O-notation completely ignores the per-element cost,
and this is where QLinkedList suffers. By the time a QList
has shifted all of its elements left once, the QLinkedList
probably has just finished allocating its first node.

So, use a QList instead.

That, however, turns the it = erase(it) loop quadratic, so
re-formulate the processing part as a lambda and use
std::remove_if. Don't even erase until we know how many
items to erase.

As a benefit, we save the final conversion of the remaining
items back to a QList, and we can use QList::op+ to build
the initial list, reducing the number of allocations
performed by that container to one.

Also saves ~770b in text size on optimized GCC 5.3 Linux
AMD64 builds.

Change-Id: Iecf9e7961dd2b6b20039b9b0d472e32b3fae6994
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
bb10
Marc Mutz 2016-01-14 16:35:40 +01:00
parent 7a17340636
commit 57671bebbc
1 changed files with 13 additions and 24 deletions

View File

@ -2513,13 +2513,7 @@ QGraphicsAnchorLayoutPrivate::getGraphParts(Orientation orientation)
edgeL1 = graph[orientation].edgeData(layoutFirstVertex[orientation], layoutLastVertex[orientation]);
}
QLinkedList<QSimplexConstraint *> remainingConstraints;
for (int i = 0; i < constraints[orientation].count(); ++i) {
remainingConstraints += constraints[orientation].at(i);
}
for (int i = 0; i < itemCenterConstraints[orientation].count(); ++i) {
remainingConstraints += itemCenterConstraints[orientation].at(i);
}
QList<QSimplexConstraint *> remainingConstraints = constraints[orientation] + itemCenterConstraints[orientation];
QList<QSimplexConstraint *> trunkConstraints;
QSet<QSimplexVariable *> trunkVariables;
@ -2529,12 +2523,11 @@ QGraphicsAnchorLayoutPrivate::getGraphParts(Orientation orientation)
trunkVariables += edgeL2;
bool dirty;
auto end = remainingConstraints.end();
do {
dirty = false;
QLinkedList<QSimplexConstraint *>::iterator it = remainingConstraints.begin();
while (it != remainingConstraints.end()) {
QSimplexConstraint *c = *it;
auto isMatch = [&trunkConstraints, &trunkVariables](QSimplexConstraint *c) -> bool {
bool match = false;
// Check if this constraint have some overlap with current
@ -2552,8 +2545,7 @@ QGraphicsAnchorLayoutPrivate::getGraphParts(Orientation orientation)
trunkConstraints += c;
for (auto jt = c->variables.cbegin(), end = c->variables.cend(); jt != end; ++jt)
trunkVariables.insert(jt.key());
it = remainingConstraints.erase(it);
dirty = true;
return true;
} else {
// Note that we don't erase the constraint if it's not
// a match, since in a next iteration of a do-while we
@ -2563,24 +2555,21 @@ QGraphicsAnchorLayoutPrivate::getGraphParts(Orientation orientation)
// remainingConstraints[1] and it shares with
// remainingConstraints[0], we need a second iteration
// of the do-while loop to match both.
++it;
return false;
}
}
};
const auto newEnd = std::remove_if(remainingConstraints.begin(), end, isMatch);
dirty = newEnd != end;
end = newEnd;
} while (dirty);
remainingConstraints.erase(end, remainingConstraints.end());
QList< QList<QSimplexConstraint *> > result;
result += trunkConstraints;
if (!remainingConstraints.isEmpty()) {
QList<QSimplexConstraint *> nonTrunkConstraints;
nonTrunkConstraints.reserve(remainingConstraints.size());
QLinkedList<QSimplexConstraint *>::iterator it = remainingConstraints.begin();
while (it != remainingConstraints.end()) {
nonTrunkConstraints += *it;
++it;
}
result += nonTrunkConstraints;
}
if (!remainingConstraints.isEmpty())
result += remainingConstraints; // non-trunk constraints
return result;
}