From 49a1d6a57b0668544423aca46c5db55c2596654b Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Tue, 22 Aug 2017 21:52:28 +0200 Subject: [PATCH] Avoid value copy when fulfilled from promise --- src/qtpromise/qpromise.h | 1 + src/qtpromise/qpromise.inl | 24 +++------- src/qtpromise/qpromise_p.h | 65 +++++++++++++++++++++----- tests/auto/benchmark/tst_benchmark.cpp | 31 ++++++++++++ 4 files changed, 92 insertions(+), 29 deletions(-) diff --git a/src/qtpromise/qpromise.h b/src/qtpromise/qpromise.h index 3e7ec50..643e67d 100644 --- a/src/qtpromise/qpromise.h +++ b/src/qtpromise/qpromise.h @@ -49,6 +49,7 @@ public: // STATIC inline static QPromise reject(E&& error); protected: + friend struct QtPromisePrivate::PromiseFulfill >; friend class QPromiseResolve; friend class QPromiseReject; diff --git a/src/qtpromise/qpromise.inl b/src/qtpromise/qpromise.inl index ce288b5..80bd2e2 100644 --- a/src/qtpromise/qpromise.inl +++ b/src/qtpromise/qpromise.inl @@ -12,28 +12,18 @@ public: : m_promise(new QPromise(std::move(p))) { } - void operator()(const T& value) const + template + void operator()(V&& value) const { - resolve(value); - } - - void operator()(T&& value) const - { - resolve(std::move(value)); + Q_ASSERT(!m_promise.isNull()); + if (m_promise->isPending()) { + m_promise->m_d->resolve(std::forward(value)); + m_promise->m_d->dispatch(); + } } private: QSharedPointer > m_promise; - - template - void resolve(U&& value) const - { - Q_ASSERT(!m_promise.isNull()); - if (m_promise->isPending()) { - m_promise->m_d->resolve(std::forward(value)); - m_promise->m_d->dispatch(); - } - } }; template <> diff --git a/src/qtpromise/qpromise_p.h b/src/qtpromise/qpromise_p.h index 9539161..439d50c 100644 --- a/src/qtpromise/qpromise_p.h +++ b/src/qtpromise/qpromise_p.h @@ -79,13 +79,17 @@ struct PromiseFulfill > const QtPromise::QPromiseResolve& resolve, const QtPromise::QPromiseReject& reject) { - promise.then( - [=](const T& value) { - resolve(value); - }, - [=]() { // catch all - reject(std::current_exception()); + if (promise.isFulfilled()) { + resolve(promise.m_d->value()); + } else if (promise.isRejected()) { + reject(promise.m_d->error()); + } else { + promise.then([=]() { + resolve(promise.m_d->value()); + }, [=]() { // catch all + reject(promise.m_d->error()); }); + } } }; @@ -98,13 +102,17 @@ struct PromiseFulfill > const TResolve& resolve, const TReject& reject) { - promise.then( - [=]() { + if (promise.isFulfilled()) { + resolve(); + } else if (promise.isRejected()) { + reject(promise.m_d->error()); + } else { + promise.then([=]() { resolve(); - }, - [=]() { // catch all - reject(std::current_exception()); + }, [=]() { // catch all + reject(promise.m_d->error()); }); + } } }; @@ -377,6 +385,20 @@ public: setSettled(); } + void reject(const QSharedPointer& error) + { + Q_ASSERT(isPending()); + Q_ASSERT(m_error.isNull()); + m_error = error; + this->setSettled(); + } + + const QSharedPointer& error() const + { + Q_ASSERT(isRejected()); + return m_error; + } + void dispatch() { if (isPending()) { @@ -438,6 +460,7 @@ class PromiseData : public PromiseDataBase public: void resolve(T&& value) { + Q_ASSERT(this->isPending()); Q_ASSERT(m_value.isNull()); m_value.reset(new T(std::move(value))); this->setSettled(); @@ -445,11 +468,26 @@ public: void resolve(const T& value) { + Q_ASSERT(this->isPending()); Q_ASSERT(m_value.isNull()); m_value.reset(new T(value)); this->setSettled(); } + void resolve(const QSharedPointer& value) + { + Q_ASSERT(this->isPending()); + Q_ASSERT(m_value.isNull()); + m_value = value; + this->setSettled(); + } + + const QSharedPointer& value() const + { + Q_ASSERT(this->isFulfilled()); + return m_value; + } + void notify(const QVector& handlers) Q_DECL_OVERRIDE { QSharedPointer value(m_value); @@ -473,7 +511,10 @@ class PromiseData : public PromiseDataBase using Handler = typename PromiseDataBase::Handler; public: - void resolve() { setSettled(); } + void resolve() + { + setSettled(); + } protected: void notify(const QVector& handlers) Q_DECL_OVERRIDE diff --git a/tests/auto/benchmark/tst_benchmark.cpp b/tests/auto/benchmark/tst_benchmark.cpp index e109ed7..4c4e791 100644 --- a/tests/auto/benchmark/tst_benchmark.cpp +++ b/tests/auto/benchmark/tst_benchmark.cpp @@ -14,6 +14,7 @@ private Q_SLOTS: void valueResolve(); void valueReject(); void valueThen(); + void valueDelayed(); void errorReject(); void errorThen(); @@ -162,6 +163,36 @@ void tst_benchmark::valueThen() } } +void tst_benchmark::valueDelayed() +{ + { // should not copy the value on continutation if fulfilled + int value = -1; + Data::logs().reset(); + QPromise::resolve(42).then([&](int res) { + return QPromise::resolve(Data(res + 1)); + }).then([&](const Data& res) { + value = res.value(); + }).wait(); + + QCOMPARE(Data::logs().ctor, 1); + QCOMPARE(Data::logs().copy, 0); + QCOMPARE(Data::logs().move, 1); // move value to the input promise data + QCOMPARE(Data::logs().refs, 0); + QCOMPARE(value, 43); + } + { // should not create value on continutation if rejected + Data::logs().reset(); + QPromise::resolve(42).then([&]() { + return QPromise::reject(QString("foo")); + }).wait(); + + QCOMPARE(Data::logs().ctor, 0); + QCOMPARE(Data::logs().copy, 0); + QCOMPARE(Data::logs().move, 0); + QCOMPARE(Data::logs().refs, 0); + } +} + void tst_benchmark::errorReject() { { // should create one copy of the error when rejected by rvalue