gdbus: use larger buffer for larger reads from socket 91/270491/3
authorAdrian Szyndela <adrian.s@samsung.com>
Wed, 2 Feb 2022 13:49:06 +0000 (14:49 +0100)
committerAdrian Szyndela <adrian.s@samsung.com>
Mon, 7 Mar 2022 06:21:58 +0000 (07:21 +0100)
When reading a new message from a socket used for D-Bus,
the first read was 16 bytes to include message size,
and then the exact message size was read into buffer.

Now, data is read into a buffer of default size 4096 bytes.
It gives some chance that the data already includes whole
messages, and some of subsequent polls and reads are not needed anymore.

This speeds up receiving of messages.

Change-Id: I1b7f92b0946a3160a5a3efa59344cf0d53920e06

gio/gdbusprivate.c

index 2bf9a61..63e8f91 100644 (file)
 
 #include "glibintl.h"
 
+#define READ_BUFFER_MIN_READ_SIZE_FOR_HEADER 16
+/* TODO: 4096 is randomly chosen; might want a better chosen default minimum */
+#define READ_BUFFER_MIN_BUFFER_SIZE 4096
+
 static gboolean _g_dbus_worker_do_initial_read (gpointer data);
 static void schedule_pending_close (GDBusWorker *worker);
 
@@ -563,6 +567,36 @@ _g_dbus_worker_unfreeze (GDBusWorker *worker)
 
 static void _g_dbus_worker_do_read_unlocked (GDBusWorker *worker);
 
+static gssize _g_dbus_worker_buffer_get_next_full_message_length (GDBusWorker *worker)
+{
+  GError *error;
+
+  if (worker->read_buffer == NULL)
+    return 0;
+
+  if (worker->read_buffer_cur_size >= READ_BUFFER_MIN_READ_SIZE_FOR_HEADER)
+    {
+      gssize message_len;
+      /* OK, got the header - determine how many more bytes are needed */
+      error = NULL;
+      message_len = g_dbus_message_bytes_needed ((guchar *) worker->read_buffer,
+                                                 READ_BUFFER_MIN_READ_SIZE_FOR_HEADER,
+                                                 &error);
+      if (message_len == -1)
+        {
+          g_warning ("_g_dbus_worker_do_read_cb: error determining bytes needed: %s", error->message);
+          _g_dbus_worker_emit_disconnected (worker, FALSE, error);
+          g_error_free (error);
+          return -1;
+        }
+      if (message_len <= worker->read_buffer_cur_size)
+        return message_len;
+
+      worker->read_buffer_bytes_wanted = message_len;
+    }
+  return 0;
+}
+
 /* called in private thread shared by all GDBusConnection instances (without read-lock held) */
 static void
 _g_dbus_worker_do_read_cb (GInputStream  *input_stream,
@@ -572,6 +606,7 @@ _g_dbus_worker_do_read_cb (GInputStream  *input_stream,
   GDBusWorker *worker = user_data;
   GError *error;
   gssize bytes_read;
+  gssize message_len;
 
   g_mutex_lock (&worker->read_lock);
 
@@ -718,97 +753,82 @@ _g_dbus_worker_do_read_cb (GInputStream  *input_stream,
   read_message_print_transport_debug (bytes_read, worker);
 
   worker->read_buffer_cur_size += bytes_read;
-  if (worker->read_buffer_bytes_wanted == worker->read_buffer_cur_size)
+
+  while ((message_len = _g_dbus_worker_buffer_get_next_full_message_length (worker) ) > 0)
     {
       /* OK, got what we asked for! */
-      if (worker->read_buffer_bytes_wanted == 16)
-        {
-          gssize message_len;
-          /* OK, got the header - determine how many more bytes are needed */
-          error = NULL;
-          message_len = g_dbus_message_bytes_needed ((guchar *) worker->read_buffer,
-                                                     16,
-                                                     &error);
-          if (message_len == -1)
-            {
-              g_warning ("_g_dbus_worker_do_read_cb: error determining bytes needed: %s", error->message);
-              _g_dbus_worker_emit_disconnected (worker, FALSE, error);
-              g_error_free (error);
-              goto out;
-            }
-
-          worker->read_buffer_bytes_wanted = message_len;
-          _g_dbus_worker_do_read_unlocked (worker);
-        }
-      else
-        {
-          GDBusMessage *message;
-          error = NULL;
+      GDBusMessage *message;
+      error = NULL;
 
-          /* TODO: use connection->priv->auth to decode the message */
+      /* TODO: use connection->priv->auth to decode the message */
 
-          message = g_dbus_message_new_from_blob ((guchar *) worker->read_buffer,
-                                                  worker->read_buffer_cur_size,
-                                                  worker->capabilities,
-                                                  &error);
-          if (message == NULL)
-            {
-              gchar *s;
-              s = _g_dbus_hexdump (worker->read_buffer, worker->read_buffer_cur_size, 2);
-              g_warning ("Error decoding D-Bus message of %" G_GSIZE_FORMAT " bytes\n"
-                         "The error is: %s\n"
-                         "The payload is as follows:\n"
-                         "%s",
-                         worker->read_buffer_cur_size,
-                         error->message,
-                         s);
-              g_free (s);
-              _g_dbus_worker_emit_disconnected (worker, FALSE, error);
-              g_error_free (error);
-              goto out;
-            }
+      message = g_dbus_message_new_from_blob ((guchar *) worker->read_buffer,
+                                              message_len,
+                                              worker->capabilities,
+                                              &error);
+      if (message == NULL)
+        {
+          gchar *s;
+          s = _g_dbus_hexdump (worker->read_buffer, message_len, 2);
+          g_warning ("Error decoding D-Bus message of %" G_GSIZE_FORMAT " bytes\n"
+                     "The error is: %s\n"
+                     "The payload is as follows:\n"
+                     "%s",
+                     message_len,
+                     error->message,
+                     s);
+          g_free (s);
+          _g_dbus_worker_emit_disconnected (worker, FALSE, error);
+          g_error_free (error);
+          goto out;
+        }
 
 #ifdef G_OS_UNIX
-          if (worker->read_fd_list != NULL)
-            {
-              g_dbus_message_set_unix_fd_list (message, worker->read_fd_list);
-              g_object_unref (worker->read_fd_list);
-              worker->read_fd_list = NULL;
-            }
+      if (worker->read_fd_list != NULL)
+        {
+          g_dbus_message_set_unix_fd_list (message, worker->read_fd_list);
+          g_object_unref (worker->read_fd_list);
+          worker->read_fd_list = NULL;
+        }
 #endif
 
-          if (G_UNLIKELY (_g_dbus_debug_message ()))
+      if (G_UNLIKELY (_g_dbus_debug_message ()))
+        {
+          gchar *s;
+          _g_dbus_debug_print_lock ();
+          g_print ("========================================================================\n"
+                   "GDBus-debug:Message:\n"
+                   "  <<<< RECEIVED D-Bus message (%" G_GSIZE_FORMAT " bytes)\n",
+                   message_len);
+          s = g_dbus_message_print (message, 2);
+          g_print ("%s", s);
+          g_free (s);
+          if (G_UNLIKELY (_g_dbus_debug_payload ()))
             {
-              gchar *s;
-              _g_dbus_debug_print_lock ();
-              g_print ("========================================================================\n"
-                       "GDBus-debug:Message:\n"
-                       "  <<<< RECEIVED D-Bus message (%" G_GSIZE_FORMAT " bytes)\n",
-                       worker->read_buffer_cur_size);
-              s = g_dbus_message_print (message, 2);
-              g_print ("%s", s);
+              s = _g_dbus_hexdump (worker->read_buffer, message_len, 2);
+              g_print ("%s\n", s);
               g_free (s);
-              if (G_UNLIKELY (_g_dbus_debug_payload ()))
-                {
-                  s = _g_dbus_hexdump (worker->read_buffer, worker->read_buffer_cur_size, 2);
-                  g_print ("%s\n", s);
-                  g_free (s);
-                }
-              _g_dbus_debug_print_unlock ();
             }
+          _g_dbus_debug_print_unlock ();
+        }
 
-          /* yay, got a message, go deliver it */
-          _g_dbus_worker_queue_or_deliver_received_message (worker, g_steal_pointer (&message));
+      /* yay, got a message, go deliver it */
+      _g_dbus_worker_queue_or_deliver_received_message (worker, g_steal_pointer (&message));
 
-          /* start reading another message! */
-          worker->read_buffer_bytes_wanted = 0;
-          worker->read_buffer_cur_size = 0;
-          _g_dbus_worker_do_read_unlocked (worker);
+      /* set up reading another message */
+      if (worker->read_buffer_cur_size > message_len)
+        {
+          memmove (worker->read_buffer,
+                   worker->read_buffer + message_len,
+                   worker->read_buffer_cur_size - message_len);
         }
+      worker->read_buffer_cur_size -= message_len;
+      worker->read_buffer_bytes_wanted = 0;
     }
-  else
+
+  if (message_len == 0)
     {
-      /* didn't get all the bytes we requested - so repeat the request... */
+      /* start reading another message or repeat the request to get all the bytes */
       _g_dbus_worker_do_read_unlocked (worker);
     }
 
@@ -833,15 +853,21 @@ _g_dbus_worker_do_read_unlocked (GDBusWorker *worker)
   /* if bytes_wanted is zero, it means start reading a message */
   if (worker->read_buffer_bytes_wanted == 0)
     {
-      worker->read_buffer_cur_size = 0;
-      worker->read_buffer_bytes_wanted = 16;
+      if (worker->socket != NULL)
+        {
+          worker->read_buffer_bytes_wanted = READ_BUFFER_MIN_BUFFER_SIZE;
+        }
+      else
+        {
+          worker->read_buffer_cur_size = 0;
+          worker->read_buffer_bytes_wanted = READ_BUFFER_MIN_READ_SIZE_FOR_HEADER;
+        }
     }
 
   /* ensure we have a (big enough) buffer */
   if (worker->read_buffer == NULL || worker->read_buffer_bytes_wanted > worker->read_buffer_allocated_size)
     {
-      /* TODO: 4096 is randomly chosen; might want a better chosen default minimum */
-      worker->read_buffer_allocated_size = MAX (worker->read_buffer_bytes_wanted, 4096);
+      worker->read_buffer_allocated_size = MAX (worker->read_buffer_bytes_wanted, READ_BUFFER_MIN_BUFFER_SIZE);
       worker->read_buffer = g_realloc (worker->read_buffer, worker->read_buffer_allocated_size);
     }