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

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 <qatomic.h>
 #include <qstring.h>
+#include <qdbusmessage.h>
 
 struct DBusMessage;
 
@@ -69,7 +70,11 @@ public:
     ~QDBusMessagePrivate();
 
     QList<QVariant> 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);