[exim-dev] Speeding up spool_in.c a bit

Top Page
Delete this message
Reply to this message
Author: Nico Erfurth
Date:  
To: exim-dev
Subject: [exim-dev] Speeding up spool_in.c a bit
Hi list,

as exim is reading the spool-files pretty often I started to browse
through the code of spool_in.c on the hunt for places to optimize it a
bit. I've found the "option-reading" code to be rather "direct" in it's
approach, so I've changed it to check the first letter via a switch/case
statement and reordered the code a bit to fit the scheme. On my system
this brings down the string compares in this section for a usual message
from 80-100 to 12-15. The patch is rather big, but it doesn't change much.

While looking in there, I was wondering why some compares use Ustrncmp,
and others Ustrcmp. Backward-compatibility issues? As I wasn't sure
about the reasons I've added tolower to the switch-statement.

Nico

P.S. As this is a rather sensitive part of the code, please check the
patch before commiting it. I'll be away for the rest of the week, so I
won't be able to fix my errors this time. :)
--- exim-snapshot/src/spool_in.c    2006-09-25 04:15:50.000000000 +0200
+++ exim-snapshot-faster-spool/src/spool_in.c    2006-09-25 12:52:35.000000000 +0200
@@ -370,153 +370,169 @@


/* Now there may be a number of optional lines, each starting with "-".
If you add a new setting here, make sure you set the default above. */
-
for (;;)
{
if (Ufgets(big_buffer, big_buffer_size, f) == NULL) goto SPOOL_READ_ERROR;
if (big_buffer[0] != '-') break;
big_buffer[Ustrlen(big_buffer) - 1] = 0;

+  switch(tolower(big_buffer[1])) {
   /* For long-term backward compatibility, we recognize "-acl", which was used
   before the number of ACL variables changed from 10 to 20. This was before the
   subsequent change to an arbitrary number of named variables. This code is
   retained so that upgrades from very old versions can still handle old-format
   spool files. The value given after "-acl" is a number that is 0-9 for
   connection variables, and 10-19 for message variables. */
+    case 'a':
+      if (Ustrncmp(big_buffer, "-acl ", 5) == 0)
+        {
+        int index, count;
+        uschar name[4];
+        tree_node *node;
+
+        if (sscanf(CS big_buffer + 5, "%d %d", &index, &count) != 2)
+          goto SPOOL_FORMAT_ERROR;
+
+        (void) string_format(name, 4, "%c%d", (index < 10 ? 'c' : 'm'), index);
+        node = acl_var_create(name);
+        node->data.ptr = store_get(count + 1);


-  if (Ustrncmp(big_buffer, "-acl ", 5) == 0)
-    {
-    int index, count;
-    uschar name[4];
-    tree_node *node;
-
-    if (sscanf(CS big_buffer + 5, "%d %d", &index, &count) != 2)
-      goto SPOOL_FORMAT_ERROR;
-
-    (void) string_format(name, 4, "%c%d", (index < 10 ? 'c' : 'm'), index);
-    node = acl_var_create(name);
-    node->data.ptr = store_get(count + 1);
+        if (fread(node->data.ptr, 1, count+1, f) < count) goto SPOOL_READ_ERROR;
+        ((uschar*)node->data.ptr)[count] = 0;
+        }


-    if (fread(node->data.ptr, 1, count+1, f) < count) goto SPOOL_READ_ERROR;
-    ((uschar*)node->data.ptr)[count] = 0;
-    }
+      /* Nowadays we use "-aclc" and "-aclm" for the different types of ACL
+      variable, because Exim allows any number of them, with arbitrary names.
+      The line in the spool file is "-acl[cm] <name> <length>". The name excludes
+      the c or m. */


-  /* Nowadays we use "-aclc" and "-aclm" for the different types of ACL
-  variable, because Exim allows any number of them, with arbitrary names.
-  The line in the spool file is "-acl[cm] <name> <length>". The name excludes
-  the c or m. */
+      else if (Ustrncmp(big_buffer, "-aclc ", 6) == 0 ||
+               Ustrncmp(big_buffer, "-aclm ", 6) == 0)
+        {
+        uschar *name, *endptr;
+        int count;
+        tree_node *node;
+
+        endptr = Ustrchr(big_buffer + 6, ' ');
+        if (endptr == NULL) goto SPOOL_FORMAT_ERROR;
+        name = string_sprintf("%c%.*s", big_buffer[4], endptr - big_buffer - 6,
+          big_buffer + 6);
+        if (sscanf(CS endptr, " %d", &count) != 1) goto SPOOL_FORMAT_ERROR;
+
+        node = acl_var_create(name);
+        node->data.ptr = store_get(count + 1);
+        if (fread(node->data.ptr, 1, count+1, f) < count) goto SPOOL_READ_ERROR;
+        ((uschar*)node->data.ptr)[count] = 0;
+        }
+      else if (Ustrcmp(big_buffer, "-allow_unqualified_recipient") == 0)
+        allow_unqualified_recipient = TRUE;
+      else if (Ustrcmp(big_buffer, "-allow_unqualified_sender") == 0)
+        allow_unqualified_sender = TRUE;
+      else if (Ustrncmp(big_buffer, "-auth_id", 8) == 0)
+        authenticated_id = string_copy(big_buffer + 9);
+      else if (Ustrncmp(big_buffer, "-auth_sender", 12) == 0)
+        authenticated_sender = string_copy(big_buffer + 13);
+      else if (Ustrncmp(big_buffer, "-active_hostname", 16) == 0)
+        smtp_active_hostname = string_copy(big_buffer + 17);
+      break;
+
+    case 'b':
+      if (Ustrncmp(big_buffer, "-body_linecount", 15) == 0)
+        body_linecount = Uatoi(big_buffer + 15);
+      else if (Ustrncmp(big_buffer, "-body_zerocount", 15) == 0)
+        body_zerocount = Uatoi(big_buffer + 15);
+#ifdef EXPERIMENTAL_BRIGHTMAIL
+      else if (Ustrncmp(big_buffer, "-bmi_verdicts ", 14) == 0)
+        bmi_verdicts = string_copy(big_buffer + 14);
+#endif
+      break;


-  else if (Ustrncmp(big_buffer, "-aclc ", 6) == 0 ||
-           Ustrncmp(big_buffer, "-aclm ", 6) == 0)
-    {
-    uschar *name, *endptr;
-    int count;
-    tree_node *node;
-
-    endptr = Ustrchr(big_buffer + 6, ' ');
-    if (endptr == NULL) goto SPOOL_FORMAT_ERROR;
-    name = string_sprintf("%c%.*s", big_buffer[4], endptr - big_buffer - 6,
-      big_buffer + 6);
-    if (sscanf(CS endptr, " %d", &count) != 1) goto SPOOL_FORMAT_ERROR;
-
-    node = acl_var_create(name);
-    node->data.ptr = store_get(count + 1);
-    if (fread(node->data.ptr, 1, count+1, f) < count) goto SPOOL_READ_ERROR;
-    ((uschar*)node->data.ptr)[count] = 0;
-    }
+    case 'h':
+      if (Ustrcmp(big_buffer, "-host_lookup_deferred") == 0)
+        host_lookup_deferred = TRUE;
+      else if (Ustrcmp(big_buffer, "-host_lookup_failed") == 0)
+        host_lookup_failed = TRUE;
+      else if (Ustrncmp(big_buffer, "-host_auth", 10) == 0)
+        sender_host_authenticated = string_copy(big_buffer + 11);
+      else if (Ustrncmp(big_buffer, "-host_name", 10) == 0)
+        sender_host_name = string_copy(big_buffer + 11);
+      else if (Ustrncmp(big_buffer, "-helo_name", 10) == 0)
+        sender_helo_name = string_copy(big_buffer + 11);
+      else if (Ustrncmp(big_buffer, "-host_address", 13) == 0)
+        {
+        sender_host_port = host_address_extract_port(big_buffer + 14);
+        sender_host_address = string_copy(big_buffer + 14);
+        }
+      break;
+      
+    case 'i':
+      if (Ustrncmp(big_buffer, "-interface_address", 18) == 0)
+        {
+        interface_port = host_address_extract_port(big_buffer + 19);
+        interface_address = string_copy(big_buffer + 19);
+        }


-  /* Other values */
+      else if (Ustrncmp(big_buffer, "-ident", 6) == 0)
+        sender_ident = string_copy(big_buffer + 7);
+      break;
+
+    case 'l':
+      if (Ustrcmp(big_buffer, "-local") == 0) sender_local = TRUE;
+      else if (Ustrcmp(big_buffer, "-localerror") == 0)
+        local_error_message = TRUE;
+      else if (Ustrncmp(big_buffer, "-local_scan ", 12) == 0)
+        local_scan_data = string_copy(big_buffer + 12);
+      break;


-  else if (Ustrcmp(big_buffer, "-local") == 0) sender_local = TRUE;
-  else if (Ustrcmp(big_buffer, "-localerror") == 0)
-    local_error_message = TRUE;
-  else if (Ustrncmp(big_buffer, "-local_scan ", 12) == 0)
-    local_scan_data = string_copy(big_buffer + 12);
+    case 's':
 #ifdef WITH_CONTENT_SCAN
-  else if (Ustrncmp(big_buffer, "-spam_score_int ", 16) == 0)
-    spam_score_int = string_copy(big_buffer + 16);
+      if (Ustrncmp(big_buffer, "-spam_score_int ", 16) == 0)
+        spam_score_int = string_copy(big_buffer + 16);
 #endif
-#ifdef EXPERIMENTAL_BRIGHTMAIL
-  else if (Ustrncmp(big_buffer, "-bmi_verdicts ", 14) == 0)
-    bmi_verdicts = string_copy(big_buffer + 14);
+      else if (Ustrncmp(big_buffer, "-sender_set_untrusted", 21) == 0)
+        sender_set_untrusted = TRUE;
+      break;
+#ifdef SUPPORT_TLS
+    case 't':
+      if (Ustrncmp(big_buffer, "-tls_certificate_verified", 25) == 0)
+        tls_certificate_verified = TRUE;
+      else if (Ustrncmp(big_buffer, "-tls_cipher", 11) == 0)
+        tls_cipher = string_copy(big_buffer + 12);
+      else if (Ustrncmp(big_buffer, "-tls_peerdn", 11) == 0)
+        tls_peerdn = string_copy(big_buffer + 12);
+      break;
 #endif
-  else if (Ustrcmp(big_buffer, "-host_lookup_deferred") == 0)
-    host_lookup_deferred = TRUE;
-  else if (Ustrcmp(big_buffer, "-host_lookup_failed") == 0)
-    host_lookup_failed = TRUE;
-  else if (Ustrncmp(big_buffer, "-body_linecount", 15) == 0)
-    body_linecount = Uatoi(big_buffer + 15);
-  else if (Ustrncmp(big_buffer, "-body_zerocount", 15) == 0)
-    body_zerocount = Uatoi(big_buffer + 15);
-  else if (Ustrncmp(big_buffer, "-frozen", 7) == 0)
-    {
-    deliver_freeze = TRUE;
-    deliver_frozen_at = Uatoi(big_buffer + 7);
-    }
-  else if (Ustrcmp(big_buffer, "-allow_unqualified_recipient") == 0)
-    allow_unqualified_recipient = TRUE;
-  else if (Ustrcmp(big_buffer, "-allow_unqualified_sender") == 0)
-    allow_unqualified_sender = TRUE;
-  else if (Ustrcmp(big_buffer, "-deliver_firsttime") == 0)
-    deliver_firsttime = TRUE;
-  else if (Ustrcmp(big_buffer, "-manual_thaw") == 0)
-    deliver_manual_thaw = TRUE;
-  else if (Ustrncmp(big_buffer, "-auth_id", 8) == 0)
-    authenticated_id = string_copy(big_buffer + 9);
-  else if (Ustrncmp(big_buffer, "-auth_sender", 12) == 0)
-    authenticated_sender = string_copy(big_buffer + 13);
-  else if (Ustrncmp(big_buffer, "-sender_set_untrusted", 21) == 0)
-    sender_set_untrusted = TRUE;
-
-  #ifdef SUPPORT_TLS
-  else if (Ustrncmp(big_buffer, "-tls_certificate_verified", 25) == 0)
-    tls_certificate_verified = TRUE;
-  else if (Ustrncmp(big_buffer, "-tls_cipher", 11) == 0)
-    tls_cipher = string_copy(big_buffer + 12);
-  else if (Ustrncmp(big_buffer, "-tls_peerdn", 11) == 0)
-    tls_peerdn = string_copy(big_buffer + 12);
-  #endif
-
-  /* We now record the port number after the address, separated by a
-  dot. For compatibility during upgrading, do nothing if there
-  isn't a value (it gets left at zero). */
+    default:
+      if (Ustrncmp(big_buffer, "-frozen", 7) == 0)
+        {
+        deliver_freeze = TRUE;
+        deliver_frozen_at = Uatoi(big_buffer + 7);
+        }
+      else if (Ustrcmp(big_buffer, "-deliver_firsttime") == 0)
+        deliver_firsttime = TRUE;
+      else if (Ustrcmp(big_buffer, "-manual_thaw") == 0)
+        deliver_manual_thaw = TRUE;


-  else if (Ustrncmp(big_buffer, "-host_address", 13) == 0)
-    {
-    sender_host_port = host_address_extract_port(big_buffer + 14);
-    sender_host_address = string_copy(big_buffer + 14);
-    }


-  else if (Ustrncmp(big_buffer, "-interface_address", 18) == 0)
-    {
-    interface_port = host_address_extract_port(big_buffer + 19);
-    interface_address = string_copy(big_buffer + 19);
-    }
+      /* We now record the port number after the address, separated by a
+      dot. For compatibility during upgrading, do nothing if there
+      isn't a value (it gets left at zero). */


-  else if (Ustrncmp(big_buffer, "-active_hostname", 16) == 0)
-    smtp_active_hostname = string_copy(big_buffer + 17);
-  else if (Ustrncmp(big_buffer, "-host_auth", 10) == 0)
-    sender_host_authenticated = string_copy(big_buffer + 11);
-  else if (Ustrncmp(big_buffer, "-host_name", 10) == 0)
-    sender_host_name = string_copy(big_buffer + 11);
-  else if (Ustrncmp(big_buffer, "-helo_name", 10) == 0)
-    sender_helo_name = string_copy(big_buffer + 11);
-  else if (Ustrncmp(big_buffer, "-ident", 6) == 0)
-    sender_ident = string_copy(big_buffer + 7);
-  else if (Ustrncmp(big_buffer, "-received_protocol", 18) == 0)
-    received_protocol = string_copy(big_buffer + 19);
-  else if (Ustrncmp(big_buffer, "-N", 2) == 0)
-    dont_deliver = TRUE;
-
-  /* To allow new versions of Exim that add additional flags to interwork
-  with older versions that do not understand them, just ignore any flagged
-  lines that we don't recognize. Otherwise it wouldn't be possible to back
-  off a new version that left new-style flags written on the spool. That's
-  why the following line is commented out. */


-    /* else goto SPOOL_FORMAT_ERROR; */
-  }
+      else if (Ustrncmp(big_buffer, "-received_protocol", 18) == 0)
+        received_protocol = string_copy(big_buffer + 19);
+      else if (Ustrncmp(big_buffer, "-N", 2) == 0)
+        dont_deliver = TRUE;


+      /* To allow new versions of Exim that add additional flags to interwork
+      with older versions that do not understand them, just ignore any flagged
+      lines that we don't recognize. Otherwise it wouldn't be possible to back
+      off a new version that left new-style flags written on the spool. That's
+      why the following line is commented out. */
+
+        /* else goto SPOOL_FORMAT_ERROR; */
+    }
+  }
 /* Build sender_fullhost if required */


#ifndef COMPILE_UTILITY