[education/rkward] /: Port away from QScriptEngine

Thomas Friedrichsmeier null at kde.org
Fri Apr 15 22:12:57 BST 2022


Git commit 7f5e645152d5d73c993913ad5aab01c3c0583ab0 by Thomas Friedrichsmeier.
Committed on 15/04/2022 at 21:12.
Pushed by tfry into branch 'master'.

Port away from QScriptEngine

M  +1    -13   CMakeLists.txt
M  +1    -1    rkward/plugin/CMakeLists.txt
M  +0    -4    rkward/scriptbackends/CMakeLists.txt
M  +6    -74   rkward/scriptbackends/qtscriptbackend.cpp
M  +5    -26   rkward/scriptbackends/qtscriptbackend.h
M  +1    -1    rkward/scriptbackends/qtscripti18n.cpp
M  +2    -2    rkward/scriptbackends/qtscripti18n.h
M  +9    -26   rkward/scriptbackends/rkcomponentscripting.cpp
M  +2    -2    rkward/scriptbackends/rkcomponentscripting.h

https://invent.kde.org/education/rkward/commit/7f5e645152d5d73c993913ad5aab01c3c0583ab0

diff --git a/CMakeLists.txt b/CMakeLists.txt
index b14f974e..0e130c89 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -18,7 +18,7 @@ INCLUDE(ECMAddAppIcon)
 INCLUDE(ECMMarkNonGuiExecutable)
 INCLUDE(FeatureSummary)
 
-FIND_PACKAGE(Qt5 5.9 CONFIG REQUIRED COMPONENTS Widgets Core Xml Network Script PrintSupport)
+FIND_PACKAGE(Qt5 5.9 CONFIG REQUIRED COMPONENTS Widgets Core Xml Network Qml PrintSupport)
 FIND_PACKAGE(KF5 5.44 REQUIRED COMPONENTS CoreAddons DocTools I18n XmlGui TextEditor WidgetsAddons Parts Config Notifications WindowSystem Archive OPTIONAL_COMPONENTS Crash)
 IF(NOT NO_QT_WEBENGINE)
 	FIND_PACKAGE(Qt5 5.9 OPTIONAL_COMPONENTS WebEngineWidgets)
@@ -39,18 +39,6 @@ IF(FORCE_PRETTY_MAKEFILE)
 	SET(CMAKE_VERBOSE_MAKEFILE OFF)
 ENDIF(FORCE_PRETTY_MAKEFILE)
 
-# TODO: Temporary. Remove once we are sure that using QJSEngine has no negative performance impact on nested plugins, or we have mitigated it.
-#       For now, meant to be used on Mac, where QScriptEngine is crashy.
-IF(APPLE)
-SET(USE_QJSENGINE TRUE)
-ELSE()
-SET(USE_QJSENGINE FALSE)
-ENDIF()
-IF(USE_QJSENGINE)
-	ADD_DEFINITIONS(-DUSE_QJSENGINE)
-	FIND_PACKAGE(Qt5 5.9 CONFIG REQUIRED COMPONENTS Qml)
-ENDIF(USE_QJSENGINE)
-
 ADD_DEFINITIONS(${QT_DEFINITIONS} -DQT_NO_CAST_TO_ASCII)
 ADD_DEFINITIONS(-DQT_USE_FAST_CONCATENATION -DQT_USE_FAST_OPERATOR_PLUS)
 ADD_DEFINITIONS(-DQT_NO_URL_CAST_FROM_STRING)
diff --git a/rkward/plugin/CMakeLists.txt b/rkward/plugin/CMakeLists.txt
index 42db6b94..b70c908d 100644
--- a/rkward/plugin/CMakeLists.txt
+++ b/rkward/plugin/CMakeLists.txt
@@ -32,4 +32,4 @@ SET(plugin_STAT_SRCS
    )
 
 ADD_LIBRARY(plugin STATIC ${plugin_STAT_SRCS})
-TARGET_LINK_LIBRARIES(plugin Qt5::Widgets Qt5::Script KF5::TextEditor KF5::WindowSystem)
+TARGET_LINK_LIBRARIES(plugin Qt5::Widgets KF5::TextEditor KF5::WindowSystem)
diff --git a/rkward/scriptbackends/CMakeLists.txt b/rkward/scriptbackends/CMakeLists.txt
index 308073e0..18b04ebd 100644
--- a/rkward/scriptbackends/CMakeLists.txt
+++ b/rkward/scriptbackends/CMakeLists.txt
@@ -9,10 +9,6 @@ SET(scriptbackends_STAT_SRCS
    )
 
 ADD_LIBRARY(scriptbackends STATIC ${scriptbackends_STAT_SRCS})
-IF(USE_QJSENGINE)
 TARGET_LINK_LIBRARIES(scriptbackends Qt5::Qml Qt5::Widgets Qt5::Xml KF5::WidgetsAddons KF5::I18n)
-ELSE()
-TARGET_LINK_LIBRARIES(scriptbackends Qt5::Script Qt5::Widgets Qt5::Xml KF5::WidgetsAddons KF5::I18n)
-ENDIF()
 
 INSTALL(FILES common.js rkcomponentscripting.js DESTINATION ${DATA_INSTALL_DIR}/rkward/phpfiles )
diff --git a/rkward/scriptbackends/qtscriptbackend.cpp b/rkward/scriptbackends/qtscriptbackend.cpp
index e034aeff..a4c9487e 100644
--- a/rkward/scriptbackends/qtscriptbackend.cpp
+++ b/rkward/scriptbackends/qtscriptbackend.cpp
@@ -249,7 +249,6 @@ QVariant QtScriptBackendThread::getUiLabelPair (const QString &identifier) {
 	return getValue (identifier, RKStandardComponent::UiLabelPair);
 }
 
-#ifdef USE_QJSENGINE
 bool QtScriptBackendThread::scriptError(const QJSValue &val) {
 	RK_TRACE (PHP);
 
@@ -260,20 +259,6 @@ bool QtScriptBackendThread::scriptError(const QJSValue &val) {
 
 	return true;
 }
-#else
-bool QtScriptBackendThread::scriptError(const RKJSValue &val) {
-	RK_TRACE (PHP);
-	Q_UNUSED(val);
-
-	if (!engine.hasUncaughtException ()) return false;
-
-	QString message = i18n ("Script Error: %1\nBacktrace:\n%2", engine.uncaughtException ().toString (), engine.uncaughtExceptionBacktrace ().join ("\n"));
-	engine.clearExceptions ();
-	emit error(message);
-
-	return true;
-}
-#endif
 
 bool QtScriptBackendThread::includeFile (const QString &filename) {
 	RK_TRACE (PHP);
@@ -284,17 +269,13 @@ bool QtScriptBackendThread::includeFile (const QString &filename) {
 		_filename = script_path.toLocalFile ();
 	}
 
-		QFile file (_filename);
-		if (!file.open (QIODevice::ReadOnly | QIODevice::Text)) {
+	QFile file(_filename);
+	if (!file.open (QIODevice::ReadOnly | QIODevice::Text)) {
 		emit error(i18n("The file \"%1\" (needed by \"%2\") could not be found. Please check your installation.", _filename, _scriptfile));
 		return false;
 	}
 
-#ifndef USE_QJSENGINE
-	// evaluate in global context
-	engine.currentContext ()->setActivationObject (engine.globalObject ());
-#endif
-	RKJSValue result = engine.evaluate (QString::fromUtf8 (file.readAll ()), _filename);
+	QJSValue result = engine.evaluate (QString::fromUtf8 (file.readAll ()), _filename);
 	if (scriptError(result)) return false;
 
 	return true;
@@ -303,22 +284,11 @@ bool QtScriptBackendThread::includeFile (const QString &filename) {
 void QtScriptBackendThread::run () {
 	RK_TRACE (PHP);
 
-	RKJSValue backend_object = engine.newQObject (this);
+	QJSValue backend_object = engine.newQObject (this);
 	engine.globalObject ().setProperty ("_RK_backend", backend_object);
 	RKMessageCatalogObject::addI18nToScriptEngine (&engine, catalog);
 
-#ifdef USE_Q_SCRIPT_PROGRAM
-	if (!RKPrecompiledQtScripts::loadCommonScript (&engine, _commonfile)) {
-		if (!engine.hasUncaughtException ()) {
-			emit error (i18n ("Could not open common script file \"%1\"", _commonfile));
-		} else {
-			scriptError (QScriptValue());
-		}
-		return;
-	}
-#else
-	if (!includeFile (_commonfile)) return;
-#endif
+	if (!includeFile (_commonfile)) return;  // TODO: import this as a module and re-use the engine in the next thread?
 	if (!includeFile (_scriptfile)) return;
 
 	emit commandDone("startup complete");
@@ -344,48 +314,10 @@ void QtScriptBackendThread::run () {
 		}
 
 		// do it!
-		RKJSValue result = engine.evaluate (command);
+		QJSValue result = engine.evaluate (command);
 		if (scriptError(result)) return;
 		emit commandDone(result.toString());
 
 		command.clear ();
 	}
 }
-
-#ifdef USE_Q_SCRIPT_PROGRAM
-namespace RKPrecompiledQtScripts {
-	QMap<QString, QScriptProgram> compiled_includes;
-	QMutex compiled_includes_mutex;
-
-	bool loadCommonScript (QScriptEngine* engine, const QString &scriptfile) {
-		RK_TRACE (PHP);
-
-		// NOTE: QScriptProgram cannot be evaluated concurrently in several threads (see https://bugreports.qt-project.org/browse/QTBUG-29246).
-		// Neither would our QMap caching logic work in concurrent threads. Thus the mutex. This clearly has the drawback that QScript evaluation threads
-		// may be waiting for each other during initialization. However, we assume that
-		// - Typically only few such threads are running
-		// - Responsiveness (i.e. UI startup speed), not throughput is the main goal, here.
-		// - The important script engines are those running in the UI thread (and thus necessarily sequentially). As long as these are initialized before
-		//   the other threads, they will clearly profit from pre-compiling
-		QMutexLocker ml (&compiled_includes_mutex);
-		if (!compiled_includes.contains (scriptfile)) {
-			RK_DEBUG (PHP, DL_DEBUG, "Compiling common script file %s", qPrintable (scriptfile));
-			QFile file (scriptfile);
-			if (!file.open (QIODevice::ReadOnly | QIODevice::Text)) {
-				return false;
-			}
-			compiled_includes.insert (scriptfile, QScriptProgram (QString::fromUtf8 (file.readAll ()), scriptfile));
-			file.close ();
-		} else {
-			RK_DEBUG (PHP, DL_DEBUG, "Script file %s is already compiled", qPrintable (scriptfile));
-		}
-		engine->evaluate (compiled_includes[scriptfile]);
-		if (engine->hasUncaughtException ()) {
-			compiled_includes.remove (scriptfile);
-			return false;
-		}
-		return true;
-	}
-}
-#endif
-
diff --git a/rkward/scriptbackends/qtscriptbackend.h b/rkward/scriptbackends/qtscriptbackend.h
index 276dc0f0..880f42b4 100644
--- a/rkward/scriptbackends/qtscriptbackend.h
+++ b/rkward/scriptbackends/qtscriptbackend.h
@@ -54,30 +54,18 @@ private:
 
 #include <QThread>
 #include <QMutex>
-
-//#define USE_QJSENGINE
-#ifdef USE_QJSENGINE
 #include <QtQml/QJSEngine>
-#define RKJSEngine QJSEngine
-#define RKJSValue QJSValue
-#define RKJSValueList QJSValueList
-template<typename T> RKJSValue rkJSMakeArray(RKJSEngine *engine, QVector<T> list) {
+
+template<typename T> QJSValue rkJSMakeArray(QJSEngine *engine, QVector<T> list) {
 	auto ret = engine->newArray(list.size());
 	for(int i = 0; i < list.size(); ++i) ret.setProperty(i, list.at(i));
 	return ret;
 }
-template<typename T> RKJSValue rkJSMakeArray(RKJSEngine *engine, QList<T> list) {
+template<typename T> QJSValue rkJSMakeArray(QJSEngine *engine, QList<T> list) {
 	auto ret = engine->newArray(list.size());
 	for(int i = 0; i < list.size(); ++i) ret.setProperty(i, list.at(i));
 	return ret;
 }
-#else
-#include <QScriptEngine>
-#define RKJSEngine QScriptEngine
-#define RKJSValue QScriptValue
-#define RKJSValueList QScriptValueList
-#define rkJSMakeArray(engine, val) qScriptValueFromSequence(engine, val)
-#endif
 
 class QtScriptBackendThread : public QThread {
 	Q_OBJECT
@@ -104,7 +92,7 @@ protected:
 	void run () override;
 private:
 	/** for any script error in the last evaluation. If there was an error, a message is generated, and this function returns true (and the thread should be made to exit!) */
-	bool scriptError(const RKJSValue &val);
+	bool scriptError(const QJSValue &val);
 	QVariant getValue (const QString &identifier, const int hint);
 
 	QString _command;
@@ -112,7 +100,7 @@ private:
 	QString _commonfile;
 	QString _scriptfile;
 
-	RKJSEngine engine;
+	QJSEngine engine;
 	const RKMessageCatalog *catalog;
 
 	bool killed;
@@ -123,13 +111,4 @@ private:
 	bool sleeping;
 };
 
-#ifndef USE_QJSENGINE
-// TODO: support some level of pre-parsing with QJSEngine
-#define USE_Q_SCRIPT_PROGRAM
-#include <QScriptProgram>
-namespace RKPrecompiledQtScripts {
-	bool loadCommonScript (QScriptEngine *engine, const QString &scriptfile);
-};
-#endif
-
 #endif
diff --git a/rkward/scriptbackends/qtscripti18n.cpp b/rkward/scriptbackends/qtscripti18n.cpp
index f52094c3..18a30c5f 100644
--- a/rkward/scriptbackends/qtscripti18n.cpp
+++ b/rkward/scriptbackends/qtscripti18n.cpp
@@ -9,7 +9,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
 
 #include "../debug.h"
 
-void RKMessageCatalogObject::addI18nToScriptEngine (RKJSEngine* engine, const RKMessageCatalog* catalog) {
+void RKMessageCatalogObject::addI18nToScriptEngine (QJSEngine* engine, const RKMessageCatalog* catalog) {
 	auto handle = engine->newQObject (new RKMessageCatalogObject (catalog, engine));
 	engine->globalObject ().setProperty ("_i18n", handle);
 }
diff --git a/rkward/scriptbackends/qtscripti18n.h b/rkward/scriptbackends/qtscripti18n.h
index b03e246a..92dfe365 100644
--- a/rkward/scriptbackends/qtscripti18n.h
+++ b/rkward/scriptbackends/qtscripti18n.h
@@ -10,7 +10,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
 
 #include <QObject>
 
-#include "qtscriptbackend.h"  // TODO: For RKJSEngine, only. Move back to .cpp-file
+#include "qtscriptbackend.h"  // TODO: For QJSEngine, only. Move back to .cpp-file
 
 #include "../misc/rkmessagecatalog.h"
 
@@ -34,7 +34,7 @@ public:
 		return (catalog->translate(msgctxt, msgid_singular, msgid_plural, count, args));
 	};
 /** Add an RKMessageCatalog, and the required glue code to the given QScriptEngine. */
-	static void addI18nToScriptEngine (RKJSEngine *engine, const RKMessageCatalog *catalog);
+	static void addI18nToScriptEngine (QJSEngine *engine, const RKMessageCatalog *catalog);
 private:
 	const RKMessageCatalog *catalog;
 };
diff --git a/rkward/scriptbackends/rkcomponentscripting.cpp b/rkward/scriptbackends/rkcomponentscripting.cpp
index 0342e71e..25a7537a 100644
--- a/rkward/scriptbackends/rkcomponentscripting.cpp
+++ b/rkward/scriptbackends/rkcomponentscripting.cpp
@@ -27,7 +27,7 @@ RKComponentScriptingProxy::RKComponentScriptingProxy (RKComponent *component) :
 	RK_ASSERT (component);
 	RKComponentScriptingProxy::component = component;
 
-	RKJSValue backend_object = engine.newQObject(this);
+	QJSValue backend_object = engine.newQObject(this);
 	engine.globalObject ().setProperty ("_rkward", backend_object);
 	RKMessageCatalogObject::addI18nToScriptEngine (&engine, component->xmlHelper ()->messageCatalog ());
 }
@@ -62,25 +62,16 @@ void RKComponentScriptingProxy::initialize (const QString& file, const QString&
 	evaluate (_command);
 }
 
-void RKComponentScriptingProxy::handleScriptError(const RKJSValue &val, const QString& current_file) {
+void RKComponentScriptingProxy::handleScriptError(const QJSValue &val, const QString& current_file) {
 	RK_TRACE (PHP);
 
 	QString file = current_file;
 	if (file.isEmpty ()) file = _scriptfile;
-#ifdef USE_QJSENGINE
 	if (val.isError()) {
 		QString message = i18n("Script Error at '%1' line %2: %3\n", file.isEmpty() ? i18n("inlined code") : file, val.property("lineNumber").toInt(), val.toString());
 		KMessageBox::detailedError (0, message, val.property("stack").toString());
 		emit haveError();
 	}
-#else
-	if (engine.hasUncaughtException ()) {
-		QString message = i18n ("Script Error at '%1' line %2: %3\n", file.isEmpty() ? i18n ("inlined code") : file, engine.uncaughtExceptionLineNumber (), engine.uncaughtException ().toString ());
-		KMessageBox::detailedError (0, message, engine.uncaughtExceptionBacktrace ().join ("\n"));
-		engine.clearExceptions ();
-		emit haveError();
-	}
-#endif
 }
 
 void RKComponentScriptingProxy::include (const QString& filename) {
@@ -105,11 +96,7 @@ void RKComponentScriptingProxy::include (const QString& filename) {
 void RKComponentScriptingProxy::evaluate (const QString &code) {
 	RK_TRACE (PHP);
 
-#ifndef USE_QJSENGINE
-	// evaluate in global context
-	engine.currentContext ()->setActivationObject (engine.globalObject ());
-#endif
-	RKJSValue result = engine.evaluate (code, _scriptfile);
+	QJSValue result = engine.evaluate (code, _scriptfile);
 
 	handleScriptError(result);
 }
@@ -157,7 +144,7 @@ QVariant RKComponentScriptingProxy::doRCommand (const QString& command, const QS
 	return (QVariant (com.command->id ()));
 }
 
-static RKJSValue marshall (RKJSEngine *engine, RData *data) {
+static QJSValue marshall (QJSEngine *engine, RData *data) {
 	RK_TRACE (PHP);
 
 	if (data->getDataType() == RData::StringVector) {
@@ -168,7 +155,7 @@ static RKJSValue marshall (RKJSEngine *engine, RData *data) {
 		return (rkJSMakeArray(engine, data->realVector()));
 	} else if (data->getDataType() == RData::StructureVector) {
 		const RData::RDataStorage& rs = data->structureVector();
-		RKJSValue ret = engine->newArray (rs.size ());
+		QJSValue ret = engine->newArray (rs.size ());
 		for (int i = 0; i < rs.size (); ++i) {
 			ret.setProperty (i, marshall (engine, rs[i]));
 		}
@@ -176,7 +163,7 @@ static RKJSValue marshall (RKJSEngine *engine, RData *data) {
 	} else {
 		RK_ASSERT (false);
 	}
-	return RKJSValue ();
+	return QJSValue ();
 }
 
 void RKComponentScriptingProxy::scriptRCommandFinished (RCommand* command) {
@@ -196,15 +183,11 @@ void RKComponentScriptingProxy::scriptRCommandFinished (RCommand* command) {
 	if (command->wasCanceled ()) return;
 	if (command->failed ()) RK_DEBUG (PHP, DL_ERROR, "Plugin script R command %s failed. Full output wsa %s", qPrintable (command->command ()), qPrintable (command->fullOutput ()));
 
-	RKJSValueList args;
+	QJSValueList args;
 	args.append (marshall (&engine, command));
-	args.append (RKJSValue (command->id ()));
-	RKJSValue callback_obj = engine.globalObject ().property (callback);
-#ifdef USE_QJSENGINE
+	args.append (QJSValue (command->id ()));
+	QJSValue callback_obj = engine.globalObject ().property (callback);
 	auto res = callback_obj.call(args);
-#else
-	auto res = callback_obj.call(engine.globalObject(), args);
-#endif
 	handleScriptError(res);
 }
 
diff --git a/rkward/scriptbackends/rkcomponentscripting.h b/rkward/scriptbackends/rkcomponentscripting.h
index 0ebe83a3..d1fcd84e 100644
--- a/rkward/scriptbackends/rkcomponentscripting.h
+++ b/rkward/scriptbackends/rkcomponentscripting.h
@@ -58,7 +58,7 @@ private slots:
 	void scriptRCommandFinished (RCommand* command);
 private:
 	RKComponent* component;
-	RKJSEngine engine;
+	QJSEngine engine;
 	struct OutstandingCommand {
 		RCommand *command;
 		QString callback;
@@ -70,7 +70,7 @@ private:
 	void handleChange (RKComponentBase* changed);
 	QHash<RKComponentBase*, QString> component_commands;
 
-	void handleScriptError (const RKJSValue &val, const QString& current_file=QString ());
+	void handleScriptError (const QJSValue &val, const QString& current_file=QString ());
 };
 
 #endif


More information about the rkward-tracker mailing list