You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by fe...@apache.org on 2004/09/26 03:47:16 UTC

svn commit: rev 47231 - spamassassin/trunk/lib/Mail/SpamAssassin

Author: felicity
Date: Sat Sep 25 18:47:16 2004
New Revision: 47231

Modified:
   spamassassin/trunk/lib/Mail/SpamAssassin/PerMsgStatus.pm
Log:
move check_tick plugin call to bottom of priority loop, clean up some whitespace/comments

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/PerMsgStatus.pm
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/PerMsgStatus.pm	(original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/PerMsgStatus.pm	Sat Sep 25 18:47:16 2004
@@ -158,11 +158,7 @@
     $self->{html} = $self->{msg}->{metadata}->{html};
 
     my $bodytext = $self->get_decoded_body_text_array();
-
     my $fulltext = $self->{msg}->get_pristine();
-
-    # use $bodytext here because $decoded is too stripped
-    # TVD: leave it up to get_uri_list to do the right thing ...
     my @uris = $self->get_uri_list();
 
     foreach my $priority (sort { $a <=> $b } keys %{$self->{conf}->{priorities}}) {
@@ -199,14 +195,15 @@
       $self->do_body_uri_tests($priority, @uris);
       $self->do_body_eval_tests($priority, $decoded);
   
-      # XXX - we may need to call this more often than once through the loop
-      $self->{main}->call_plugins ("check_tick", { permsgstatus => $self });
-
       $self->do_rawbody_tests($priority, $bodytext);
       $self->do_rawbody_eval_tests($priority, $bodytext);
   
       $self->do_full_tests($priority, \$fulltext);
       $self->do_full_eval_tests($priority, \$fulltext);
+
+      # we may need to call this more often than once through the loop, but
+      # it needs to be done at least once, either at the beginning or the end.
+      $self->{main}->call_plugins ("check_tick", { permsgstatus => $self });
     }
 
     # sanity check, it is possible that no rules >= HARVEST_DNSBL_PRIORITY ran so the harvest

Re: svn commit: rev 47231 - spamassassin/trunk/lib/Mail/SpamAssassin

Posted by Theo Van Dinter <fe...@kluge.net>.
On Mon, Sep 27, 2004 at 11:25:12AM -0700, Dan Quinlan wrote:
> Doing early exit via a plugin is not the right way.  The Scan module
> should be subclassed with multiple scanning techniques which define
> their own check() function.  And, yes, that is about both performance
> and flexibility.  I'm nearly certain the check_tick API is not the right
> way.

Yeah, we discussed this over breakfast -- I'm not 100% on using check_tick,
but it makes the most sense given the current code: it allows users to
enable/disable early exit by loading or not loading the plugin, and the sum
change to the core SA code is only a few lines (look for a "early exit?" flag
and break out of the priority loop, skip AWL, etc.)

There are better ways to do it, some of which we discussed, but requires
a lot of redesign of the code, which is fine if we want to do that.

Anyway, I'm planning to write up my general thoughts about early exit, and we
all can come up with a general plan and design specs.  I haven't actively
started down a path, so it's not like there's been any wasted effort on this
yet.  My stuff is mostly random thoughts and scribbles in my notebook right
now.

-- 
Randomly Generated Tagline:
"For a while, all that stood between America and annihilation was a man with
 a drinking problem." - Some program on the Learning Channel

Re: svn commit: rev 47231 - spamassassin/trunk/lib/Mail/SpamAssassin

Posted by Daniel Quinlan <qu...@pathname.com>.
>>> move check_tick plugin call to bottom of priority loop, clean up some
>>> whitespace/comments

>> Why?  What's the effect on performance and otherwise?

Theo Van Dinter <fe...@kluge.net> writes:

> This is related to my wanting to make shortcut (aka early exit) a
> plugin, but ignoring that...  There is no performance change, and it
> lets the plugins be called at the end of the loop, not in the middle
> so the plugin is actually useful for "end of priority loop"
> operations.

Doing early exit via a plugin is not the right way.  The Scan module
should be subclassed with multiple scanning techniques which define
their own check() function.  And, yes, that is about both performance
and flexibility.  I'm nearly certain the check_tick API is not the right
way.
 
> On an aside, not everything in 3.1 is going to be about performance, and we
> don't need to justify every little change (this was a very little change).

There was no reason given for the change and no bug number, I just
wanted to know why the change was made since the log message didn't
say.

Daniel

-- 
Daniel Quinlan                     ApacheCon! 13-17 November (3 SpamAssassin
http://www.pathname.com/~quinlan/  http://www.apachecon.com/  sessions & more)

Re: svn commit: rev 47231 - spamassassin/trunk/lib/Mail/SpamAssassin

Posted by Theo Van Dinter <fe...@kluge.net>.
On Sun, Sep 26, 2004 at 01:43:24AM -0700, Dan Quinlan wrote:
> > move check_tick plugin call to bottom of priority loop, clean up some
> > whitespace/comments
> 
> Why?  What's the effect on performance and otherwise?

This is related to my wanting to make shortcut (aka early exit) a plugin, but
ignoring that...   There is no performance change, and it lets the plugins be
called at the end of the loop, not in the middle so the plugin is actually
useful for "end of priority loop" operations.

On an aside, not everything in 3.1 is going to be about performance, and we
don't need to justify every little change (this was a very little change).

-- 
Randomly Generated Tagline:
"Luge strategy? Lie flat and try not to die." - Tim Steeves

Re: svn commit: rev 47231 - spamassassin/trunk/lib/Mail/SpamAssassin

Posted by Daniel Quinlan <qu...@pathname.com>.
felicity@apache.org writes:

> move check_tick plugin call to bottom of priority loop, clean up some
> whitespace/comments

Why?  What's the effect on performance and otherwise?

Daniel

-- 
Daniel Quinlan                     ApacheCon! 13-17 November (3 SpamAssassin
http://www.pathname.com/~quinlan/  http://www.apachecon.com/  sessions & more)

Re: svn commit: rev 47231 - spamassassin/trunk/lib/Mail/SpamAssassin

Posted by Daniel Quinlan <qu...@pathname.com>.
felicity@apache.org writes:

> move check_tick plugin call to bottom of priority loop, clean up some
> whitespace/comments

Why?  What's the effect on performance and otherwise?

Daniel

-- 
Daniel Quinlan                     ApacheCon! 13-17 November (3 SpamAssassin
http://www.pathname.com/~quinlan/  http://www.apachecon.com/  sessions & more)