Significantly improve performance of binding evaluation

Avoid any QVariant or type dependent code in the cpp files.
Instead, let the binding wrapper determine if the value
has changed and return true/false accordingly.

This required also some reworking of the guard mechanism
for notified properties, where the guard function wrapper
now calls first the binding evaluation function and then
passes the result to the guard.

Change-Id: I350d07a508ccc0c5db7054a0efa4f270b6a78ec3
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Lars Knoll 2020-07-08 12:07:13 +02:00
parent be1ce6b269
commit bbfecdee1e
6 changed files with 81 additions and 55 deletions

View File

@ -95,8 +95,8 @@ QPropertyBase::~QPropertyBase()
QUntypedPropertyBinding QPropertyBase::setBinding(const QUntypedPropertyBinding &binding,
void *propertyDataPtr,
void *staticObserver,
void (*staticObserverCallback)(void*, void*),
bool (*guardCallback)(void *, void*))
QPropertyObserverCallback staticObserverCallback,
QtPrivate::QPropertyGuardFunction guardCallback)
{
QPropertyBindingPrivatePtr oldBinding;
QPropertyBindingPrivatePtr newBinding = binding.d;

View File

@ -120,7 +120,7 @@ class Q_CORE_EXPORT QUntypedPropertyBinding
{
public:
// writes binding result into dataPtr
using BindingEvaluationFunction = std::function<void(const QMetaType &metaType, void *dataPtr)>;
using BindingEvaluationFunction = QtPrivate::QPropertyBindingFunction;
QUntypedPropertyBinding();
QUntypedPropertyBinding(const QMetaType &metaType, BindingEvaluationFunction function, const QPropertyBindingSourceLocation &location);
@ -151,10 +151,14 @@ class QPropertyBinding : public QUntypedPropertyBinding
struct BindingAdaptor
{
Functor impl;
void operator()(const QMetaType &/*metaType*/, void *dataPtr)
bool operator()(const QMetaType &/*metaType*/, void *dataPtr)
{
PropertyType *propertyPtr = static_cast<PropertyType *>(dataPtr);
*propertyPtr = impl();
PropertyType newValue = impl();
if (newValue == *propertyPtr)
return false;
*propertyPtr = std::move(newValue);
return true;
}
};
@ -352,20 +356,12 @@ namespace Qt {
}
}
namespace detail {
template <typename F>
struct ExtractClassFromFunctionPointer;
template<typename T, typename C>
struct ExtractClassFromFunctionPointer<T C::*> { using Class = C; };
}
template <typename T, auto Callback, auto ValueGuard=nullptr>
class QNotifiedProperty
{
public:
using value_type = T;
using Class = typename detail::ExtractClassFromFunctionPointer<decltype(Callback)>::Class;
using Class = typename QtPrivate::detail::ExtractClassFromFunctionPointer<decltype(Callback)>::Class;
private:
static bool constexpr ValueGuardModifiesArgument = std::is_invocable_r_v<bool, decltype(ValueGuard), Class, T&>;
static bool constexpr CallbackAcceptsOldValue = std::is_invocable_v<decltype(Callback), Class, T>;
@ -378,17 +374,8 @@ public:
!HasValueGuard,
"Guard has wrong signature");
private:
// type erased guard functions, casts its arguments to the correct types
static constexpr bool (*GuardTEHelper)(void *, void*) = [](void *o, void *newValue){
if constexpr (HasValueGuard) { // Guard->* is invalid if Guard == nullptr
return (reinterpret_cast<Class *>(o)->*(ValueGuard))(*static_cast<T *>(newValue));
} else {
Q_UNUSED(o); // some compilers complain about unused variables
Q_UNUSED(newValue);
return true;
}
};
static constexpr bool(*GuardTE)(void *, void*) = HasValueGuard ? GuardTEHelper : nullptr;
static constexpr QtPrivate::QPropertyGuardFunction GuardTE =
QtPrivate::QPropertyGuardFunctionHelper<T, Class, ValueGuard>::guard;
public:
QNotifiedProperty() = default;

View File

@ -104,31 +104,26 @@ bool QPropertyBindingPrivate::evaluateIfDirtyAndReturnTrueIfValueChanged()
BindingEvaluationState evaluationFrame(this);
bool changed = false;
if (metaType.id() == QMetaType::Bool) {
auto propertyPtr = reinterpret_cast<QPropertyBase *>(propertyDataPtr);
bool newValue = false;
evaluationFunction(metaType, &newValue);
if (!error.hasError()) {
bool updateAllowed = true;
if (hasStaticObserver && staticGuardCallback)
updateAllowed = staticGuardCallback(staticObserver, &newValue);
if (updateAllowed && propertyPtr->extraBit() != newValue) {
if (hasStaticObserver && staticGuardCallback) {
if (metaType.id() == QMetaType::Bool) {
auto propertyPtr = reinterpret_cast<QPropertyBase *>(propertyDataPtr);
bool newValue = propertyPtr->extraBit();
changed = staticGuardCallback(metaType, &newValue, evaluationFunction, staticObserver);
if (changed && !error.hasError())
propertyPtr->setExtraBit(newValue);
changed = true;
}
} else {
changed = staticGuardCallback(metaType, propertyDataPtr, evaluationFunction, staticObserver);
}
} else {
QVariant resultVariant(metaType.id(), nullptr);
evaluationFunction(metaType, resultVariant.data());
if (!error.hasError()) {
bool updateAllowed = true;
if (hasStaticObserver && staticGuardCallback)
updateAllowed = staticGuardCallback(staticObserver, resultVariant.data());
if (updateAllowed && !metaType.equals(propertyDataPtr, resultVariant.constData())) {
changed = true;
metaType.destruct(propertyDataPtr);
metaType.construct(propertyDataPtr, resultVariant.constData());
}
if (metaType.id() == QMetaType::Bool) {
auto propertyPtr = reinterpret_cast<QPropertyBase *>(propertyDataPtr);
bool newValue = propertyPtr->extraBit();
changed = evaluationFunction(metaType, &newValue);
if (changed && !error.hasError())
propertyPtr->setExtraBit(newValue);
} else {
changed = evaluationFunction(metaType, propertyDataPtr);
}
}

View File

@ -80,8 +80,8 @@ private:
ObserverArray inlineDependencyObservers;
struct {
void *staticObserver;
void (*staticObserverCallback)(void*, void*);
bool (*staticGuardCallback)(void*, void*);
QtPrivate::QPropertyObserverCallback staticObserverCallback;
QtPrivate::QPropertyGuardFunction staticGuardCallback;
};
};
QScopedPointer<std::vector<QPropertyObserver>> heapObservers;
@ -108,7 +108,8 @@ public:
void setDirty(bool d) { dirty = d; }
void setProperty(void *propertyPtr) { propertyDataPtr = propertyPtr; }
void setStaticObserver(void *observer, void (*callback)(void*, void*), bool (*guardCallback)(void *, void*))
void setStaticObserver(void *observer, QtPrivate::QPropertyObserverCallback callback,
QtPrivate::QPropertyGuardFunction guardCallback)
{
if (observer) {
if (!hasStaticObserver) {

View File

@ -61,9 +61,17 @@ class QUntypedPropertyBinding;
class QPropertyBindingPrivate;
using QPropertyBindingPrivatePtr = QExplicitlySharedDataPointer<QPropertyBindingPrivate>;
struct QPropertyBasePointer;
class QMetaType;
namespace QtPrivate {
// writes binding result into dataPtr
using QPropertyBindingFunction = std::function<bool(const QMetaType &metaType, void *dataPtr)>;
using QPropertyGuardFunction = bool(*)(const QMetaType &, void *dataPtr,
QPropertyBindingFunction, void *owner);
using QPropertyObserverCallback = void (*)(void *, void *);
class Q_CORE_EXPORT QPropertyBase
{
// Mutable because the address of the observer of the currently evaluating binding is stored here, for
@ -84,8 +92,8 @@ public:
QUntypedPropertyBinding setBinding(const QUntypedPropertyBinding &newBinding,
void *propertyDataPtr, void *staticObserver = nullptr,
void (*staticObserverCallback)(void *, void *) = nullptr,
bool (*guardCallback)(void *, void *) = nullptr);
QPropertyObserverCallback staticObserverCallback = nullptr,
QPropertyGuardFunction guardCallback = nullptr);
QPropertyBindingPrivate *binding();
void evaluateIfDirty();
@ -211,6 +219,38 @@ private:
quintptr *d = nullptr;
};
namespace detail {
template <typename F>
struct ExtractClassFromFunctionPointer;
template<typename T, typename C>
struct ExtractClassFromFunctionPointer<T C::*> { using Class = C; };
}
// type erased guard functions, casts its arguments to the correct types
template<typename T, typename Class, auto Guard, bool = std::is_same_v<decltype(Guard), std::nullptr_t>>
struct QPropertyGuardFunctionHelper
{
static constexpr QPropertyGuardFunction guard = nullptr;
};
template<typename T, typename Class, auto Guard>
struct QPropertyGuardFunctionHelper<T, Class, Guard, false>
{
static auto guard(const QMetaType &metaType, void *dataPtr,
QPropertyBindingFunction eval, void *owner) -> bool
{
T t;
eval(metaType, &t);
if (!(static_cast<Class *>(owner)->*Guard)(t))
return false;
T *data = static_cast<T *>(dataPtr);
if (*data == t)
return false;
*data = std::move(t);
return true;
};
};
} // namespace QtPrivate
QT_END_NAMESPACE

View File

@ -622,18 +622,20 @@ void tst_QProperty::genericPropertyBinding()
{
QUntypedPropertyBinding doubleBinding(QMetaType::fromType<double>(),
[](const QMetaType &, void *) -> void {
[](const QMetaType &, void *) -> bool {
Q_ASSERT(false);
return true;
}, QPropertyBindingSourceLocation());
QVERIFY(!property.setBinding(doubleBinding));
}
QUntypedPropertyBinding intBinding(QMetaType::fromType<int>(),
[](const QMetaType &metaType, void *dataPtr) -> void {
[](const QMetaType &metaType, void *dataPtr) -> bool {
Q_ASSERT(metaType.id() == qMetaTypeId<int>());
int *intPtr = reinterpret_cast<int*>(dataPtr);
*intPtr = 100;
return true;
}, QPropertyBindingSourceLocation());
QVERIFY(property.setBinding(intBinding));
@ -648,9 +650,10 @@ void tst_QProperty::genericPropertyBindingBool()
QVERIFY(!property.value());
QUntypedPropertyBinding boolBinding(QMetaType::fromType<bool>(),
[](const QMetaType &, void *dataPtr) -> void {
[](const QMetaType &, void *dataPtr) -> bool {
auto boolPtr = reinterpret_cast<bool *>(dataPtr);
*boolPtr = true;
return true;
}, QPropertyBindingSourceLocation());
QVERIFY(property.setBinding(boolBinding));