Mailing List Archive: Speeding up spool_in.c a bit

Speeding up spool_in.c a bit

masta at perlgolf

Sep 25, 2006, 4:08 AM

Post #1 of 7 (2022 views)

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. :)

Re: Speeding up spool_in.c a bit [ In reply to ]

Sep 25, 2006, 4:19 AM

Post #2 of 7 (1988 views)

On Mon, 25 Sep 2006, Nico Erfurth wrote:

> 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.

12-15 what? Microseconds? Does this significantly affect the amount of
processing needed to handle a message. I'm afraid I'm a bit skeptical
that is will be significant, but I am not always good at guessing these
things. (And remember that Exim is I/O limited; cpu usage isn't usually
an issue.)

Aside: what has happened, of course, is that the number of things stored
on the spool has increased a lot since I first wrote that code. If we
are going to optimize it, maybe a binary-chop search on an ordered list
would be faster still?

> 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.

Eh? The difference between Ustrcmp and Ustrncmp has nothing to do with
upper/lower case! It should all be in lower case.

> 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. :)

I suspect I won't get to it till next week, in fact.

Philip

--
Philip Hazel University of Cambridge Computing Service
Get the Exim 4 book: http://www.uit.co.uk/exim-book

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Re: Speeding up spool_in.c a bit [ In reply to ]

johannes at sipsolutions

Sep 25, 2006, 4:24 AM

Post #3 of 7 (1998 views)

On Mon, 2006-09-25 at 13:08 +0200, Nico Erfurth wrote:
> + case 'a':
> + if (Ustrncmp(big_buffer, "-acl ", 5) == 0)

> + else if (Ustrncmp(big_buffer, "-aclc ", 6) == 0 ||
> + Ustrncmp(big_buffer, "-aclm ", 6) == 0)

Wouldn't it make sense to switch those two around saving another string
comparison in the common case that only new variables are present, and
possibly even moving the "-acl " part all the way to the end? As-is,
that'll always be executed but never result in a match (on new-style acl
spool files).

johannes

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Re: Speeding up spool_in.c a bit [ In reply to ]

masta at perlgolf

Sep 25, 2006, 4:30 AM

Post #4 of 7 (2001 views)

Philip Hazel wrote:

>> 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.
>
> 12-15 what? Microseconds? Does this significantly affect the amount of

Number of calls on Ustrncmp (I didn't test it for Ustrcmp, as I didn't
recognize the usage of it first).

> processing needed to handle a message. I'm afraid I'm a bit skeptical
> that is will be significant, but I am not always good at guessing these
> things. (And remember that Exim is I/O limited; cpu usage isn't usually
> an issue.)

With all the features added over the time, CPU becomes an issue more and
more. Especially with spam and virus-scanning. But yes, IO should still
be the bigger problem. I would be interested in a real benchmark here.

> Aside: what has happened, of course, is that the number of things stored
> on the spool has increased a lot since I first wrote that code. If we
> are going to optimize it, maybe a binary-chop search on an ordered list
> would be faster still?

Just a guess, but I don't think it will be much faster, especially
because the options are of different types, so you need some special
treatment anyway.

> Eh? The difference between Ustrcmp and Ustrncmp has nothing to do with
> upper/lower case! It should all be in lower case.

Uuhh, sorry I've got confused here. :) I'm getting a bit rusty in my old
days. Mea maxima culpa.

Nico

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Re: Speeding up spool_in.c a bit [ In reply to ]

masta at perlgolf

Sep 25, 2006, 4:32 AM

Post #5 of 7 (1978 views)

Johannes Berg wrote:
> On Mon, 2006-09-25 at 13:08 +0200, Nico Erfurth wrote:
>> + case 'a':
>> + if (Ustrncmp(big_buffer, "-acl ", 5) == 0)
>
>> + else if (Ustrncmp(big_buffer, "-aclc ", 6) == 0 ||
>> + Ustrncmp(big_buffer, "-aclm ", 6) == 0)
>
> Wouldn't it make sense to switch those two around saving another string
> comparison in the common case that only new variables are present, and
> possibly even moving the "-acl " part all the way to the end? As-is,
> that'll always be executed but never result in a match (on new-style acl
> spool files).

Switching both around would be a good idea. As may be reordering the
compares by "most commonly used".

Nico

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Re: Speeding up spool_in.c a bit [ In reply to ]

Sep 25, 2006, 4:34 AM

Post #6 of 7 (1996 views)

On Mon, 25 Sep 2006, Nico Erfurth wrote:

> > are going to optimize it, maybe a binary-chop search on an ordered list
> > would be faster still?
>
> Just a guess, but I don't think it will be much faster, especially
> because the options are of different types, so you need some special
> treatment anyway.

True, but the way you program that is to use the binary chop to
recognize option number N, and then switch on N. But yes, I agree there
may not be much in it.

--
Philip Hazel University of Cambridge Computing Service
Get the Exim 4 book: http://www.uit.co.uk/exim-book

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

Re: Speeding up spool_in.c a bit [ In reply to ]

Oct 10, 2006, 4:22 AM

Post #7 of 7 (1969 views)

On Mon, 25 Sep 2006, Nico Erfurth wrote:

> 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.

I have now put on this patch, modified just a bit. (I found only one
bug: you had the "if" of an if-else set within an "#ifdef
WITH_CONTENT_SCAN" so it would have broken on a non-content-scanning
compile.) I decided that, since we were optimizing here, it might also
be worth not rescanning the first two characters of each name, so I've
changed the string compares to start at the third character. I also put
everything into the individual switch cases; it seemed untidy without.

This code is now committed, and will be in tonight's snapshot. It passes
all my tests, so it can't be *very* broken. :-)

Philip

--
Philip Hazel, University of Cambridge Computing Service.

--
## List details at http://www.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##