https://bugreports.qt.io/browse/QTBUG-130767 https://bugs.kde.org/show_bug.cgi?id=494804 https://codereview.qt-project.org/c/qt/qtdeclarative/+/604180 (+required https://code.qt.io/cgit/qt/qtdeclarative.git/commit/?id=104b0d6e88) --- a/src/qml/jsruntime/qv4qobjectwrapper_p.h +++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h @@ -224,2 +224,15 @@ +// We generally musn't pass ReturnedValue as arguments to other functions. +// In this case, we do it solely for marking purposes so it's fine. +inline void markIfPastMarkWeakValues(ExecutionEngine *engine, ReturnedValue rv) +{ + const auto gcState = engine->memoryManager->gcStateMachine->state; + if (gcState != GCStateMachine::Invalid && gcState >= GCState::MarkWeakValues) { + QV4::WriteBarrier::markCustom(engine, [rv](QV4::MarkStack *ms) { + auto *m = StaticValue::fromReturnedValue(rv).m(); + m->mark(ms); + }); + } +} + inline ReturnedValue QObjectWrapper::wrap(ExecutionEngine *engine, QObject *object) @@ -235,3 +248,5 @@ - return wrap_slowPath(engine, object); + const auto rv = wrap_slowPath(engine, object); + markIfPastMarkWeakValues(engine, rv); + return rv; } @@ -244,3 +259,5 @@ - return wrapConst_slowPath(engine, object); + const auto rv = wrapConst_slowPath(engine, object); + markIfPastMarkWeakValues(engine, rv); + return rv; } --- a/src/qml/memory/qv4mm.cpp +++ b/src/qml/memory/qv4mm.cpp @@ -62,2 +62,4 @@ Q_DECLARE_LOGGING_CATEGORY(lcGcAllocatorStats) +Q_LOGGING_CATEGORY(lcGcStateTransitions, "qt.qml.gc.stateTransitions") +Q_DECLARE_LOGGING_CATEGORY(lcGcStateTransitions) @@ -682,3 +684,3 @@ that->mm->engine->isGCOngoing = true; - return MarkGlobalObject; + return GCState::MarkGlobalObject; } @@ -688,3 +690,3 @@ that->mm->engine->markObjects(that->mm->m_markStack.get()); - return MarkJSStack; + return GCState::MarkJSStack; } @@ -694,3 +696,3 @@ that->mm->collectFromJSStack(that->mm->markStack()); - return InitMarkPersistentValues; + return GCState::InitMarkPersistentValues; } @@ -700,5 +702,5 @@ if (!that->mm->m_persistentValues) - return InitMarkWeakValues; // no persistent values to mark + return GCState::InitMarkWeakValues; // no persistent values to mark stateData = GCIteratorStorage { that->mm->m_persistentValues->begin() }; - return MarkPersistentValues; + return GCState::MarkPersistentValues; } @@ -719,3 +721,3 @@ if (wasDrainNecessary(markStack, that->deadline) && that->deadline.hasExpired()) - return MarkPersistentValues; + return GCState::MarkPersistentValues; PersistentValueStorage::Iterator& it = get(stateData).it; @@ -724,3 +726,3 @@ if (!it.p) - return InitMarkWeakValues; + return GCState::InitMarkWeakValues; if (Managed *m = (*it).as()) @@ -729,3 +731,3 @@ } - return MarkPersistentValues; + return GCState::MarkPersistentValues; } @@ -735,3 +737,3 @@ stateData = GCIteratorStorage { that->mm->m_weakValues->begin() }; - return MarkWeakValues; + return GCState::MarkWeakValues; } @@ -742,3 +744,3 @@ if (wasDrainNecessary(markStack, that->deadline) && that->deadline.hasExpired()) - return MarkWeakValues; + return GCState::MarkWeakValues; PersistentValueStorage::Iterator& it = get(stateData).it; @@ -747,3 +749,3 @@ if (!it.p) - return MarkDrain; + return GCState::MarkDrain; QObjectWrapper *qobjectWrapper = (*it).as(); @@ -768,3 +770,3 @@ } - return MarkWeakValues; + return GCState::MarkWeakValues; } @@ -775,3 +777,3 @@ that->mm->markStack()->drain(); - return MarkReady; + return GCState::MarkReady; } @@ -779,4 +781,4 @@ return drainState == MarkStack::DrainState::Complete - ? MarkReady - : MarkDrain; + ? GCState::MarkReady + : GCState::MarkDrain; } @@ -786,3 +788,3 @@ //Possibility to do some clean up, stat printing, etc... - return InitCallDestroyObjects; + return GCState::InitCallDestroyObjects; } @@ -803,5 +805,5 @@ if (!that->mm->m_weakValues) - return FreeWeakMaps; // no need to call destroy objects + return GCState::FreeWeakMaps; // no need to call destroy objects stateData = GCIteratorStorage { that->mm->m_weakValues->begin() }; - return CallDestroyObjects; + return GCState::CallDestroyObjects; } @@ -818,3 +820,3 @@ if (!it.p) - return FreeWeakMaps; + return GCState::FreeWeakMaps; Managed *m = (*it).managed(); @@ -828,3 +830,3 @@ } - return CallDestroyObjects; + return GCState::CallDestroyObjects; } @@ -845,3 +847,3 @@ freeWeakMaps(that->mm); - return FreeWeakSets; + return GCState::FreeWeakSets; } @@ -863,3 +865,3 @@ freeWeakSets(that->mm); - return HandleQObjectWrappers; + return GCState::HandleQObjectWrappers; } @@ -869,3 +871,3 @@ that->mm->cleanupDeletedQObjectWrappersInSweep(); - return DoSweep; + return GCState::DoSweep; } @@ -893,3 +895,3 @@ - return Invalid; + return GCState::Invalid; } @@ -1493,4 +1495,8 @@ } + qCDebug(lcGcStateTransitions) << "Preparing to execute the" + << QMetaEnum::fromType().key(state) << "state"; GCStateInfo& stateInfo = stateInfoMap[int(state)]; state = stateInfo.execute(this, stateData); + qCDebug(lcGcStateTransitions) << "Transitioning to the" + << QMetaEnum::fromType().key(state) << "state"; if (stateInfo.breakAfter) @@ -1507,4 +1513,8 @@ while (state != GCState::Invalid) { + qCDebug(lcGcStateTransitions) << "Preparing to execute the" + << QMetaEnum::fromType().key(state) << "state"; GCStateInfo& stateInfo = stateInfoMap[int(state)]; state = stateInfo.execute(this, stateData); + qCDebug(lcGcStateTransitions) << "Transitioning to the" + << QMetaEnum::fromType().key(state) << "state"; } @@ -1516 +1526,3 @@ QT_END_NAMESPACE + +#include "moc_qv4mm_p.cpp" --- a/src/qml/memory/qv4mm_p.h +++ b/src/qml/memory/qv4mm_p.h @@ -30,22 +30,2 @@ -enum GCState { - MarkStart = 0, - MarkGlobalObject, - MarkJSStack, - InitMarkPersistentValues, - MarkPersistentValues, - InitMarkWeakValues, - MarkWeakValues, - MarkDrain, - MarkReady, - InitCallDestroyObjects, - CallDestroyObjects, - FreeWeakMaps, - FreeWeakSets, - HandleQObjectWrappers, - DoSweep, - Invalid, - Count, -}; - struct GCData { virtual ~GCData(){};}; @@ -55,11 +35,39 @@ }; -struct GCStateMachine; - -struct GCStateInfo { - using ExtraData = std::variant; - GCState (*execute)(GCStateMachine *, ExtraData &) = nullptr; // Function to execute for this state, returns true if ready to transition - bool breakAfter{false}; -}; struct GCStateMachine { + Q_GADGET_EXPORT(Q_QML_EXPORT) + +public: + enum GCState { + MarkStart = 0, + MarkGlobalObject, + MarkJSStack, + InitMarkPersistentValues, + MarkPersistentValues, + InitMarkWeakValues, + MarkWeakValues, + MarkDrain, + MarkReady, + InitCallDestroyObjects, + CallDestroyObjects, + FreeWeakMaps, + FreeWeakSets, + HandleQObjectWrappers, + DoSweep, + Invalid, + Count, + }; + Q_ENUM(GCState) + + struct StepTiming { + qint64 rolling_sum = 0; + qint64 count = 0; + }; + + struct GCStateInfo { + using ExtraData = std::variant; + GCState (*execute)(GCStateMachine *, ExtraData &) = nullptr; // Function to execute for this state, returns true if ready to transition + bool breakAfter{false}; + }; + using ExtraData = GCStateInfo::ExtraData; @@ -96,2 +104,4 @@ +using GCState = GCStateMachine::GCState; +using GCStateInfo = GCStateMachine::GCStateInfo;