Re: Qsmtpd::Address design question

2005-10-11 Thread John Peacock
Brian Grossman wrote:
 Sorry, I don't see it.  The iscom and brianinit were declared as
 fields in the subclass, so should have just extended the array.

Duh, sorry.  I just read through the POD on 'fields' and I see that what I took
to be a hash access is converted internally to an array access.  My mistake...

John


Re: Qsmtpd::Address design question

2005-10-10 Thread John Peacock

Brian Grossman wrote:

Since we're talking 10's of thousands of concurrency here in some cases,
the memory savings for [] vs {} may be worthwhile.


But by that argument, there is no point in using an array at all.  The 
original implementation stored a single value in $rcpt-[0], so there is 
no reason not to use a blessed scalar (not common, but completely 
practical).



The Danga classes in qpsmtpd trunk use this mechanism to avoid blessing
a {}.  There are several examples in qpsmtpd/trunk/lib/Danga/.  Eg.
Danga::Client inherits from Danga::TimeoutSocket.


I've read through your contrived example, and I don't see how this is 
any more efficient.  The fields pseudo-pragma creates a global hash 
(which would not be shared by forked processes) plus it _does_ 
initialize a hash for each object, see below:



  sub init {
  my Brian::Address $self = shift;
  $self-{iscom} = 1 if $self-{domain} =~ m/\.com$/i;
  $self-{brianinit} = 1;
  }


I think this is one of those times when clarity of operation is 
preferred over a possibly premature optimization.


John


Re: Qsmtpd::Address design question

2005-10-10 Thread Brian Grossman
On Mon, 10 Oct 2005 11:32:21 -0400
John Peacock [EMAIL PROTECTED] wrote:

  Since we're talking 10's of thousands of concurrency here in some
  cases, the memory savings for [] vs {} may be worthwhile.
 
 But by that argument, there is no point in using an array at all.  The 
 original implementation stored a single value in $rcpt-[0], so there is 
 no reason not to use a blessed scalar (not common, but completely 
 practical).

Except for the desire to store something in it in private plugins, of course.


 I've read through your contrived example, and I don't see how this is 
 any more efficient.  The fields pseudo-pragma creates a global hash 
 (which would not be shared by forked processes) plus it _does_ 
 initialize a hash for each object, see below:
 
sub init {
my Brian::Address $self = shift;
$self-{iscom} = 1 if $self-{domain} =~ m/\.com$/i;
$self-{brianinit} = 1;
}

Sorry, I don't see it.  The iscom and brianinit were declared as
fields in the subclass, so should have just extended the array.

Forkserver's going to be miserable whether it's array or hash.  I was
thinking in terms of PollServer and Matt's (I think it was Matt's) claim of
concurrency 10,000 in his spamtrap.

Of course, there's so many hashes running around already, surely another
one won't make all that much difference.

 I think this is one of those times when clarity of operation is 
 preferred over a possibly premature optimization.

Okay.  I'll be happy regardless which implementation you choose.  I was
just pointing out another option.

Brian


Re: Qsmtpd::Address design question

2005-10-10 Thread Matt Sergeant

On 10 Oct 2005, at 17:29, Brian Grossman wrote:


Forkserver's going to be miserable whether it's array or hash.  I was
thinking in terms of PollServer and Matt's (I think it was Matt's) 
claim of

concurrency 10,000 in his spamtrap.

Of course, there's so many hashes running around already, surely 
another

one won't make all that much difference.


Indeed. We run about 160M handling a peak of 3000 concurrent 
connections. So just multiply by 4 to get 10k.




Re: Qsmtpd::Address design question

2005-10-10 Thread Peter Eisch
On 10/10/05 4:58 PM, Matt Sergeant [EMAIL PROTECTED] wrote:

 Indeed. We run about 160M handling a peak of 3000 concurrent
 connections. So just multiply by 4 to get 10k.
 

Do you run spamassassin on these systems?  If so, how do you get around the
load that it imposes?

Thanks,

peter



Re: Qsmtpd::Address design question

2005-10-10 Thread Matt Sergeant

On 10 Oct 2005, at 18:48, Peter Eisch wrote:


On 10/10/05 4:58 PM, Matt Sergeant [EMAIL PROTECTED] wrote:


Indeed. We run about 160M handling a peak of 3000 concurrent
connections. So just multiply by 4 to get 10k.


Do you run spamassassin on these systems?  If so, how do you get 
around the

load that it imposes?


No, it's a spamtrap, so we want all the spam we can get :-)

I run only about 4 plugins, and have to work my ass off to get them to 
have as little affect on performance as possible.


Matt.



Re: Qsmtpd::Address design question

2005-10-08 Thread Peter J. Holzer
On 2005-10-07 14:30:32 -0400, Matt Sergeant wrote:
 On 7 Oct 2005, at 14:01, John Peacock wrote:
 I believe you originally wrote Qpsmtpd::Address and I was wondering 
 why you made it an array ref, but never use anything except the first 
 slot.
 It would seem to be better to make it a hash with keys for user, 
 domain/host, and address.  The current interface just parses the 
 zeroth entry any time a portion of the address is requested.
 
 The reason I ask is that while trying to wrap my head around adding an 
 anti-spam appliance (dspam) into my configuration, it became obvious 
 to me that I needed to be able to add a note to the recipient during 
 the rcpt hook (without creating some ghastly hash in notes-() to 
 store it).

So far I have always resorted to the ghastly hash method when I needed
this, but the idea that it would be more natural to attach some
information directly to a Qpsmtpd::Address object occured to me, too.

 I don't think that there should be any visible difference, nor should 
 the hash deference be a significant slow down.  Could you comment on 
 why you did it that way?
 
 Commenting for Peter - I suspect in order to get it working as quickly 
 as possible, he just cut and paste from Mail::Address (which we were 
 using before).

That pretty much sums it up. 

hp

-- 
   _  | Peter J. Holzer| Ich sehe nun ein, dass Computer wenig
|_|_) | Sysadmin WSR   | geeignet sind, um sich was zu merken.
| |   | [EMAIL PROTECTED] |
__/   | http://www.hjp.at/ |-- Holger Lembke in dan-am


pgpNAMw3cLmnz.pgp
Description: PGP signature


Re: Qsmtpd::Address design question

2005-10-08 Thread John Peacock

Matt Sergeant wrote:
new() vs parse() for a start (new if the address is , and parse() if 
it isn't !!!)


Hysterical raisins (that's how Mail::Address did it).  Consider it gone (i.e. 
parse == new).



parse() returning a list or undef.


parse() will return an object (since it is just wrapping new).


format() in general.


I don't know what the problem you have with that.  I just has to jump through 
some hoops to quote non-alpha characters).



We should use overload.


I really don't think the overhead is worth it.  All usage in the core is 
specifically $rcpt-address, so overloading the stringify operator won't buy us 
anything.



user()/host() don't cache their values.


They will now (since I am storing each independently in the hash).

Let me write some POD and then I can commit what I have working now.

John


Re: Qsmtpd::Address design question

2005-10-07 Thread John Peacock

Matt Sergeant wrote:
I've also noticed some ugly warts in Qpsmtpd::Address (when trying to 
document it - it became apparent that it wasn't designed as much as put 
together quickly). It could do with a revisit based on how we want to 
use it.


What was it that bothered you?  Should we convert that fat block of BNF 
into much more readable POD?  We could also use extended regex's and 
document the various bits inline rather than having to go back up to 
figure out the BNF...


John

p.s. I wish someone would write a BNF - Parse::RecDescent converter (or 
some other parser or even Perl extended regex).


Re: Qsmtpd::Address design question

2005-10-07 Thread Matt Sergeant

On 7 Oct 2005, at 15:13, John Peacock wrote:

What was it that bothered you?  Should we convert that fat block of 
BNF into much more readable POD?  We could also use extended regex's 
and document the various bits inline rather than having to go back up 
to figure out the BNF...


new() vs parse() for a start (new if the address is , and parse() if 
it isn't !!!)


parse() returning a list or undef.

format() in general.

We should use overload.

user()/host() don't cache their values.

I think that was it.