You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by Daniel Quinlan <qu...@pathname.com> on 2004/03/10 11:22:51 UTC
possible areas for speedups
Scanning 100 ham and 100 ham on a well-trained Bayes DB and with
auto-learning off.
First, some base data:
top 20 routines:
Total Elapsed Time = 18.36313 Seconds
User+System Time = 17.22313 Seconds
9.74 1.677 8.510 804 0.0021 0.0106 ::PerMsgStatus::run_eval_tests
5.63 0.970 1.449 82037 0.0000 0.0000 ::PerMsgStatus::get
4.01 0.690 0.671 37988 0.0000 0.0000 DB_File::FETCH
3.65 0.629 1.071 5134 0.0001 0.0002 ::Bayes::tokenize_line
2.85 0.491 4.789 201 0.0024 0.0238 ::Bayes::scan
2.85 0.490 1.596 29737 0.0000 0.0001 ::Bayes::compute_prob_for_token
2.50 0.430 0.466 35653 0.0000 0.0000 ::Message::Node::header
2.35 0.404 1.789 201 0.0020 0.0089 ::PerMsgStatus::_head_tests
2.32 0.400 0.676 35919 0.0000 0.0000 ::Message::Node::get_header
2.32 0.400 0.935 29737 0.0000 0.0000 ::BayesStoreDBM::tok_unpack
2.28 0.392 4.351 201 0.0020 0.0216 ::PerMsgStatus::_body_tests
1.80 0.310 0.458 47786 0.0000 0.0000 ::BayesStoreDBM::is_magic_token
1.57 0.270 0.226 87237 0.0000 0.0000 UNIVERSAL::can
1.34 0.230 1.136 29737 0.0000 0.0000 ::BayesStoreDBM::tok_get
1.28 0.220 0.196 47786 0.0000 0.0000 ::BayesStoreDBM::get_magic_re
1.22 0.210 1.522 201 0.0010 0.0076 ::Bayes::tokenize
1.05 0.180 0.175 10050 0.0000 0.0000 ::PerMsgStatus::domain_ratio
1.05 0.180 0.320 402 0.0004 0.0008 ::PerMsgStatus::get_uri_list
0.98 0.169 0.209 2 0.0846 0.1043 ::Conf::_parse
0.87 0.150 0.159 201 0.0007 0.0008 ::PerMsgStatus::_meta_tests
Now, some hand-waving about things someone could look at:
1. It seems like we spend a fair bit time figuring out whether tokens
are magic tokens:
%Time ExclSec CumulS #Calls sec/call Csec/c Name
1.80 0.310 0.458 47786 0.0000 0.0000 ::BayesStoreDBM::is_magic_token
1.28 0.220 0.196 47786 0.0000 0.0000 ::BayesStoreDBM::get_magic_re
2. header, get_header, get all use a lot of time when added together:
%Time ExclSec CumulS #Calls sec/call Csec/c Name
5.63 0.970 1.449 82037 0.0000 0.0000 ::PerMsgStatus::get
2.50 0.430 0.466 35653 0.0000 0.0000 ::Message::Node::header
2.32 0.400 0.676 35919 0.0000 0.0000 ::Message::Node::get_header
3. slow stuff:
%Time ExclSec CumulS #Calls sec/call Csec/c Name
1.05 0.180 0.175 10050 0.0000 0.0000 ::PerMsgStatus::domain_ratio
1.05 0.180 0.320 402 0.0004 0.0008 ::PerMsgStatus::get_uri_list
Safe to ignore domain_ratio since it will only be run once per
message instead of a bazillion times soon enough, but get_uri_list is
being run twice per message instead of once and might be made
faster. The URI list should be part of the message metadata if it
isn't already.
4. something interesting seen in one run:
%Time ExclSec CumulS #Calls sec/call Csec/c Name
22.0 4.418 7.496 1 4.4179 7.4957 ::BayesStoreDBM::calculate_expire_delta
--
Daniel Quinlan anti-spam (SpamAssassin), Linux,
http://www.pathname.com/~quinlan/ and open source consulting
Re: possible areas for speedups
Posted by Theo Van Dinter <fe...@kluge.net>.
On Wed, Mar 10, 2004 at 03:33:05PM -0800, Dan Quinlan wrote:
> > This is very likely due to backward compatibility (ie: we read v0 and
> > v1 DBs).
>
> Could we store the magic_re in some variable to avoid calling that
> function 47786 times for 200 messages? :-)
Yeah, or we can just hard code it if we remove backward compat in check()
and dump(). That's actually my preferred route at this point.
If we'd like to vote on it, I can make the changes pretty quickly.
> Should that be happening so soon on a Bayes DB that is in regular use?
> It's my autolearning+train-on-error Bayes DB I've been using for months.
It'll kick in anytime something is determined to be "odd" in the DB,
which can be figured out from the -D output.
--
Randomly Generated Tagline:
Step aside, everyone! Sensitive love letters are my specialty. Dear
Baby, Welcome to Dumpsville. Population: you.
-- Homer Simpson
Bart the Lover
Re: possible areas for speedups
Posted by Daniel Quinlan <qu...@pathname.com>.
Theo Van Dinter <fe...@kluge.net> writes:
>> 1.80 0.310 0.458 47786 0.0000 0.0000 ::BayesStoreDBM::is_magic_token
>> 1.28 0.220 0.196 47786 0.0000 0.0000 ::BayesStoreDBM::get_magic_re
> This is very likely due to backward compatibility (ie: we read v0 and
> v1 DBs).
Could we store the magic_re in some variable to avoid calling that
function 47786 times for 200 messages? :-)
>> 4. something interesting seen in one run:
>> %Time ExclSec CumulS #Calls sec/call Csec/c Name
>> 22.0 4.418 7.496 1 4.4179 7.4957 ::BayesStoreDBM::calculate_expire_delta
> not surprising. that's expiry pass 1. that's why we do estimation when
> possible to skip it.
Should that be happening so soon on a Bayes DB that is in regular use?
It's my autolearning+train-on-error Bayes DB I've been using for months.
Daniel
--
Daniel Quinlan anti-spam (SpamAssassin), Linux,
http://www.pathname.com/~quinlan/ and open source consulting
Re: possible areas for speedups
Posted by Theo Van Dinter <fe...@kluge.net>.
On Wed, Mar 10, 2004 at 11:23:22AM -0800, Justin Mason wrote:
> How's about a config setting for "support upgrading from v0/v1 dbs",
> defaulting to off?
Unnecessary since we're not talking about upgrading. :)
> What I'd hate is to have to say to someone: "you need to install
> SpamAssassin 2.63, run a command, then install SpamAssassin 3.0.0
> over that and run another command". I've run into products
> that do that, before, and it's a world of pain for users.
No no no... Upgrading would still work, but using the old DB for check()
and dump and such won't (it currently does) -- our upgrade process is
"upgrade on first write", which is why we recommend people do "sa-learn
--rebuild" after they upgrade (and optionally do the --import if they
have to...)
--
Randomly Generated Tagline:
"If you're running the latest version of IIS, then you're not vulnerable to
this [security hole], but you're vulnerable to something new." - Phil Cox
Re: possible areas for speedups
Posted by Theo Van Dinter <fe...@kluge.net>.
On Wed, Mar 10, 2004 at 01:31:18PM -0600, Michael Parker wrote:
> I'll agree on this. How about a script in tools that can do the
> upgrade? Then we get the code out of BayesStore* and still provide
> the ability to go from 2.5x to 3.0.0.
Unnecessary. See my last message in the thread.
--
Randomly Generated Tagline:
"It's beyond my comprehension why anyone would attach a keyboard to a
production Sun box in the first place :-)" - Michael Wei
Re: possible areas for speedups
Posted by Michael Parker <pa...@pobox.com>.
On Wed, Mar 10, 2004 at 11:23:22AM -0800, Justin Mason wrote:
> >Ok, I'll ask. Anyone for removing backward compatibility for v0/v1 DBs?
>
> How's about a config setting for "support upgrading from v0/v1 dbs",
> defaulting to off?
That defeats the purpose of removing support, getting rid of the
db_version check and all of the code associated with that.
> What I'd hate is to have to say to someone: "you need to install
> SpamAssassin 2.63, run a command, then install SpamAssassin 3.0.0
> over that and run another command". I've run into products
> that do that, before, and it's a world of pain for users.
I'll agree on this. How about a script in tools that can do the
upgrade? Then we get the code out of BayesStore* and still provide
the ability to go from 2.5x to 3.0.0.
Michael
Re: possible areas for speedups
Posted by Michael Parker <pa...@pobox.com>.
On Wed, Mar 10, 2004 at 03:34:50PM -0800, Daniel Quinlan wrote:
> Michael Parker <pa...@pobox.com> writes:
>
> > Ok, I'll ask. Anyone for removing backward compatibility for v0/v1 DBs?
>
> Is that really going to speed us up in the long run? I mean, we'll have
> a new DB version at some point in the future...
>
It may not give us that much of a speed boost, will just remove some
of the old kruft. More of a nice little cleanup than optimization.
Michael
Re: possible areas for speedups
Posted by Theo Van Dinter <fe...@kluge.net>.
On Wed, Mar 10, 2004 at 03:34:50PM -0800, Dan Quinlan wrote:
> Is that really going to speed us up in the long run? I mean, we'll have
> a new DB version at some point in the future...
Well, yeah. We don't have to call get_magic_re 47k times. In fact,
get_magic_re() can go away. There's a bunch of other code that can go
away too.
--
Randomly Generated Tagline:
"There are two kinds of security: the one that will keep your sister out,
and the one that will keep the Government out." - Bruce Schneier
Re: possible areas for speedups
Posted by Daniel Quinlan <qu...@pathname.com>.
Michael Parker <pa...@pobox.com> writes:
> Ok, I'll ask. Anyone for removing backward compatibility for v0/v1 DBs?
Is that really going to speed us up in the long run? I mean, we'll have
a new DB version at some point in the future...
Daniel
--
Daniel Quinlan anti-spam (SpamAssassin), Linux,
http://www.pathname.com/~quinlan/ and open source consulting
Re: possible areas for speedups
Posted by Theo Van Dinter <fe...@kluge.net>.
On Wed, Mar 10, 2004 at 12:32:13PM -0600, Michael Parker wrote:
> Ok, I'll ask. Anyone for removing backward compatibility for v0/v1 DBs?
Actually for 3.0 that's probably not a bad idea. There are likely not
going to be a lot of people going 2.5x->3.x or (for v1) pre-2.6x->3.x,
but rather 2.6x->3.x which is all v2 anyway.
--
Randomly Generated Tagline:
"It's always darkest before dawn. So if you're going to steal your
neighbour's newspaper, that's the time to do it." - Zen Musings
Re: possible areas for speedups
Posted by Michael Parker <pa...@pobox.com>.
On Wed, Mar 10, 2004 at 01:26:05PM -0500, Theo Van Dinter wrote:
> On Wed, Mar 10, 2004 at 02:22:51AM -0800, Dan Quinlan wrote:
> > 1. It seems like we spend a fair bit time figuring out whether tokens
> > are magic tokens:
> >
> > %Time ExclSec CumulS #Calls sec/call Csec/c Name
> > 1.80 0.310 0.458 47786 0.0000 0.0000 ::BayesStoreDBM::is_magic_token
> > 1.28 0.220 0.196 47786 0.0000 0.0000 ::BayesStoreDBM::get_magic_re
>
> This is very likely due to backward compatibility (ie: we read v0 and v1 DBs).
>
Ok, I'll ask. Anyone for removing backward compatibility for v0/v1 DBs?
> > 4. something interesting seen in one run:
> > %Time ExclSec CumulS #Calls sec/call Csec/c Name
> > 22.0 4.418 7.496 1 4.4179 7.4957 ::BayesStoreDBM::calculate_expire_delta
>
> not surprising. that's expiry pass 1. that's why we do estimation when
> possible to skip it.
You could set bayes_auto_expire to 0 for your next test. I actually
might have found one small speed up in the expiry_due code that could
help a little.
Michael
Re: possible areas for speedups
Posted by Theo Van Dinter <fe...@kluge.net>.
On Wed, Mar 10, 2004 at 10:15:18PM -0500, Theo Van Dinter wrote:
> I'm seeing if I can't get this together right now.
Ok, it's pretty much fixed. The fix (caching) requires a defined()
check, which I don't really like, but it should overall be faster than
the constant/multiple subroutine calls. r9346
--
Randomly Generated Tagline:
Just don't make the '9' format pack/unpack numbers... :-)
-- Larry Wall in <19...@wall.org>
Re: possible areas for speedups
Posted by Theo Van Dinter <fe...@kluge.net>.
On Wed, Mar 10, 2004 at 01:26:05PM -0500, Theo Van Dinter wrote:
> > 1.80 0.310 0.458 47786 0.0000 0.0000 ::BayesStoreDBM::is_magic_token
> > 1.28 0.220 0.196 47786 0.0000 0.0000 ::BayesStoreDBM::get_magic_re
>
> This is very likely due to backward compatibility (ie: we read v0 and v1 DBs).
Well... Doing a little digging ...
is_magic_token() was added in when the BayesStoreSQL stuff occured.
it's what calls get_magic_re() all that much. Pretty much all of the
rest of BayesStore does the proper hard code if necessary or cache
get_magic_re() before a loop ...
The only place it's used is in tokenize_line():
next if ( $self->{store}->is_magic_token($token) ); # skip false magic tokens
So this is fixable...
I'm seeing if I can't get this together right now.
--
Randomly Generated Tagline:
ines
Re: possible areas for speedups
Posted by Theo Van Dinter <fe...@kluge.net>.
On Wed, Mar 10, 2004 at 02:22:51AM -0800, Dan Quinlan wrote:
> 1. It seems like we spend a fair bit time figuring out whether tokens
> are magic tokens:
>
> %Time ExclSec CumulS #Calls sec/call Csec/c Name
> 1.80 0.310 0.458 47786 0.0000 0.0000 ::BayesStoreDBM::is_magic_token
> 1.28 0.220 0.196 47786 0.0000 0.0000 ::BayesStoreDBM::get_magic_re
This is very likely due to backward compatibility (ie: we read v0 and v1 DBs).
> 4. something interesting seen in one run:
> %Time ExclSec CumulS #Calls sec/call Csec/c Name
> 22.0 4.418 7.496 1 4.4179 7.4957 ::BayesStoreDBM::calculate_expire_delta
not surprising. that's expiry pass 1. that's why we do estimation when
possible to skip it.
--
Randomly Generated Tagline:
"Here look after this penguin for me, oh by the way it bites" - Alan Cox