From a24bf3db83d81d7d7677a1f0f750f208d32998a8 Mon Sep 17 00:00:00 2001 From: Christian Mollekopf Date: Sat, 28 Jul 2018 22:29:35 +0200 Subject: Avoid unnecessary Identifier conversions in performance ciritical code. This fixes the performance regressions to a state where we are roughly at the same performance as pre Identifier (but not any better either). --- common/datastorequery.cpp | 14 +++++++------- common/storage/entitystore.cpp | 12 ++++++------ common/storage/entitystore.h | 4 ++-- common/storage/key.cpp | 6 ++++++ common/storage/key.h | 2 ++ common/typeindex.cpp | 27 ++++++++++++++++----------- common/typeindex.h | 2 +- 7 files changed, 40 insertions(+), 27 deletions(-) diff --git a/common/datastorequery.cpp b/common/datastorequery.cpp index 52243da..43b4660 100644 --- a/common/datastorequery.cpp +++ b/common/datastorequery.cpp @@ -46,14 +46,10 @@ class Source : public FilterBase { QVector mIncrementalIds; QVector::ConstIterator mIncrementalIt; - Source (const QVector &ids, DataStoreQuery *store) + Source (const QVector &ids, DataStoreQuery *store) : FilterBase(store), - mIds() + mIds(ids) { - mIds.reserve(ids.size()); - for (const auto &id : ids) { - mIds.append(Identifier::fromDisplayByteArray(id)); - } mIt = mIds.constBegin(); } @@ -614,7 +610,11 @@ void DataStoreQuery::setupQuery(const Sink::QueryBase &query_) mSource = [&]() { if (!query.ids().isEmpty()) { //We have a set of ids as a starting point - return Source::Ptr::create(query.ids().toVector(), this); + QVector ids; + for (const auto & id: query.ids()) { + ids.append(Identifier::fromDisplayByteArray(id)); + } + return Source::Ptr::create(ids, this); } else { QSet appliedFilters; auto resultSet = mStore.indexLookup(mType, query, appliedFilters, appliedSorting); diff --git a/common/storage/entitystore.cpp b/common/storage/entitystore.cpp index 71690fe..1ba7c6c 100644 --- a/common/storage/entitystore.cpp +++ b/common/storage/entitystore.cpp @@ -426,19 +426,19 @@ bool EntityStore::cleanupRevisions(qint64 revision) return cleanupIsNecessary; } -QVector EntityStore::fullScan(const QByteArray &type) +QVector EntityStore::fullScan(const QByteArray &type) { SinkTraceCtx(d->logCtx) << "Looking for : " << type; if (!d->exists()) { SinkTraceCtx(d->logCtx) << "Database is not existing: " << type; - return QVector(); + return {}; } //The scan can return duplicate results if we have multiple revisions, so we use a set to deduplicate. - QSet keys; + QSet keys; DataStore::mainDatabase(d->getTransaction(), type) .scan(QByteArray(), [&](const QByteArray &key, const QByteArray &value) -> bool { - const auto uid = Sink::Storage::Key::fromInternalByteArray(key).identifier().toDisplayByteArray(); + const auto uid = Sink::Storage::Key::fromInternalByteArray(key).identifier(); if (keys.contains(uid)) { //Not something that should persist if the replay works, so we keep a message for now. SinkTraceCtx(d->logCtx) << "Multiple revisions for key: " << key; @@ -452,11 +452,11 @@ QVector EntityStore::fullScan(const QByteArray &type) return keys.toList().toVector(); } -QVector EntityStore::indexLookup(const QByteArray &type, const QueryBase &query, QSet &appliedFilters, QByteArray &appliedSorting) +QVector EntityStore::indexLookup(const QByteArray &type, const QueryBase &query, QSet &appliedFilters, QByteArray &appliedSorting) { if (!d->exists()) { SinkTraceCtx(d->logCtx) << "Database is not existing: " << type; - return QVector(); + return {}; } return d->typeIndex(type).query(query, appliedFilters, appliedSorting, d->getTransaction(), d->resourceContext.instanceId()); } diff --git a/common/storage/entitystore.h b/common/storage/entitystore.h index 619b884..7979798 100644 --- a/common/storage/entitystore.h +++ b/common/storage/entitystore.h @@ -57,8 +57,8 @@ public: void abortTransaction(); bool hasTransaction() const; - QVector fullScan(const QByteArray &type); - QVector indexLookup(const QByteArray &type, const QueryBase &query, QSet &appliedFilters, QByteArray &appliedSorting); + QVector fullScan(const QByteArray &type); + QVector indexLookup(const QByteArray &type, const QueryBase &query, QSet &appliedFilters, QByteArray &appliedSorting); QVector indexLookup(const QByteArray &type, const QByteArray &property, const QVariant &value); void indexLookup(const QByteArray &type, const QByteArray &property, const QVariant &value, const std::function &callback); template diff --git a/common/storage/key.cpp b/common/storage/key.cpp index 23d7a6a..2327061 100644 --- a/common/storage/key.cpp +++ b/common/storage/key.cpp @@ -26,6 +26,12 @@ using Sink::Storage::Identifier; using Sink::Storage::Key; using Sink::Storage::Revision; + +uint Sink::Storage::qHash(const Sink::Storage::Identifier &identifier) +{ + return qHash(identifier.toInternalByteArray()); +} + QDebug &operator<<(QDebug &dbg, const Identifier &id) { dbg << id.toDisplayString(); diff --git a/common/storage/key.h b/common/storage/key.h index 211aea7..baabe38 100644 --- a/common/storage/key.h +++ b/common/storage/key.h @@ -119,6 +119,8 @@ private: Revision rev; }; +SINK_EXPORT uint qHash(const Sink::Storage::Identifier &); + } // namespace Storage } // namespace Sink diff --git a/common/typeindex.cpp b/common/typeindex.cpp index 47512e8..646a60f 100644 --- a/common/typeindex.cpp +++ b/common/typeindex.cpp @@ -265,10 +265,10 @@ void TypeIndex::remove(const Identifier &identifier, const Sink::ApplicationDoma } } -static QVector indexLookup(Index &index, QueryBase::Comparator filter, +static QVector indexLookup(Index &index, QueryBase::Comparator filter, std::function valueToKey = getByteArray) { - QVector keys; + QVector keys; QByteArrayList lookupKeys; if (filter.comparator == Query::Comparator::Equals) { lookupKeys << valueToKey(filter.value); @@ -283,7 +283,7 @@ static QVector indexLookup(Index &index, QueryBase::Comparator filte for (const auto &lookupKey : lookupKeys) { index.lookup(lookupKey, [&](const QByteArray &value) { - keys << Identifier::fromInternalByteArray(value).toDisplayByteArray(); + keys << Identifier::fromInternalByteArray(value); }, [lookupKey](const Index::Error &error) { SinkWarning() << "Lookup error in index: " << error.message << lookupKey; @@ -293,7 +293,7 @@ static QVector indexLookup(Index &index, QueryBase::Comparator filte return keys; } -static QVector sortedIndexLookup(Index &index, QueryBase::Comparator filter) +static QVector sortedIndexLookup(Index &index, QueryBase::Comparator filter) { if (filter.comparator == Query::Comparator::In || filter.comparator == Query::Comparator::Contains) { SinkWarning() << "In and Contains comparison not supported on sorted indexes"; @@ -303,7 +303,7 @@ static QVector sortedIndexLookup(Index &index, QueryBase::Comparator return indexLookup(index, filter, toSortableByteArray); } - QVector keys; + QVector keys; QByteArray lowerBound, upperBound; auto bounds = filter.value.value(); @@ -318,7 +318,7 @@ static QVector sortedIndexLookup(Index &index, QueryBase::Comparator index.rangeLookup(lowerBound, upperBound, [&](const QByteArray &value) { - keys << Identifier::fromInternalByteArray(value).toDisplayByteArray(); + keys << Identifier::fromInternalByteArray(value); }, [bounds](const Index::Error &error) { SinkWarning() << "Lookup error in index:" << error.message @@ -328,14 +328,14 @@ static QVector sortedIndexLookup(Index &index, QueryBase::Comparator return keys; } -static QVector sampledIndexLookup(Index &index, QueryBase::Comparator filter) +static QVector sampledIndexLookup(Index &index, QueryBase::Comparator filter) { if (filter.comparator != Query::Comparator::Overlap) { SinkWarning() << "Comparisons other than Overlap not supported on sampled period indexes"; return {}; } - QVector keys; + QVector keys; auto bounds = filter.value.value(); @@ -349,7 +349,7 @@ static QVector sampledIndexLookup(Index &index, QueryBase::Comparato index.rangeLookup(lowerBucket, upperBucket, [&](const QByteArray &value) { - keys << Identifier::fromInternalByteArray(value).toDisplayByteArray(); + keys << Identifier::fromInternalByteArray(value); }, [bounds](const Index::Error &error) { SinkWarning() << "Lookup error in index:" << error.message @@ -359,13 +359,18 @@ static QVector sampledIndexLookup(Index &index, QueryBase::Comparato return keys; } -QVector TypeIndex::query(const Sink::QueryBase &query, QSet &appliedFilters, QByteArray &appliedSorting, Sink::Storage::DataStore::Transaction &transaction, const QByteArray &resourceInstanceId) +QVector TypeIndex::query(const Sink::QueryBase &query, QSet &appliedFilters, QByteArray &appliedSorting, Sink::Storage::DataStore::Transaction &transaction, const QByteArray &resourceInstanceId) { const auto baseFilters = query.getBaseFilters(); for (auto it = baseFilters.constBegin(); it != baseFilters.constEnd(); it++) { if (it.value().comparator == QueryBase::Comparator::Fulltext) { FulltextIndex fulltextIndex{resourceInstanceId}; - const auto keys = fulltextIndex.lookup(it.value().value.toString()); + QVector keys; + const auto ids = fulltextIndex.lookup(it.value().value.toString()); + keys.reserve(ids.size()); + for (const auto &id : ids) { + keys.append(Identifier::fromDisplayByteArray(id)); + } appliedFilters << it.key(); SinkTraceCtx(mLogCtx) << "Fulltext index lookup found " << keys.size() << " keys."; return keys; diff --git a/common/typeindex.h b/common/typeindex.h index f2cabaf..875eb7a 100644 --- a/common/typeindex.h +++ b/common/typeindex.h @@ -94,7 +94,7 @@ public: void modify(const Sink::Storage::Identifier &identifier, const Sink::ApplicationDomain::ApplicationDomainType &oldEntity, const Sink::ApplicationDomain::ApplicationDomainType &newEntity, Sink::Storage::DataStore::Transaction &transaction, const QByteArray &resourceInstanceId); void remove(const Sink::Storage::Identifier &identifier, const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Storage::DataStore::Transaction &transaction, const QByteArray &resourceInstanceId); - QVector query(const Sink::QueryBase &query, QSet &appliedFilters, QByteArray &appliedSorting, Sink::Storage::DataStore::Transaction &transaction, const QByteArray &resourceInstanceId); + QVector query(const Sink::QueryBase &query, QSet &appliedFilters, QByteArray &appliedSorting, Sink::Storage::DataStore::Transaction &transaction, const QByteArray &resourceInstanceId); QVector lookup(const QByteArray &property, const QVariant &value, Sink::Storage::DataStore::Transaction &transaction); template -- cgit v1.2.3