diff options
| author | Olivier De Cannière <olivier.decanniere@qt.io> | 2025-10-15 11:29:57 +0200 |
|---|---|---|
| committer | Olivier De Cannière <olivier.decanniere@qt.io> | 2025-11-13 09:00:55 +0100 |
| commit | f33d72c73ad6ab54ddfa2f73e721e600b7a43702 (patch) | |
| tree | 871c318344f300ea92b0cd5f975763a295484eab /src/qml/jsruntime/qv4referenceobject.cpp | |
| parent | 869bf6ffc03484ddf10ba405b7345120510d434c (diff) | |
JSRuntime: Don't immediately connect reference objects
The change in a7349e6433d092398d76cafa5408753f06892cd7 reduced the
number of readbacks of reference objects by using connections to set
the dirty state only when necessary.
Establishing connections, however, comes at a cost and this introduced a
regression in the delegates_item_childrenRect QmlBench benchmark.
The mitigation introduced in this patch is to delay creating the
connection. When a reference object is created, it does not read or
create a connection but is simply marked as dirty. Only on a subsequent
read from within a different statement than the reference object's
creation's statement, does a connection get created.
This keeps the benefit of reducing unnecessary reads on objects that are
used multiple times while not spending too much on creating connections.
This brings back the benchmark to its original level of performance.
The referenceObjectChainReadsBackAsRequiredBasedOnParentSignals test
was changed to expect 8 reads instead of 4. Since all the accesses are
part of the same statement, no connection is ever created and more
reads are required than before.
The referenceObjectDoesNotFetchWithoutNotifyEventDateObject test is
reformatted to separate the assignment from the reads. Otherwise no
connection is ever created.
The referenceObjectDoesNotLeakAConnectionToTheDestroyedSignalOnANotifyBindable
test can be updated to expect 0 connections. With this change, we only
connect if the reference object is read again from a different
statement.
Amends a7349e6433d092398d76cafa5408753f06892cd7
Fixes: QTBUG-140757
Pick-to: 6.10
Change-Id: I5d02ec6266c51fbbe5f2e01405081dd5a167a833
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Diffstat (limited to 'src/qml/jsruntime/qv4referenceobject.cpp')
| -rw-r--r-- | src/qml/jsruntime/qv4referenceobject.cpp | 70 |
1 files changed, 70 insertions, 0 deletions
diff --git a/src/qml/jsruntime/qv4referenceobject.cpp b/src/qml/jsruntime/qv4referenceobject.cpp index 336b715ca0..93be59b212 100644 --- a/src/qml/jsruntime/qv4referenceobject.cpp +++ b/src/qml/jsruntime/qv4referenceobject.cpp @@ -550,6 +550,76 @@ void Heap::ReferenceObject::connectToBindable(QObject *obj, int property, QQmlEn QObject::connect(obj, &QObject::destroyed, [this]() { setDirty(true); })); } +bool ReferenceObject::shouldConnect(Heap::ReferenceObject *ref) +{ + if (ref->isAlwaysDirty()) + return false; + + // We want to connect when we are reading the reference object from a statement that is not + // its creation statement. In other words, when first storing it in a variable and then + // reusing it, potentially multiple times. This ensures that the cost of the connection itself + // is only paid when the user separated the declaration from the usage in their code, in the + // hope that this catches more complex and frequent uses, like accesses in a loop, where + // unnecessary copies would impact performance more significantly, and not one time accesses. + if (CppStackFrame *frame = ref->internalClass->engine->currentStackFrame) { + const auto *refObject = static_cast<Heap::ReferenceObject *>(ref); + if (frame->v4Function && frame->v4Function == refObject->function() + && frame->statementNumber() == refObject->statementIndex()) { + return false; + } + } + return true; +} + +void ReferenceObject::connect(Heap::ReferenceObject *ref) +{ + int property = ref->property(); + Heap::Object *object = ref->object(); + + while (object + && object->internalClass->vtable->type != Managed::Type_V4QObjectWrapper + && object->internalClass->vtable->type != Managed::Type_QMLTypeWrapper) + { + const auto type = object->internalClass->vtable->type; + if (type != Managed::Type_V4ReferenceObject + && type != Managed::Type_V4Sequence + && type != Managed::Type_DateObject + && type != Managed::Type_QMLValueTypeWrapper) { + break; + } + + property = static_cast<QV4::Heap::ReferenceObject *>(object)->property(); + object = static_cast<QV4::Heap::ReferenceObject *>(object)->object(); + } + + const auto doConnect = [&](auto wrapper) { + // The object may be valid on the first read but not on the second, check for that. + if (QObject *obj = wrapper->object()) { + auto *refObject = static_cast<Heap::ReferenceObject *>(ref); + + auto *qmlEngine = object->internalClass->engine->qmlEngine(); + if (qmlEngine && obj->metaObject()->property(property).isBindable()) + refObject->connectToBindable(obj, property, qmlEngine); + else if (qmlEngine && obj->metaObject()->property(property).hasNotifySignal()) + refObject->connectToNotifySignal(obj, property, qmlEngine); + } + }; + + if (object && object->internalClass->vtable->type == Managed::Type_V4QObjectWrapper) { + doConnect(static_cast<QV4::Heap::QObjectWrapper *>(object)); + } else if (object && object->internalClass->vtable->type == Managed::Type_QMLTypeWrapper) { + auto wrapper = static_cast<QV4::Heap::QQmlTypeWrapper *>(object); + Scope scope(object->internalClass->engine); + Scoped<QV4::QQmlTypeWrapper> scopedWrapper(scope, wrapper); + doConnect(scopedWrapper); + } + + if (!ref->isConnected()) { + // Mark AlwaysDirty to prevent further connection failures on every read + ref->setAlwaysDirty(true); + } +} + } // namespace QV4 QT_END_NAMESPACE |