Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event queue compressing proxy #4566

Merged
merged 12 commits into from
Dec 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/control/control.cpp
src/control/controlaudiotaperpot.cpp
src/control/controlbehavior.cpp
src/control/controlcompressingproxy.cpp
src/control/controleffectknob.cpp
src/control/controlencoder.cpp
src/control/controlindicator.cpp
Expand Down Expand Up @@ -1589,6 +1590,7 @@ add_executable(mixxx-test
src/test/controller_mapping_validation_test.cpp
src/test/controllerscriptenginelegacy_test.cpp
src/test/controlobjecttest.cpp
src/test/controlobjectscripttest.cpp
src/test/coreservicestest.cpp
src/test/coverartcache_test.cpp
src/test/coverartutils_test.cpp
Expand Down
71 changes: 71 additions & 0 deletions src/control/controlcompressingproxy.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#include "control/controlcompressingproxy.h"

#include "moc_controlcompressingproxy.cpp"

namespace {
constexpr int kMaxNumOfRecursions = 128;
}

// Event queue compressing proxy
CompressingProxy::CompressingProxy(const ConfigKey& key,
const RuntimeLoggingCategory& logger,
QObject* pParent)
: QObject(pParent), m_key(key), m_logger(logger), m_recursionDepth(0) {
}

// This function is called recursive by QCoreApplication::sendPostedEvents, until no more events are in the queue.
// When the last event is found, QCoreApplication::sendPostedEvents returns for the processQueuedEvents() instance of this event,
// and m_recursiveSearchForLastEventOngoing is set to false, while returning true itself.
// All previous started instances of processQueuedEvents() will return false in consequence,
// because the return value depends on the member variable and not on the stack of the instance.
CompressingProxy::StateOfProcessQueuedEvent CompressingProxy::processQueuedEvents() {
m_recursiveSearchForLastEventOngoing = true;
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved

if (m_recursionDepth >= kMaxNumOfRecursions) {
// To many events in queue -> Delete all unprocessed events in queue, to prevent stack overflow
QCoreApplication::removePostedEvents(this, QEvent::MetaCall);

qCWarning(m_logger) << "Deleted unprocessed events for " + m_key.group +
", " + m_key.item +
", because too many events were in the queue. This "
"indicates a serious performance problem with the "
"controller mapping.";

// We just return, without resetting m_recursiveSearchForLastEventOngoing,
// this ensures that the event found in the last iteration will be send
return StateOfProcessQueuedEvent::NoEvent;
}

m_recursionDepth++;
// sendPostedEvents recursive executes slotValueChanged until no more events for this slot are in the queue
// Each call of QCoreApplication::sendPostedEvents triggers the processing of the next event in the queue,
// by sending the signal to execute slotValueChanged again
QCoreApplication::sendPostedEvents(this, QEvent::MetaCall);

if (m_recursiveSearchForLastEventOngoing && m_recursionDepth > 1) {
qCWarning(m_logger)
<< "Skipped" << (m_recursionDepth - 1)
<< "superseded events from " + m_key.group + ", " + m_key.item +
", because the controller mapping couldn't process "
"them fast enough.";
}

m_recursionDepth--;

// Execution continues here, when last event for this slot is processed

StateOfProcessQueuedEvent returnValue;
if (m_recursiveSearchForLastEventOngoing) {
returnValue = StateOfProcessQueuedEvent::LastEvent;
} else {
returnValue = StateOfProcessQueuedEvent::OutdatedEvent;
}
m_recursiveSearchForLastEventOngoing = false;
return returnValue;
}

void CompressingProxy::slotValueChanged(double value, QObject* obj) {
if (processQueuedEvents() == StateOfProcessQueuedEvent::LastEvent) {
emit signalValueChanged(value, obj);
}
}
38 changes: 38 additions & 0 deletions src/control/controlcompressingproxy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#pragma once

#include <QApplication>
#include <QMetaObject>

#include "preferences/configobject.h"
#include "util/runtimeloggingcategory.h"

class CompressingProxy : public QObject {
Q_OBJECT
private:
enum class StateOfProcessQueuedEvent {
LastEvent,
OutdatedEvent,
NoEvent
};
StateOfProcessQueuedEvent processQueuedEvents();

// Members needed for warning message
const ConfigKey m_key;
const RuntimeLoggingCategory m_logger;

// Functional members
bool m_recursiveSearchForLastEventOngoing;
int m_recursionDepth;

public slots:
void slotValueChanged(double value, QObject* obj);

signals:
void signalValueChanged(double, QObject*);

public:
// No default constructor, since the proxy must be a child of the object with the Qt event queue
explicit CompressingProxy(const ConfigKey& key,
const RuntimeLoggingCategory& logger,
QObject* pParent = nullptr);
};
71 changes: 61 additions & 10 deletions src/control/controlobjectscript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,63 @@
ControlObjectScript::ControlObjectScript(
const ConfigKey& key, const RuntimeLoggingCategory& logger, QObject* pParent)
: ControlProxy(key, pParent, ControlFlag::AllowMissingOrInvalid),
m_logger(logger) {
m_logger(logger),
m_proxy(key, logger, this),
m_skipSuperseded(false) {
}

bool ControlObjectScript::addScriptConnection(const ScriptConnection& conn) {
if (m_scriptConnections.isEmpty()) {
// Only connect the slots when they are actually needed
// by script connections.
connect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
this,
&ControlObjectScript::slotValueChanged,
Qt::QueuedConnection);
m_skipSuperseded = conn.skipSuperseded;
if (conn.skipSuperseded) {
connect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
&m_proxy,
&CompressingProxy::slotValueChanged,
Qt::QueuedConnection);
connect(&m_proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged,
Qt::DirectConnection);
} else {
connect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
this,
&ControlObjectScript::slotValueChanged,
Qt::QueuedConnection);
}
connect(this,
&ControlObjectScript::trigger,
this,
&ControlObjectScript::slotValueChanged,
Qt::QueuedConnection);
} else {
// At least one callback function is already connected to this CO
if (conn.skipSuperseded == false && m_skipSuperseded == true) {
// Disconnect proxy if this is first callback function connected with skipSuperseded false
m_skipSuperseded = false;
qCWarning(m_logger) << conn.key.group + ", " + conn.key.item +
"is connected to different callback functions with "
"differing state of the skipSuperseded. Disable "
"skipping of superseded events for all these "
"callback functions.";
disconnect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
&m_proxy,
&CompressingProxy::slotValueChanged);
disconnect(&m_proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged);
connect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
this,
&ControlObjectScript::slotValueChanged,
Qt::QueuedConnection);
}
}

for (const auto& priorConnection : qAsConst(m_scriptConnections)) {
Expand Down Expand Up @@ -54,10 +94,21 @@ bool ControlObjectScript::removeScriptConnection(const ScriptConnection& conn) {
}
if (m_scriptConnections.isEmpty()) {
// no ScriptConnections left, so disconnect signals
disconnect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
this,
&ControlObjectScript::slotValueChanged);
if (m_skipSuperseded) {
disconnect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
&m_proxy,
&CompressingProxy::slotValueChanged);
disconnect(&m_proxy,
&CompressingProxy::signalValueChanged,
this,
&ControlObjectScript::slotValueChanged);
} else {
disconnect(m_pControl.data(),
&ControlDoublePrivate::valueChanged,
this,
&ControlObjectScript::slotValueChanged);
}
disconnect(this,
&ControlObjectScript::trigger,
this,
Expand Down
6 changes: 5 additions & 1 deletion src/control/controlobjectscript.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <QVector>

#include "control/controlcompressingproxy.h"
#include "control/controlproxy.h"
#include "controllers/scripting/legacy/scriptconnection.h"
#include "util/runtimeloggingcategory.h"
Expand Down Expand Up @@ -36,9 +37,12 @@ class ControlObjectScript : public ControlProxy {

protected slots:
// Receives the value from the master control by a unique queued connection
void slotValueChanged(double v, QObject*);
// This is specified virtual, to allow gmock to replace it in the test case
virtual void slotValueChanged(double v, QObject*);
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved

private:
QVector<ScriptConnection> m_scriptConnections;
const RuntimeLoggingCategory m_logger;
CompressingProxy m_proxy;
bool m_skipSuperseded; // This flag is combined for all connections of this Control Object
};
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,16 @@ double ControllerScriptInterfaceLegacy::getDefaultParameter(

QJSValue ControllerScriptInterfaceLegacy::makeConnection(
const QString& group, const QString& name, const QJSValue& callback) {
return ControllerScriptInterfaceLegacy::makeConnectionInternal(group, name, callback, false);
}

QJSValue ControllerScriptInterfaceLegacy::makeCompressedConnection(
const QString& group, const QString& name, const QJSValue& callback) {
return ControllerScriptInterfaceLegacy::makeConnectionInternal(group, name, callback, true);
}

QJSValue ControllerScriptInterfaceLegacy::makeConnectionInternal(
const QString& group, const QString& name, const QJSValue& callback, bool skipSuperseded) {
auto pJsEngine = m_pScriptEngineLegacy->jsEngine();
VERIFY_OR_DEBUG_ASSERT(pJsEngine) {
return QJSValue();
Expand Down Expand Up @@ -245,6 +255,7 @@ QJSValue ControllerScriptInterfaceLegacy::makeConnection(
connection.controllerEngine = m_pScriptEngineLegacy;
connection.callback = callback;
connection.id = QUuid::createUuid();
connection.skipSuperseded = skipSuperseded;

if (coScript->addScriptConnection(connection)) {
return pJsEngine->newQObject(
Expand Down
12 changes: 10 additions & 2 deletions src/controllers/scripting/legacy/controllerscriptinterfacelegacy.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ class ControllerScriptInterfaceLegacy : public QObject {
Q_INVOKABLE void reset(const QString& group, const QString& name);
Q_INVOKABLE double getDefaultValue(const QString& group, const QString& name);
Q_INVOKABLE double getDefaultParameter(const QString& group, const QString& name);
Q_INVOKABLE QJSValue makeConnection(
const QString& group, const QString& name, const QJSValue& callback);
Q_INVOKABLE QJSValue makeConnection(const QString& group,
const QString& name,
const QJSValue& callback);
Q_INVOKABLE QJSValue makeCompressedConnection(const QString& group,
const QString& name,
const QJSValue& callback);
// DEPRECATED: Use makeConnection instead.
Q_INVOKABLE QJSValue connectControl(const QString& group,
const QString& name,
Expand Down Expand Up @@ -66,6 +70,10 @@ class ControllerScriptInterfaceLegacy : public QObject {
virtual void timerEvent(QTimerEvent* event);

private:
QJSValue makeConnectionInternal(const QString& group,
const QString& name,
const QJSValue& callback,
bool skipSuperseded = false);
QHash<ConfigKey, ControlObjectScript*> m_controlCache;
ControlObjectScript* getControlObjectScript(const QString& group, const QString& name);

Expand Down
1 change: 1 addition & 0 deletions src/controllers/scripting/legacy/scriptconnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class ScriptConnection {
QJSValue callback;
ControllerScriptInterfaceLegacy* engineJSProxy;
ControllerScriptEngineLegacy* controllerEngine;
bool skipSuperseded;

void executeCallback(double value) const;

Expand Down
Loading