Separate the error case when evaluating bindings

There's no point in returning a usually empty error when
evaluating bindings, adding overhead to the regular code
path.

Instead, the error can be set on the currently evaluating
binding if required. This streamlines the functor used to
wrap the binding and should thus expand to less code and
execute faster in the regular case.

To achieve this, expose a pointer to the currently evaluating
binding in the private API (as QtQml needs it to be able to
report errors).

The error case now requires one additional TLS lookup, but
we don't really care about performance in that case anyway.

Change-Id: Iecb450e765244930a41d813fcf8eb4013957a6a3
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Lars Knoll 2020-07-08 10:57:21 +02:00
parent 0d1208f0f0
commit be1ce6b269
5 changed files with 25 additions and 45 deletions

View File

@ -206,6 +206,11 @@ BindingEvaluationState::~BindingEvaluationState()
*currentState = previousState;
}
QPropertyBindingPrivate *QPropertyBindingPrivate::currentlyEvaluatingBinding()
{
return currentBindingEvaluationState ? currentBindingEvaluationState->binding : nullptr;
}
void QPropertyBase::evaluateIfDirty()
{
QPropertyBasePointer d{this};

View File

@ -119,9 +119,8 @@ private:
class Q_CORE_EXPORT QUntypedPropertyBinding
{
public:
using BindingEvaluationResult = QPropertyBindingError;
// writes binding result into dataPtr
using BindingEvaluationFunction = std::function<BindingEvaluationResult(const QMetaType &metaType, void *dataPtr)>;
using BindingEvaluationFunction = std::function<void(const QMetaType &metaType, void *dataPtr)>;
QUntypedPropertyBinding();
QUntypedPropertyBinding(const QMetaType &metaType, BindingEvaluationFunction function, const QPropertyBindingSourceLocation &location);
@ -152,19 +151,10 @@ class QPropertyBinding : public QUntypedPropertyBinding
struct BindingAdaptor
{
Functor impl;
QUntypedPropertyBinding::BindingEvaluationResult operator()(const QMetaType &/*metaType*/, void *dataPtr)
void operator()(const QMetaType &/*metaType*/, void *dataPtr)
{
std::variant<PropertyType, QPropertyBindingError> result(impl());
if (auto errorPtr = std::get_if<QPropertyBindingError>(&result))
return *errorPtr;
if (auto valuePtr = std::get_if<PropertyType>(&result)) {
PropertyType *propertyPtr = reinterpret_cast<PropertyType *>(dataPtr);
*propertyPtr = std::move(*valuePtr);
return {};
}
return {};
PropertyType *propertyPtr = static_cast<PropertyType *>(dataPtr);
*propertyPtr = impl();
}
};
@ -191,25 +181,12 @@ public:
{}
};
namespace QtPrivate {
template<typename... Ts>
constexpr auto is_variant_v = false;
template<typename... Ts>
constexpr auto is_variant_v<std::variant<Ts...>> = true;
}
namespace Qt {
template <typename Functor>
auto makePropertyBinding(Functor &&f, const QPropertyBindingSourceLocation &location = QT_PROPERTY_DEFAULT_BINDING_LOCATION,
std::enable_if_t<std::is_invocable_v<Functor>> * = 0)
{
if constexpr (QtPrivate::is_variant_v<std::invoke_result_t<Functor>>) {
return QPropertyBinding<std::variant_alternative_t<0, std::invoke_result_t<Functor>>>(std::forward<Functor>(f), location);
} else {
return QPropertyBinding<std::invoke_result_t<Functor>>(std::forward<Functor>(f), location);
}
// Work around bogus warning
Q_UNUSED(QtPrivate::is_variant_v<bool>);
return QPropertyBinding<std::invoke_result_t<Functor>>(std::forward<Functor>(f), location);
}
}

View File

@ -103,14 +103,12 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
BindingEvaluationState evaluationFrame(this);
QPropertyBindingError evalError;
QUntypedPropertyBinding::BindingEvaluationResult result;
bool changed = false;
if (metaType.id() == QMetaType::Bool) {
auto propertyPtr = reinterpret_cast<QPropertyBase *>(propertyDataPtr);
bool newValue = false;
evalError = evaluationFunction(metaType, &newValue);
if (evalError.type() == QPropertyBindingError::NoError) {
evaluationFunction(metaType, &newValue);
if (!error.hasError()) {
bool updateAllowed = true;
if (hasStaticObserver && staticGuardCallback)
updateAllowed = staticGuardCallback(staticObserver, &newValue);
@ -121,8 +119,8 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
}
} else {
QVariant resultVariant(metaType.id(), nullptr);
evalError = evaluationFunction(metaType, resultVariant.data());
if (evalError.type() == QPropertyBindingError::NoError) {
evaluationFunction(metaType, resultVariant.data());
if (!error.hasError()) {
bool updateAllowed = true;
if (hasStaticObserver && staticGuardCallback)
updateAllowed = staticGuardCallback(staticObserver, resultVariant.data());
@ -134,9 +132,6 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
}
}
if (evalError.type() != QPropertyBindingError::NoError)
error = evalError;
dirty = false;
return changed;
}

View File

@ -179,6 +179,11 @@ public:
static QPropertyBindingPrivate *get(const QUntypedPropertyBinding &binding)
{ return binding.d.data(); }
void setError(QPropertyBindingError &&e)
{ error = std::move(e); }
static QPropertyBindingPrivate *currentlyEvaluatingBinding();
};
QT_END_NAMESPACE

View File

@ -505,9 +505,10 @@ void tst_QProperty::bindingSourceLocation()
void tst_QProperty::bindingError()
{
QProperty<int> prop = Qt::makePropertyBinding([]() -> std::variant<int, QPropertyBindingError> {
QProperty<int> prop = Qt::makePropertyBinding([]() -> int {
QPropertyBindingError error(QPropertyBindingError::UnknownError, QLatin1String("my error"));
return error;
QPropertyBindingPrivate::currentlyEvaluatingBinding()->setError(std::move(error));
return 0;
});
QCOMPARE(prop.value(), 0);
QCOMPARE(prop.binding().error().description(), QString("my error"));
@ -621,20 +622,18 @@ void tst_QProperty::genericPropertyBinding()
{
QUntypedPropertyBinding doubleBinding(QMetaType::fromType<double>(),
[](const QMetaType &, void *) -> QUntypedPropertyBinding::BindingEvaluationResult {
[](const QMetaType &, void *) -> void {
Q_ASSERT(false);
return QPropertyBindingError::NoError;
}, QPropertyBindingSourceLocation());
QVERIFY(!property.setBinding(doubleBinding));
}
QUntypedPropertyBinding intBinding(QMetaType::fromType<int>(),
[](const QMetaType &metaType, void *dataPtr) -> QUntypedPropertyBinding::BindingEvaluationResult {
[](const QMetaType &metaType, void *dataPtr) -> void {
Q_ASSERT(metaType.id() == qMetaTypeId<int>());
int *intPtr = reinterpret_cast<int*>(dataPtr);
*intPtr = 100;
return QPropertyBindingError::NoError;
}, QPropertyBindingSourceLocation());
QVERIFY(property.setBinding(intBinding));
@ -649,10 +648,9 @@ void tst_QProperty::genericPropertyBindingBool()
QVERIFY(!property.value());
QUntypedPropertyBinding boolBinding(QMetaType::fromType<bool>(),
[](const QMetaType &, void *dataPtr) -> QUntypedPropertyBinding::BindingEvaluationResult {
[](const QMetaType &, void *dataPtr) -> void {
auto boolPtr = reinterpret_cast<bool *>(dataPtr);
*boolPtr = true;
return {};
}, QPropertyBindingSourceLocation());
QVERIFY(property.setBinding(boolBinding));