You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2007/01/10 20:10:40 UTC

[Bug 5291] New: Bayes' use_ignores code is inefficient

http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5291

           Summary: Bayes' use_ignores code is inefficient
           Product: Spamassassin
           Version: SVN Trunk (Latest Devel Version)
          Platform: Other
        OS/Version: other
            Status: NEW
          Severity: normal
          Priority: P5
         Component: Learner
        AssignedTo: dev@spamassassin.apache.org
        ReportedBy: felicity@apache.org


Anytime we have to create a new PMS object just to do a simple function call and
then destroy it is, well, stupid, imo...

Bayes::learn:
  if( $self->{use_ignores} )  # Remove test when PerMsgStatus available.
  { 
    # DMK, koppel@ece.lsu.edu:  Hoping that the ultimate fix to bug 2263 will
    # make it unnecessary to construct a PerMsgStatus here.
    my $PMS = new Mail::SpamAssassin::PerMsgStatus $self->{main}, $msg;
    my $ignore = $self->ignore_message($PMS);
    $PMS->finish();
    return 0 if $ignore;
  } 


I would really like to fix this before 3.2 goes out.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5291] Bayes' use_ignores code is inefficient

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5291


felicity@apache.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|3.2.0                       |3.3.0




------- Additional Comments From felicity@apache.org  2007-01-11 09:17 -------
(In reply to comment #2)
> could a single, long-lived PerMsgStatus be created and kept around for these
checks?
> 
> it's still yucky though...

Yeah, it's yucky.  I was thinking that we could probably move some stuff into
the Bayes plugin (the eval rule portion is plugin-ized), and rearrange some of
the other functions so that this could all be cleaner, but in the end I think
that's going to be hackish as well, though less so than the current code.

So I'm moving this to 3.3 and we'll see about cleaning it up as part of bug 5293.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5291] Bayes' use_ignores code is inefficient

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5291





------- Additional Comments From jm@jmason.org  2007-01-11 06:04 -------
could a single, long-lived PerMsgStatus be created and kept around for these checks?

it's still yucky though...



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5291] Bayes' use_ignores code is inefficient

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5291


felicity@apache.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|Undefined                   |3.2.0






------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

[Bug 5291] Bayes' use_ignores code is inefficient

Posted by bu...@bugzilla.spamassassin.org.
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5291





------- Additional Comments From felicity@apache.org  2007-01-10 13:04 -------
In doing some quick digging, here's what happens in 3.2:

Bayes::learn calls Bayes::ignore_message.  ignore_message generates a new PMS
object and then calls plugins for check_wb_list.  WLBLEval is the the default
plugin that has that function.  It ends up calling check_to_in_list and
check_from_in_list in the plugin, which ends up using:

$pms->{from_in_whitelist} (set in the plugin)
$pms->all_to_addrs()
$pms->all_from_addrs()
$pms->{num_relays_untrusted} (won't this be mis-set due to incorrect pms?)
$pms->{num_relays_trusted} (ditto)
$pms->{relays_untrusted} (ditto)
$pms->{relays_trusted} (ditto)

so 1) it's horribly klugy, 2) may not work anyway due to the new pms object, and
3) is yet another reason why we really ought to break Bayes fully out into a
plugin and not have it part of the main engine code.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.