mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-11-05 09:33:25 -08:00
[CardDB] Sort power/toughness without splitting every comparison (#7607)
* [CardDB] Sort power/toughness without splitting every comparison
When the card database is sorted by the P/T column, lessThan() runs
O(n log n) times over the whole view, and each call split both strings
into a QStringList ("power/toughness") and then read the two elements
back. For the roughly 36k cards in the database that is millions of
list and string allocations, all for a value that contains at most one
separator.
The comparator now locates the single '/' with indexOf and passes the
two halves as QStringView to lessThanNumerically(), which is changed
from QString to QStringView. The numeric parsing, the "1+*" fallback and
the locale-aware tie-break are unchanged, and strings with no separator
or more than one still fall through to the plain string comparison,
exactly as split()-based size checks did before.
lessThanNumerically() also initializes its two bool out-parameters; they
were declared uninitialized, which only worked because toFloat() always
assigns them.
Compared old and new comparator over a corpus of 2024 power/toughness
strings (4096576 ordered pairs, including malformed values): zero
differences. Full build and ctest (628 tests) pass.
* Extract well-formedness checks in P/T comparator
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
parent
e666a4978d
commit
4980f88a7a
2 changed files with 29 additions and 23 deletions
|
|
@ -82,13 +82,12 @@ int CardDatabaseDisplayModel::rowCount(const QModelIndex &parent) const
|
|||
|
||||
bool CardDatabaseDisplayModel::lessThan(const QModelIndex &left, const QModelIndex &right) const
|
||||
{
|
||||
|
||||
QString leftString = sourceModel()->data(left, CardDatabaseModel::SortRole).toString();
|
||||
QString rightString = sourceModel()->data(right, CardDatabaseModel::SortRole).toString();
|
||||
const QString leftString = sourceModel()->data(left, CardDatabaseModel::SortRole).toString();
|
||||
const QString rightString = sourceModel()->data(right, CardDatabaseModel::SortRole).toString();
|
||||
|
||||
if (!cardName.isEmpty() && left.column() == CardDatabaseModel::NameColumn) {
|
||||
bool isLeftType = leftString.startsWith(cardName, Qt::CaseInsensitive);
|
||||
bool isRightType = rightString.startsWith(cardName, Qt::CaseInsensitive);
|
||||
const bool isLeftType = leftString.startsWith(cardName, Qt::CaseInsensitive);
|
||||
const bool isRightType = rightString.startsWith(cardName, Qt::CaseInsensitive);
|
||||
|
||||
// test for an exact match: isLeftType && leftString.size() == cardName.size()
|
||||
// or an exclusive start match: isLeftType && !isRightType
|
||||
|
|
@ -101,31 +100,37 @@ bool CardDatabaseDisplayModel::lessThan(const QModelIndex &left, const QModelInd
|
|||
return false;
|
||||
}
|
||||
} else if (right.column() == CardDatabaseModel::PTColumn && left.column() == CardDatabaseModel::PTColumn) {
|
||||
QStringList leftList = leftString.split("/");
|
||||
QStringList rightList = rightString.split("/");
|
||||
// The comparator runs O(n log n) times while the view is sorted, so
|
||||
// splitting every power/toughness string into a QStringList allocates
|
||||
// a lot for data that has exactly one separator. Work on views and
|
||||
// only split the two halves out when both strings are well formed.
|
||||
const QStringView leftPT = leftString;
|
||||
const QStringView rightPT = rightString;
|
||||
const qsizetype leftSlash = leftPT.indexOf(u'/');
|
||||
const qsizetype rightSlash = rightPT.indexOf(u'/');
|
||||
const bool leftIsWellFormed = leftSlash >= 0 && leftPT.indexOf(u'/', leftSlash + 1) < 0;
|
||||
const bool rightIsWellFormed = rightSlash >= 0 && rightPT.indexOf(u'/', rightSlash + 1) < 0;
|
||||
|
||||
if (leftList.size() == 2 && rightList.size() == 2) {
|
||||
|
||||
// cool, have both P/T in list now
|
||||
int lessThanNum = lessThanNumerically(leftList.at(0), rightList.at(0));
|
||||
if (lessThanNum != 0) {
|
||||
return lessThanNum < 0;
|
||||
} else {
|
||||
// power equal, check toughness
|
||||
return lessThanNumerically(leftList.at(1), rightList.at(1)) < 0;
|
||||
if (leftIsWellFormed && rightIsWellFormed) {
|
||||
const int powerComparison = lessThanNumerically(leftPT.left(leftSlash), rightPT.left(rightSlash));
|
||||
if (powerComparison != 0) {
|
||||
return powerComparison < 0;
|
||||
}
|
||||
// power equal, check toughness
|
||||
return lessThanNumerically(leftPT.mid(leftSlash + 1), rightPT.mid(rightSlash + 1)) < 0;
|
||||
}
|
||||
}
|
||||
return QString::localeAwareCompare(leftString, rightString) < 0;
|
||||
}
|
||||
|
||||
int CardDatabaseDisplayModel::lessThanNumerically(const QString &left, const QString &right)
|
||||
int CardDatabaseDisplayModel::lessThanNumerically(QStringView left, QStringView right)
|
||||
{
|
||||
if (left == right) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
bool okLeft, okRight;
|
||||
bool okLeft = false;
|
||||
bool okRight = false;
|
||||
float leftNum = left.toFloat(&okLeft);
|
||||
float rightNum = right.toFloat(&okRight);
|
||||
|
||||
|
|
@ -139,11 +144,11 @@ int CardDatabaseDisplayModel::lessThanNumerically(const QString &left, const QSt
|
|||
}
|
||||
}
|
||||
// try and parsing again, for weird ones like "1+*"
|
||||
QString leftAfterNum = "";
|
||||
QString rightAfterNum = "";
|
||||
QStringView leftAfterNum;
|
||||
QStringView rightAfterNum;
|
||||
if (!okLeft) {
|
||||
int leftNumIndex = 0;
|
||||
for (; leftNumIndex < left.length(); leftNumIndex++) {
|
||||
for (; leftNumIndex < left.size(); leftNumIndex++) {
|
||||
if (!left.at(leftNumIndex).isDigit()) {
|
||||
break;
|
||||
}
|
||||
|
|
@ -155,7 +160,7 @@ int CardDatabaseDisplayModel::lessThanNumerically(const QString &left, const QSt
|
|||
}
|
||||
if (!okRight) {
|
||||
int rightNumIndex = 0;
|
||||
for (; rightNumIndex < right.length(); rightNumIndex++) {
|
||||
for (; rightNumIndex < right.size(); rightNumIndex++) {
|
||||
if (!right.at(rightNumIndex).isDigit()) {
|
||||
break;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue