From e7ca74ed8a41eb4c05f436007417d160b6cf94f7 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 30 Jun 2009 11:13:47 +0200 Subject: Avoid revalidating message parameters. This is a small performance improvement when making a call: we don't need to validate what we already know to be valid because we either designed it to be so or because we've already validated. The D-Bus library unfortunately validates again and there's nothing we can do about it. But we can avoid doing it twice in our own code. Reviewed-By: Marius Bugge Monsen --- src/dbus/qdbusabstractinterface.cpp | 6 ++++ src/dbus/qdbusintegrator.cpp | 3 ++ src/dbus/qdbusinternalfilters.cpp | 2 +- src/dbus/qdbusmessage.cpp | 60 +++++++++++++++++++++---------------- src/dbus/qdbusmessage.h | 6 ++-- src/dbus/qdbusmessage_p.h | 9 ++++++ 6 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/dbus/qdbusabstractinterface.cpp b/src/dbus/qdbusabstractinterface.cpp index a4097c6..08da997 100644 --- a/src/dbus/qdbusabstractinterface.cpp +++ b/src/dbus/qdbusabstractinterface.cpp @@ -44,6 +44,7 @@ #include "qdbusargument.h" #include "qdbuspendingcall.h" +#include "qdbusmessage_p.h" #include "qdbusmetaobject_p.h" #include "qdbusmetatype_p.h" #include "qdbusutil_p.h" @@ -139,6 +140,7 @@ QVariant QDBusAbstractInterfacePrivate::property(const QMetaProperty &mp) const QDBusMessage msg = QDBusMessage::createMethodCall(service, path, QLatin1String(DBUS_INTERFACE_PROPERTIES), QLatin1String("Get")); + QDBusMessagePrivate::setParametersValidated(msg, true); msg << interface << QString::fromUtf8(mp.name()); QDBusMessage reply = connection.call(msg, QDBus::Block); @@ -202,6 +204,7 @@ void QDBusAbstractInterfacePrivate::setProperty(const QMetaProperty &mp, const Q QDBusMessage msg = QDBusMessage::createMethodCall(service, path, QLatin1String(DBUS_INTERFACE_PROPERTIES), QLatin1String("Set")); + QDBusMessagePrivate::setParametersValidated(msg, true); msg << interface << QString::fromUtf8(mp.name()) << qVariantFromValue(QDBusVariant(value)); QDBusMessage reply = connection.call(msg, QDBus::Block); @@ -386,6 +389,7 @@ QDBusMessage QDBusAbstractInterface::callWithArgumentList(QDBus::CallMode mode, // qDebug() << "QDBusAbstractInterface" << "Service" << service() << "Path:" << path(); QDBusMessage msg = QDBusMessage::createMethodCall(service(), path(), interface(), m); + QDBusMessagePrivate::setParametersValidated(msg, true); msg.setArguments(args); QDBusMessage reply = d->connection.call(msg, mode); @@ -418,6 +422,7 @@ QDBusPendingCall QDBusAbstractInterface::asyncCallWithArgumentList(const QString return QDBusPendingCall::fromError(d->lastError); QDBusMessage msg = QDBusMessage::createMethodCall(service(), path(), interface(), method); + QDBusMessagePrivate::setParametersValidated(msg, true); msg.setArguments(args); return d->connection.asyncCall(msg); } @@ -458,6 +463,7 @@ bool QDBusAbstractInterface::callWithCallback(const QString &method, path(), interface(), method); + QDBusMessagePrivate::setParametersValidated(msg, true); msg.setArguments(args); d->lastError = 0; diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp index 704d2e3..a0903ed 100644 --- a/src/dbus/qdbusintegrator.cpp +++ b/src/dbus/qdbusintegrator.cpp @@ -1143,6 +1143,7 @@ void QDBusConnectionPrivate::relaySignal(QObject *obj, const QMetaObject *mo, in QDBusReadLocker locker(RelaySignalAction, this); QDBusMessage message = QDBusMessage::createSignal(QLatin1String("/"), interface, QLatin1String(memberName)); + QDBusMessagePrivate::setParametersValidated(message, true); message.setArguments(args); QDBusError error; DBusMessage *msg = QDBusMessagePrivate::toDBusMessage(message, &error); @@ -2092,6 +2093,7 @@ QString QDBusConnectionPrivate::getNameOwner(const QString& serviceName) QDBusMessage msg = QDBusMessage::createMethodCall(QLatin1String(DBUS_SERVICE_DBUS), QLatin1String(DBUS_PATH_DBUS), QLatin1String(DBUS_INTERFACE_DBUS), QLatin1String("GetNameOwner")); + QDBusMessagePrivate::setParametersValidated(msg, true); msg << serviceName; QDBusMessage reply = sendWithReply(msg, QDBus::Block); if (reply.type() == QDBusMessage::ReplyMessage) @@ -2115,6 +2117,7 @@ QDBusConnectionPrivate::findMetaObject(const QString &service, const QString &pa QDBusMessage msg = QDBusMessage::createMethodCall(service, path, QLatin1String(DBUS_INTERFACE_INTROSPECTABLE), QLatin1String("Introspect")); + QDBusMessagePrivate::setParametersValidated(msg, true); QDBusMessage reply = sendWithReply(msg, QDBus::Block); diff --git a/src/dbus/qdbusinternalfilters.cpp b/src/dbus/qdbusinternalfilters.cpp index 416144d..762a49b 100644 --- a/src/dbus/qdbusinternalfilters.cpp +++ b/src/dbus/qdbusinternalfilters.cpp @@ -179,7 +179,7 @@ QString qDBusIntrospectObject(const QDBusConnectionPrivate::ObjectTreeNode &node static QDBusMessage qDBusPropertyError(const QDBusMessage &msg, const QString &interface_name) { - return msg.createErrorReply(QLatin1String(DBUS_ERROR_INVALID_ARGS), + return msg.createErrorReply(QDBusError::InvalidArgs, QString::fromLatin1("Interface %1 was not found in object %2") .arg(interface_name) .arg(msg.path())); diff --git a/src/dbus/qdbusmessage.cpp b/src/dbus/qdbusmessage.cpp index eb09de9..78de6d9 100644 --- a/src/dbus/qdbusmessage.cpp +++ b/src/dbus/qdbusmessage.cpp @@ -62,7 +62,8 @@ static inline const char *data(const QByteArray &arr) QDBusMessagePrivate::QDBusMessagePrivate() : msg(0), reply(0), type(DBUS_MESSAGE_TYPE_INVALID), - timeout(-1), localReply(0), ref(1), delayedReply(false), localMessage(false) + timeout(-1), localReply(0), ref(1), delayedReply(false), localMessage(false), + parametersValidated(false) { } @@ -115,14 +116,16 @@ DBusMessage *QDBusMessagePrivate::toDBusMessage(const QDBusMessage &message, QDB break; case DBUS_MESSAGE_TYPE_METHOD_CALL: // only service and interface can be empty -> path and name must not be empty - if (!QDBusUtil::checkBusName(d_ptr->service, QDBusUtil::EmptyAllowed, error)) - return 0; - if (!QDBusUtil::checkObjectPath(d_ptr->path, QDBusUtil::EmptyNotAllowed, error)) - return 0; - if (!QDBusUtil::checkInterfaceName(d_ptr->interface, QDBusUtil::EmptyAllowed, error)) - return 0; - if (!QDBusUtil::checkMemberName(d_ptr->name, QDBusUtil::EmptyNotAllowed, error, "method")) - return 0; + if (!d_ptr->parametersValidated) { + if (!QDBusUtil::checkBusName(d_ptr->service, QDBusUtil::EmptyAllowed, error)) + return 0; + if (!QDBusUtil::checkObjectPath(d_ptr->path, QDBusUtil::EmptyNotAllowed, error)) + return 0; + if (!QDBusUtil::checkInterfaceName(d_ptr->interface, QDBusUtil::EmptyAllowed, error)) + return 0; + if (!QDBusUtil::checkMemberName(d_ptr->name, QDBusUtil::EmptyNotAllowed, error, "method")) + return 0; + } msg = q_dbus_message_new_method_call(data(d_ptr->service.toUtf8()), d_ptr->path.toUtf8(), data(d_ptr->interface.toUtf8()), d_ptr->name.toUtf8()); @@ -136,7 +139,8 @@ DBusMessage *QDBusMessagePrivate::toDBusMessage(const QDBusMessage &message, QDB break; case DBUS_MESSAGE_TYPE_ERROR: // error name can't be empty - if (!QDBusUtil::checkErrorName(d_ptr->name, QDBusUtil::EmptyNotAllowed, error)) + if (!d_ptr->parametersValidated + && !QDBusUtil::checkErrorName(d_ptr->name, QDBusUtil::EmptyNotAllowed, error)) return 0; msg = q_dbus_message_new(DBUS_MESSAGE_TYPE_ERROR); @@ -148,12 +152,14 @@ DBusMessage *QDBusMessagePrivate::toDBusMessage(const QDBusMessage &message, QDB break; case DBUS_MESSAGE_TYPE_SIGNAL: // nothing can be empty here - if (!QDBusUtil::checkObjectPath(d_ptr->path, QDBusUtil::EmptyNotAllowed, error)) - return 0; - if (!QDBusUtil::checkInterfaceName(d_ptr->interface, QDBusUtil::EmptyAllowed, error)) - return 0; - if (!QDBusUtil::checkMemberName(d_ptr->name, QDBusUtil::EmptyNotAllowed, error, "method")) - return 0; + if (!d_ptr->parametersValidated) { + if (!QDBusUtil::checkObjectPath(d_ptr->path, QDBusUtil::EmptyNotAllowed, error)) + return 0; + if (!QDBusUtil::checkInterfaceName(d_ptr->interface, QDBusUtil::EmptyAllowed, error)) + return 0; + if (!QDBusUtil::checkMemberName(d_ptr->name, QDBusUtil::EmptyNotAllowed, error, "method")) + return 0; + } msg = q_dbus_message_new_signal(d_ptr->path.toUtf8(), d_ptr->interface.toUtf8(), d_ptr->name.toUtf8()); @@ -162,16 +168,11 @@ DBusMessage *QDBusMessagePrivate::toDBusMessage(const QDBusMessage &message, QDB Q_ASSERT(false); break; } -#if 0 - DBusError err; - q_dbus_error_init(&err); - if (q_dbus_error_is_set(&err)) { - QDBusError qe(&err); - qDebug() << "QDBusMessagePrivate::toDBusMessage" << qe; - } -#endif - if (!msg) - return 0; + + // if we got here, the parameters validated + // and since the message parameters cannot be changed once the message is created + // we can record this fact + d_ptr->parametersValidated = true; QDBusMarshaller marshaller; QVariantList::ConstIterator it = d_ptr->arguments.constBegin(); @@ -492,6 +493,13 @@ QDBusMessage QDBusMessage::createErrorReply(const QString name, const QString &m Constructs a new DBus reply message for the error type \a type using the message \a msg. Returns the DBus message. */ +QDBusMessage QDBusMessage::createErrorReply(QDBusError::ErrorType atype, const QString &amsg) const +{ + QDBusMessage msg = createErrorReply(QDBusError::errorString(atype), amsg); + msg.d_ptr->parametersValidated = true; + return msg; +} + /*! Constructs an empty, invalid QDBusMessage object. diff --git a/src/dbus/qdbusmessage.h b/src/dbus/qdbusmessage.h index 55f388a..34b1635 100644 --- a/src/dbus/qdbusmessage.h +++ b/src/dbus/qdbusmessage.h @@ -87,8 +87,9 @@ public: QDBusMessage createErrorReply(const QString name, const QString &msg) const; inline QDBusMessage createErrorReply(const QDBusError &err) const { return createErrorReply(err.name(), err.message()); } - inline QDBusMessage createErrorReply(QDBusError::ErrorType type, const QString &msg) const; + QDBusMessage createErrorReply(QDBusError::ErrorType type, const QString &msg) const; + // there are no setters; if this changes, see qdbusmessage_p.h QString service() const; QString path() const; QString interface() const; @@ -113,9 +114,6 @@ private: QDBusMessagePrivate *d_ptr; }; -inline QDBusMessage QDBusMessage::createErrorReply(QDBusError::ErrorType atype, const QString &amsg) const -{ return createErrorReply(QDBusError::errorString(atype), amsg); } - #ifndef QT_NO_DEBUG_STREAM QDBUS_EXPORT QDebug operator<<(QDebug, const QDBusMessage &); #endif diff --git a/src/dbus/qdbusmessage_p.h b/src/dbus/qdbusmessage_p.h index a0a681f..b8f23dc 100644 --- a/src/dbus/qdbusmessage_p.h +++ b/src/dbus/qdbusmessage_p.h @@ -55,6 +55,7 @@ #include #include +#include struct DBusMessage; @@ -69,7 +70,11 @@ public: ~QDBusMessagePrivate(); QList arguments; + + // the following parameters are "const": they are not changed after the constructors + // the parametersValidated member below controls whether they've been validated already QString service, path, interface, name, message, signature; + DBusMessage *msg; DBusMessage *reply; int type; @@ -79,6 +84,10 @@ public: mutable uint delayedReply : 1; uint localMessage : 1; + mutable uint parametersValidated : 1; + + static void setParametersValidated(QDBusMessage &msg, bool enable) + { msg.d_ptr->parametersValidated = enable; } static DBusMessage *toDBusMessage(const QDBusMessage &message, QDBusError *error); static QDBusMessage fromDBusMessage(DBusMessage *dmsg); -- cgit v0.12