Verified Commit b0529aff authored by Daniel Vrátil's avatar Daniel Vrátil 🤖
Browse files

SearchJob: don't assemble OR term recursively

Summary:
The code was ridiculously inefficient for large OR sets. Trying
to assemble an OR term with 28'000 subterms took minutes and the
process eventually ran out of memory. Instead just use a for loop
and a bit of counting to balance the parentheses, the string
is assembled within milliseconds.

Test Plan:
Added a test to make sure the serialized string is the same as before

A benchmark for 10'000 subterms took 4 seconds. Benchmark for 28'000
subterms got killed by the OOM-killer. After the patch the benchmark
for 28'000 subterms took 2.2 ms.

Reviewers: #kde_pim, vkrause

Reviewed By: #kde_pim, vkrause

Subscribers: kde-pim

Tags: #kde_pim

Differential Revision: https://phabricator.kde.org/D28946
parent 68facf36
......@@ -164,6 +164,19 @@ private Q_SLOTS:
<< "S: A000001 OK search done";
QTest::newRow("uidbased NOT NEW search") << scenario << true << 6 << KIMAP::Term(KIMAP::Term::New).setNegated(true);
}
{
QList<QByteArray> scenario;
scenario << FakeServer::preauth()
<< "C: A000001 UID SEARCH OR HEADER Message-Id \"<1234567@mail.box>\" (OR HEADER Message-Id \"<7654321@mail.box>\" (OR HEADER Message-Id \"<abcdefg@mail.box>\" HEADER Message-Id \"<gfedcba@mail.box>\"))"
<< "S: * SEARCH 1 2 3 4"
<< "S: A000001 OK search done";
KIMAP::Term term{KIMAP::Term::Or, {KIMAP::Term{QStringLiteral("Message-Id"), QStringLiteral("<1234567@mail.box>")},
KIMAP::Term{QStringLiteral("Message-Id"), QStringLiteral("<7654321@mail.box>")},
KIMAP::Term{QStringLiteral("Message-Id"), QStringLiteral("<abcdefg@mail.box>")},
KIMAP::Term{QStringLiteral("Message-Id"), QStringLiteral("<gfedcba@mail.box>")}}};
QTest::newRow("OR with multiple subterms") << scenario << true << 4 << term;
}
}
void testSearchTerm()
......
......@@ -69,25 +69,24 @@ Term::Term(Term::Relation relation, const QVector<Term> &subterms)
: d(new Term::Private)
{
if (subterms.size() >= 2) {
d->command += "(";
if (relation == KIMAP::Term::Or) {
d->command += "OR ";
d->command += subterms.at(0).serialize() + ' ';
if (subterms.size() >= 3) {
Term t(relation, subterms.mid(1));
d->command += t.serialize();
} else if (subterms.size() == 2) {
d->command += subterms.at(1).serialize();
for (int i = 0; i < subterms.size() - 1; ++i) {
d->command += "(OR " + subterms[i].serialize() + " ";
}
d->command += subterms.back().serialize();
for (int i = 0; i < subterms.size() - 1; ++i) {
d->command += ")";
}
} else {
d->command += "(";
for (const Term &t : subterms) {
d->command += t.serialize() + ' ';
}
if (!subterms.isEmpty()) {
d->command.chop(1);
}
d->command += ")";
}
d->command += ")";
} else if (subterms.size() == 1) {
d->command += subterms.first().serialize();
} else {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment