Skip to content

Commit

Permalink
QKeySequence: replace an inefficient QList with QVector
Browse files Browse the repository at this point in the history
QShortcutEntry is larger than a void*, so holding it in QLists
is needlessly inefficient. Worse, the code could come to depend
on the fragile property of (inefficient) QLists that references
to elements therein never are invalidated.

Fix by marking it movable, and holding it in a QVector instead.

Change-Id: I4ab3399a8036827631b7fbdfdc60b4206305e1c9
Reviewed-by: Lars Knoll <[email protected]>
Reviewed-by: Thiago Macieira <[email protected]>
  • Loading branch information
marc-kdab committed Feb 8, 2016
1 parent 8a54950 commit b30edc5
Showing 1 changed file with 7 additions and 7 deletions.
14 changes: 7 additions & 7 deletions src/gui/kernel/qshortcutmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ struct QShortcutEntry
QObject *owner;
QShortcutMap::ContextMatcher contextMatcher;
};
Q_DECLARE_TYPEINFO(QShortcutEntry, Q_MOVABLE_TYPE);

#ifdef Dump_QShortcutMap
/*! \internal
Expand Down Expand Up @@ -123,7 +124,7 @@ class QShortcutMapPrivate
}
QShortcutMap *q_ptr; // Private's parent

QList<QShortcutEntry> sequences; // All sequences!
QVector<QShortcutEntry> sequences; // All sequences!

int currentId; // Global shortcut ID number
int ambigCount; // Index of last enabled ambiguous dispatch
Expand Down Expand Up @@ -162,7 +163,7 @@ int QShortcutMap::addShortcut(QObject *owner, const QKeySequence &key, Qt::Short
Q_D(QShortcutMap);

QShortcutEntry newEntry(owner, key, context, --(d->currentId), true, matcher);
QList<QShortcutEntry>::iterator it = std::upper_bound(d->sequences.begin(), d->sequences.end(), newEntry);
const auto it = std::upper_bound(d->sequences.begin(), d->sequences.end(), newEntry);
d->sequences.insert(it, newEntry); // Insert sorted
#if defined(DEBUG_QSHORTCUTMAP)
qDebug().nospace()
Expand Down Expand Up @@ -408,8 +409,8 @@ bool QShortcutMap::hasShortcutForKeySequence(const QKeySequence &seq) const
{
Q_D(const QShortcutMap);
QShortcutEntry entry(seq); // needed for searching
QList<QShortcutEntry>::ConstIterator itEnd = d->sequences.constEnd();
QList<QShortcutEntry>::ConstIterator it = std::lower_bound(d->sequences.constBegin(), itEnd, entry);
const auto itEnd = d->sequences.cend();
auto it = std::lower_bound(d->sequences.cbegin(), itEnd, entry);

for (;it != itEnd; ++it) {
if (matches(entry.keyseq, (*it).keyseq) == QKeySequence::ExactMatch && (*it).correctContext() && (*it).enabled) {
Expand Down Expand Up @@ -455,9 +456,8 @@ QKeySequence::SequenceMatch QShortcutMap::find(QKeyEvent *e, int ignoredModifier
int result = QKeySequence::NoMatch;
for (int i = d->newEntries.count()-1; i >= 0 ; --i) {
QShortcutEntry entry(d->newEntries.at(i)); // needed for searching
QList<QShortcutEntry>::ConstIterator itEnd = d->sequences.constEnd();
QList<QShortcutEntry>::ConstIterator it =
std::lower_bound(d->sequences.constBegin(), itEnd, entry);
const auto itEnd = d->sequences.constEnd();
auto it = std::lower_bound(d->sequences.constBegin(), itEnd, entry);

int oneKSResult = QKeySequence::NoMatch;
int tempRes = QKeySequence::NoMatch;
Expand Down

0 comments on commit b30edc5

Please sign in to comment.