You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ian Holsman <ia...@cnet.com> on 2001/08/24 21:14:17 UTC

[Fwd: brianp patch Quantify results] (was thread-specific free list for pools" patch )

One of our other developers ran Brian Pane's 
thread-specific free list for pools patch (posted ~1 week ago)



here are his results.
..Ian

-----Forwarded Message-----
From: Blaise Tarr <XXXXXXXXXX>
To: pxi-webserver@cnet.com
Subject: brianp patch Quantify results


Hey,

For the baseline I used the CVS version from yesterday (8/3) morning.
Then I applied Brian's "thread-specific free list for pools" patch.

I used these configs:
    StartServers         1 
    MaxClients           1 
    MinSpareThreads      5 
    MaxSpareThreads     10 
    ThreadsPerChild     25 

For the test, I requested 500 news.com pages that have 2 virtual
includes.  The pages were copies of the same file but had different
names.  (lynx -source http://hungry.cnet.com/2file/00${i}.html)

handle_include + descendants were 9.5% faster with Brian's patch, and
accounted for 5.89% of the total time, as opposed to 6.28% of the
total time for the baseline.  Overall, Brian's patch reduced the
number of cycles by 3.74%.

Now, I must add that these are Quantify numbers, not real world
numbers. 
So, what's next?

-- 
Blaise Tarr
XXXXXXXXXXXXXXX         CNET.com
908.541.3771            The source for computers and technology.



Re: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> For the pools code, it can only be patched. It is unacceptable to toss a
> completely written-from-scratch replacement into the code base. If it takes
> a sequence of 20 patches to reach the written-from-scratch stage, then
> fine... but that means each step has been reviewable as you go along.

Nonsense.  If the new code is cleaner and shorter than the existing
code for pools, diffs won't make it any easier to understand.  Our
process cannot be allowed to get in the way of fundamental improvements
to the code base.

....Roy


Re: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> For the pools code, it can only be patched. It is unacceptable to toss a
> completely written-from-scratch replacement into the code base. If it takes
> a sequence of 20 patches to reach the written-from-scratch stage, then
> fine... but that means each step has been reviewable as you go along.

Nonsense.  If the new code is cleaner and shorter than the existing
code for pools, diffs won't make it any easier to understand.  Our
process cannot be allowed to get in the way of fundamental improvements
to the code base.

....Roy


RE: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Sander Striker <st...@apache.org>.
> On Fri, Aug 24, 2001 at 11:09:13PM +0200, Sander Striker wrote:
>>...
>> Mine takes it a bit further.  It essentially makes the freelist more
>> efficient.  Adds the possibility to request a new freelist for a pool,
>> which can optionally have locking.
> 
> Cool.
> 
>> Furthermore it resolves some
>> issues with possible segfaults in the current pools code when running
>> out of memory.
> 
> Not necessary :-)

Well, for the apr_pvsprintf routine I disagree.  When out of mem, this
routine will segfault regardless of having registered an abort function
with the pool or not.  In the modified situation, the pools abort
function is called when out of mem and NULL is returned.  I know you
consider the latter unnecessary, but I try to think of other applications
using APR and thus pools which do not register an abort.  And to my
current knowledge even httpd doesn't do so.

Sander


RE: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Sander Striker <st...@apache.org>.
> On Fri, Aug 24, 2001 at 11:09:13PM +0200, Sander Striker wrote:
>>...
>> Mine takes it a bit further.  It essentially makes the freelist more
>> efficient.  Adds the possibility to request a new freelist for a pool,
>> which can optionally have locking.
> 
> Cool.
> 
>> Furthermore it resolves some
>> issues with possible segfaults in the current pools code when running
>> out of memory.
> 
> Not necessary :-)

Well, for the apr_pvsprintf routine I disagree.  When out of mem, this
routine will segfault regardless of having registered an abort function
with the pool or not.  In the modified situation, the pools abort
function is called when out of mem and NULL is returned.  I know you
consider the latter unnecessary, but I try to think of other applications
using APR and thus pools which do not register an abort.  And to my
current knowledge even httpd doesn't do so.

Sander


Re: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Greg Stein <gs...@lyra.org>.
On Fri, Aug 24, 2001 at 11:09:13PM +0200, Sander Striker wrote:
>...
> Mine takes it a bit further.  It essentially makes the freelist more
> efficient.  Adds the possibility to request a new freelist for a pool,
> which can optionally have locking.

Cool.

> Furthermore it resolves some
> issues with possible segfaults in the current pools code when running
> out of memory.

Not necessary :-)

> However, there is a downside.  I started with an empty slate and then
> started to add things in.  I still haven't merged some things over
> (the APR_POOL_DEBUG mode, USE_MALLOC and other fancy debug stuff).  Also,
> my code isn't documented very well at the moment (OTOH, the current
> pools code isn't exactly a quick read with all those #if[def] lines...).

For the pools code, it can only be patched. It is unacceptable to toss a
completely written-from-scratch replacement into the code base. If it takes
a sequence of 20 patches to reach the written-from-scratch stage, then
fine... but that means each step has been reviewable as you go along.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: Review of possible replacement for pools

Posted by Brian Pane <bp...@pacbell.net>.
Bill Stoddard wrote:

>Why are we spending time trying to optimize pools when we haven't eliminated the
>malloc/frees in the bucket brigade calls? The miniscule performance improvements
>you -might- get optimizing pools will be completely obscured by the overhead of the
>malloc/frees.
>
I'm skeptical of this conclusion.  Ian's and Justin's experimental results
show measurable improvements in CPU usage and throughput from modified pool
code.  Fixing the malloc/free overhead in brigades will be a big win on
systems with a slow malloc, but the benchmark numbers so far indicate that
it need not be in the critical path for fixing the pool scalability.

IMHO, fixing the mutex contention in pools sooner, rather than later, is 
also
a good idea in general because it removes an architectural bottleneck 
that can
hide the effects of performance tuning in other areas.

--Brian




RE: Review of possible replacement for pools

Posted by Sander Striker <st...@apache.org>.
> > > Why are we spending time trying to optimize pools when we haven't
> > > eliminated the
> > > malloc/frees in the bucket brigade calls? The miniscule
> > > performance improvements
> > > you -might- get optimizing pools will be completely obscured by
> > > the overhead of the
> > > malloc/frees.
> > >
> > > Bill
> >
> > Because it also eliminates the locking in the threaded mpm.
> > And, I wanted to see if I could squeeze some extra performance
> > out of it and make it a bit more readable.  I don't know if
> > I succeeded in the latter.  Or the performance for that matter :)
> >
> > Btw, why couldn't we use pools in brigades again?
> >
> 
> Brigades need to be maintained across multiple requests to handle 
> http pipelined
> responses, so allocating them out of a request pool clearly will 
> not work. You could
> allocate the brigades out of a connection pool, but connections 
> can remain active for
> 100's of requests so you could leak a lot of storage for the 
> duration of the connection.

Wait, couldn't you create a new pool for each brigade, allocate
the brigade out of that pool, associate that pool with the brigade?
Then, when the brigade needs to go, just destroy that pool.
What am I missing?
 
> Bill


Re: Review of possible replacement for pools

Posted by Bill Stoddard <bi...@wstoddard.com>.

> > Why are we spending time trying to optimize pools when we haven't
> > eliminated the
> > malloc/frees in the bucket brigade calls? The miniscule
> > performance improvements
> > you -might- get optimizing pools will be completely obscured by
> > the overhead of the
> > malloc/frees.
> >
> > Bill
>
> Because it also eliminates the locking in the threaded mpm.
> And, I wanted to see if I could squeeze some extra performance
> out of it and make it a bit more readable.  I don't know if
> I succeeded in the latter.  Or the performance for that matter :)
>
> Btw, why couldn't we use pools in brigades again?
>

Brigades need to be maintained across multiple requests to handle http pipelined
responses, so allocating them out of a request pool clearly will not work. You could
allocate the brigades out of a connection pool, but connections can remain active for
100's of requests so you could leak a lot of storage for the duration of the connection.

Bill


Re: pools and buckets/brigades, WAS: RE: Review of possible replacement for pools

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Bill Stoddard" <bi...@wstoddard.com>
Sent: Saturday, August 25, 2001 1:40 PM


> > I have a problem with the original question though.  I don't understand why
> > anybody is trying to state that if people want to improve the pools code, they
> > shouldn't, because we use malloc in one place in the code.  People are
> > free to work on whatever they want.
> 
> I am not being philosophical. Sure, people can spend their time doing whatever they want.
> I assume open source developers like to see others benefit from their labor. If the goal
> is to improve the performance of the http server, (remember this was posted to dev@httpd
> and that is why I reference the http server), they are wasting their time trying to
> improve the pool code. There are clearly more serious memory allocation problems to solve
> first.

Then scratch that itch, Bill, instead of moaning about it.

> Gotta run. Party on...

My point exactly ;)


Re: pools and buckets/brigades, WAS: RE: Review of possible replacement for pools

Posted by Bill Stoddard <bi...@wstoddard.com>.
>
> I have a problem with the original question though.  I don't understand why
> anybody is trying to state that if people want to improve the pools code, they
> shouldn't, because we use malloc in one place in the code.  People are
> free to work on whatever they want.

I am not being philosophical. Sure, people can spend their time doing whatever they want.
I assume open source developers like to see others benefit from their labor. If the goal
is to improve the performance of the http server, (remember this was posted to dev@httpd
and that is why I reference the http server), they are wasting their time trying to
improve the pool code. There are clearly more serious memory allocation problems to solve
first.

Gotta run. Party on...

Bill


Re: pools and buckets/brigades, WAS: RE: Review of possible replacement for pools

Posted by Ryan Bloom <rb...@covalent.net>.
On Saturday 25 August 2001 11:38, Sander Striker wrote:
> > > Btw, why couldn't we use pools in brigades again?
> >
> > Because of lifetimes.  If you allocate buckets and brigades out of pools,
> > the lifetimes can't change, but bucket lifetimes aren't known at creation
> > time.  And, you can't use the longest possible lifetime, because that
> > would be a leak.
>
> Ah, I suppose associating a pool with a brigade wouldn't work, would it?
> The buckets can move from brigade to brigade?

There is alreadya pool associated with a brigade.  That doesn't get you
anything though.  A bucket needs to be able to move from one brigade
to another.

I have a problem with the original question though.  I don't understand why
anybody is trying to state that if people want to improve the pools code, they
shouldn't, because we use malloc in one place in the code.  People are
free to work on whatever they want.  If somebody wants to remove the malloc
calls from the buckets and bucket brigades, then they can, but that doesn't
mean that everybody has to focus on that.

Ryan
______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: pools and buckets/brigades, WAS: RE: Review of possible replacement for pools

Posted by Ryan Bloom <rb...@covalent.net>.
On Saturday 25 August 2001 11:38, Sander Striker wrote:
> > > Btw, why couldn't we use pools in brigades again?
> >
> > Because of lifetimes.  If you allocate buckets and brigades out of pools,
> > the lifetimes can't change, but bucket lifetimes aren't known at creation
> > time.  And, you can't use the longest possible lifetime, because that
> > would be a leak.
>
> Ah, I suppose associating a pool with a brigade wouldn't work, would it?
> The buckets can move from brigade to brigade?

There is alreadya pool associated with a brigade.  That doesn't get you
anything though.  A bucket needs to be able to move from one brigade
to another.

I have a problem with the original question though.  I don't understand why
anybody is trying to state that if people want to improve the pools code, they
shouldn't, because we use malloc in one place in the code.  People are
free to work on whatever they want.  If somebody wants to remove the malloc
calls from the buckets and bucket brigades, then they can, but that doesn't
mean that everybody has to focus on that.

Ryan
______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

pools and buckets/brigades, WAS: RE: Review of possible replacement for pools

Posted by Sander Striker <st...@apache.org>.
> > Btw, why couldn't we use pools in brigades again?
> 
> Because of lifetimes.  If you allocate buckets and brigades out of pools,
> the lifetimes can't change, but bucket lifetimes aren't known at creation
> time.  And, you can't use the longest possible lifetime, because that
> would be a leak.

Ah, I suppose associating a pool with a brigade wouldn't work, would it?
The buckets can move from brigade to brigade?

> Ryan

Sander


pools and buckets/brigades, WAS: RE: Review of possible replacement for pools

Posted by Sander Striker <st...@apache.org>.
> > Btw, why couldn't we use pools in brigades again?
> 
> Because of lifetimes.  If you allocate buckets and brigades out of pools,
> the lifetimes can't change, but bucket lifetimes aren't known at creation
> time.  And, you can't use the longest possible lifetime, because that
> would be a leak.

Ah, I suppose associating a pool with a brigade wouldn't work, would it?
The buckets can move from brigade to brigade?

> Ryan

Sander


Re: Review of possible replacement for pools

Posted by Ryan Bloom <rb...@covalent.net>.
On Saturday 25 August 2001 11:26, Sander Striker wrote:
> > Why are we spending time trying to optimize pools when we haven't
> > eliminated the
> > malloc/frees in the bucket brigade calls? The miniscule
> > performance improvements
> > you -might- get optimizing pools will be completely obscured by
> > the overhead of the
> > malloc/frees.
> >
> > Bill
>
> Because it also eliminates the locking in the threaded mpm.
> And, I wanted to see if I could squeeze some extra performance
> out of it and make it a bit more readable.  I don't know if
> I succeeded in the latter.  Or the performance for that matter :)
>
> Btw, why couldn't we use pools in brigades again?

Because of lifetimes.  If you allocate buckets and brigades out of pools,
the lifetimes can't change, but bucket lifetimes aren't known at creation
time.  And, you can't use the longest possible lifetime, because that
would be a leak.

Ryan

______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: Review of possible replacement for pools

Posted by Ryan Bloom <rb...@covalent.net>.
On Saturday 25 August 2001 11:26, Sander Striker wrote:
> > Why are we spending time trying to optimize pools when we haven't
> > eliminated the
> > malloc/frees in the bucket brigade calls? The miniscule
> > performance improvements
> > you -might- get optimizing pools will be completely obscured by
> > the overhead of the
> > malloc/frees.
> >
> > Bill
>
> Because it also eliminates the locking in the threaded mpm.
> And, I wanted to see if I could squeeze some extra performance
> out of it and make it a bit more readable.  I don't know if
> I succeeded in the latter.  Or the performance for that matter :)
>
> Btw, why couldn't we use pools in brigades again?

Because of lifetimes.  If you allocate buckets and brigades out of pools,
the lifetimes can't change, but bucket lifetimes aren't known at creation
time.  And, you can't use the longest possible lifetime, because that
would be a leak.

Ryan

______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

RE: Review of possible replacement for pools

Posted by Sander Striker <st...@apache.org>.
> Why are we spending time trying to optimize pools when we haven't 
> eliminated the
> malloc/frees in the bucket brigade calls? The miniscule 
> performance improvements
> you -might- get optimizing pools will be completely obscured by 
> the overhead of the
> malloc/frees.
> 
> Bill

Because it also eliminates the locking in the threaded mpm.
And, I wanted to see if I could squeeze some extra performance
out of it and make it a bit more readable.  I don't know if
I succeeded in the latter.  Or the performance for that matter :)

Btw, why couldn't we use pools in brigades again?

Sander


RE: Review of possible replacement for pools

Posted by Sander Striker <st...@apache.org>.
> Why are we spending time trying to optimize pools when we haven't 
> eliminated the
> malloc/frees in the bucket brigade calls? The miniscule 
> performance improvements
> you -might- get optimizing pools will be completely obscured by 
> the overhead of the
> malloc/frees.
> 
> Bill

Because it also eliminates the locking in the threaded mpm.
And, I wanted to see if I could squeeze some extra performance
out of it and make it a bit more readable.  I don't know if
I succeeded in the latter.  Or the performance for that matter :)

Btw, why couldn't we use pools in brigades again?

Sander


Re: Review of possible replacement for pools

Posted by Bill Stoddard <bi...@wstoddard.com>.
Why are we spending time trying to optimize pools when we haven't eliminated the
malloc/frees in the bucket brigade calls? The miniscule performance improvements
you -might- get optimizing pools will be completely obscured by the overhead of the
malloc/frees.

Bill

> Justin Erenkrantz wrote:
>
> >On Sat, Aug 25, 2001 at 08:05:03PM +0200, Sander Striker wrote:
> >
> >>Adding an option to apr_pool_create_ex to indicate that the pool is
> >>going to be shared across threads would be an option I can certainly
> >>live with.  OTOH, this will introduce an extra if(lock) pair in the
> >>apr_palloc and apr_pcalloc calls.  I'll send in a patch later on.
> >>
> >
> >If you aren't shared, you have an extra NULL-check.  I think that is
> >acceptable.  If you are shared, we make sure we can't get trounced.
> >
> I agree, as long as the NULL-check doesn't measurably impact
> performance.  One of the key success factors for the original pool
> code is the very short code path in the common case where it doesn't
> need to allocate new blocks.  Adding another branch to this code
> path might or might not slow it down measurably--but we can find
> out through benchmarking.
>
> --Brian
>
>
>


Re: Review of possible replacement for pools

Posted by Brian Pane <bp...@pacbell.net>.
Justin Erenkrantz wrote:

>On Sat, Aug 25, 2001 at 08:05:03PM +0200, Sander Striker wrote:
>
>>Adding an option to apr_pool_create_ex to indicate that the pool is
>>going to be shared across threads would be an option I can certainly
>>live with.  OTOH, this will introduce an extra if(lock) pair in the
>>apr_palloc and apr_pcalloc calls.  I'll send in a patch later on.
>>
>
>If you aren't shared, you have an extra NULL-check.  I think that is
>acceptable.  If you are shared, we make sure we can't get trounced.
>
I agree, as long as the NULL-check doesn't measurably impact
performance.  One of the key success factors for the original pool
code is the very short code path in the common case where it doesn't
need to allocate new blocks.  Adding another branch to this code
path might or might not slow it down measurably--but we can find
out through benchmarking.

--Brian




RE: Review of possible replacement for pools

Posted by Sander Striker <st...@apache.org>.
> Attached is a patch for the suggested additional flag to
> be able to share and use a pool across threads.

Grmpf.  I sent to quickly.  The lock should be used in
the cleanup functions aswell.
 
> Sander


RE: Review of possible replacement for pools

Posted by Sander Striker <st...@apache.org>.
Attached is a patch for the suggested additional flag to
be able to share and use a pool across threads.

Sander

> -----Original Message-----
> From: Justin Erenkrantz [mailto:jerenkrantz@ebuilt.com]
> Sent: 25 August 2001 19:57
> To: Sander Striker
> Cc: dev@apr.apache.org
> Subject: Re: Review of possible replacement for pools
> 
> 
> On Sat, Aug 25, 2001 at 08:05:03PM +0200, Sander Striker wrote:
> > Adding an option to apr_pool_create_ex to indicate that the pool is
> > going to be shared across threads would be an option I can certainly
> > live with.  OTOH, this will introduce an extra if(lock) pair in the
> > apr_palloc and apr_pcalloc calls.  I'll send in a patch later on.
> 
> If you aren't shared, you have an extra NULL-check.  I think that is
> acceptable.  If you are shared, we make sure we can't get trounced.
> 
> > I didn't implement apr_prealloc yet, but would like to know if people
> > would find it usefull.  If so, I will post a patch.
> 
> Probably - I've come across the need for such a function before.  But,
> it does get to be tricky to implement.  -- justin
> 
> 
> 
> 

Re: Review of possible replacement for pools

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sat, Aug 25, 2001 at 08:05:03PM +0200, Sander Striker wrote:
> Adding an option to apr_pool_create_ex to indicate that the pool is
> going to be shared across threads would be an option I can certainly
> live with.  OTOH, this will introduce an extra if(lock) pair in the
> apr_palloc and apr_pcalloc calls.  I'll send in a patch later on.

If you aren't shared, you have an extra NULL-check.  I think that is
acceptable.  If you are shared, we make sure we can't get trounced.

> I didn't implement apr_prealloc yet, but would like to know if people
> would find it usefull.  If so, I will post a patch.

Probably - I've come across the need for such a function before.  But,
it does get to be tricky to implement.  -- justin


RE: Review of possible replacement for pools

Posted by Sander Striker <st...@apache.org>.
> On Sat, Aug 25, 2001 at 01:45:21PM +0200, Sander Striker wrote:
> > This _is_ a race condition.  I reported this at the end of june on list.
> > Subject: Possible race condition in pools WAS: RE: Thoughts on locks.
> > At the end of the thread the conclusion was to fix it, most likely
> > through documentation.
> >
> > Personally I think that locking on each allocation will kill
> performance.
> > Just never use a pool in two threads.
>
> Yeah, I vaguely remember that thread.
>
> You can certainly use a pool in two threads.  If we add an option when
> creating a pool to indicate that it isn't shared, the performance
> isn't a factor.  But, allowing this race condition is going to trounce
> somebody at sometime.  The likelihood of having the context switch
> happening here is low, but it is possible.

Agreed.

> I'm not exactly sure how to fix it in the current pool code without
> killing everyone.  With your version, you can do it via an option to
> apr_pool_create_ex.  So, I don't think we'd end up losing that much
> performance if we use the option in the right places.  -- justin

Adding an option to apr_pool_create_ex to indicate that the pool is
going to be shared across threads would be an option I can certainly
live with.  OTOH, this will introduce an extra if(lock) pair in the
apr_palloc and apr_pcalloc calls.  I'll send in a patch later on.

I didn't implement apr_prealloc yet, but would like to know if people
would find it usefull.  If so, I will post a patch.

Sander


Re: Review of possible replacement for pools

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sat, Aug 25, 2001 at 01:45:21PM +0200, Sander Striker wrote:
> This _is_ a race condition.  I reported this at the end of june on list.
> Subject: Possible race condition in pools WAS: RE: Thoughts on locks.
> At the end of the thread the conclusion was to fix it, most likely
> through documentation.
> 
> Personally I think that locking on each allocation will kill performance.
> Just never use a pool in two threads.

Yeah, I vaguely remember that thread.

You can certainly use a pool in two threads.  If we add an option when
creating a pool to indicate that it isn't shared, the performance 
isn't a factor.  But, allowing this race condition is going to trounce 
somebody at sometime.  The likelihood of having the context switch
happening here is low, but it is possible.

I'm not exactly sure how to fix it in the current pool code without
killing everyone.  With your version, you can do it via an option to 
apr_pool_create_ex.  So, I don't think we'd end up losing that much
performance if we use the option in the right places.  -- justin


RE: Memory management requirements buckets/brigades, new pools

Posted by Ian Holsman <Ia...@cnet.com>.
On Sun, 2001-08-26 at 09:02, Sander Striker wrote:
> > Hi Sander.
> > is it possible to post patches from the most recent CVS version.
> > that would make it much easier for people to apply them
> 
> Ok, I agree with you on applyability, but for readability that
> is quite different.  Want me to post some patches against recent
> CVS?
[removed httpd]
if you could.
that would make it easier for me to test tomorrow.
> 
> > Thanks
> > ...Ian
> 
> Sander
-- 
Ian Holsman
Performance Measurement & Analysis
CNET Networks    -    415 364-8608

RE: Memory management requirements buckets/brigades, new pools

Posted by Sander Striker <st...@apache.org>.
> Hi Sander.
> is it possible to post patches from the most recent CVS version.
> that would make it much easier for people to apply them

Ok, I agree with you on applyability, but for readability that
is quite different.  Want me to post some patches against recent
CVS?

> Thanks
> ...Ian

Sander


RE: Memory management requirements buckets/brigades, new pools

Posted by Sander Striker <st...@apache.org>.
> Hi Sander.
> is it possible to post patches from the most recent CVS version.
> that would make it much easier for people to apply them

Ok, I agree with you on applyability, but for readability that
is quite different.  Want me to post some patches against recent
CVS?

> Thanks
> ...Ian

Sander


RE: Memory management requirements buckets/brigades, new pools

Posted by Ian Holsman <Ia...@cnet.com>.
Hi Sander.
is it possible to post patches from the most recent CVS version.
that would make it much easier for people to apply them

Thanks
..Ian
On Sun, 2001-08-26 at 08:29, Sander Striker wrote:
> [this time including the attachment...]
> 
> > Hi,
> > 
> > It seems that the memory management requirements for
> > buckets is that they have to be able to control their
> > own lifetime.  In other words, they need to be allocated
> > and freed on an individual basis.
> > It seems that their lifetime is bound by the lifetime
> > of the connection.
> > 
> > The above let me believe that buckets need a free
> > function to complement apr_palloc.  Hence the
> > attached patch that introduces apr_pfree *).  I know
> > this patch introduces some extra overhead, although
> > not much, which could be unacceptable.  OTOH would
> > this make it possible to use one memory management
> > scheme throughout apache...
> > 
> > Maybe something to consider, maybe not.  I don't
> > even know if these are the criteria or not ;)
> > 
> > 
> > Sander
> > 
> > *) patch is against the recently posted possible
> >    replacement code for pools.
> 
-- 
Ian Holsman
Performance Measurement & Analysis
CNET Networks    -    415 364-8608

RE: Memory management requirements buckets/brigades, new pools

Posted by Ian Holsman <Ia...@cnet.com>.
Hi Sander.
is it possible to post patches from the most recent CVS version.
that would make it much easier for people to apply them

Thanks
..Ian
On Sun, 2001-08-26 at 08:29, Sander Striker wrote:
> [this time including the attachment...]
> 
> > Hi,
> > 
> > It seems that the memory management requirements for
> > buckets is that they have to be able to control their
> > own lifetime.  In other words, they need to be allocated
> > and freed on an individual basis.
> > It seems that their lifetime is bound by the lifetime
> > of the connection.
> > 
> > The above let me believe that buckets need a free
> > function to complement apr_palloc.  Hence the
> > attached patch that introduces apr_pfree *).  I know
> > this patch introduces some extra overhead, although
> > not much, which could be unacceptable.  OTOH would
> > this make it possible to use one memory management
> > scheme throughout apache...
> > 
> > Maybe something to consider, maybe not.  I don't
> > even know if these are the criteria or not ;)
> > 
> > 
> > Sander
> > 
> > *) patch is against the recently posted possible
> >    replacement code for pools.
> 
-- 
Ian Holsman
Performance Measurement & Analysis
CNET Networks    -    415 364-8608

RE: Memory management requirements buckets/brigades, new pools

Posted by Sander Striker <st...@apache.org>.
[this time including the attachment...]

> Hi,
> 
> It seems that the memory management requirements for
> buckets is that they have to be able to control their
> own lifetime.  In other words, they need to be allocated
> and freed on an individual basis.
> It seems that their lifetime is bound by the lifetime
> of the connection.
> 
> The above let me believe that buckets need a free
> function to complement apr_palloc.  Hence the
> attached patch that introduces apr_pfree *).  I know
> this patch introduces some extra overhead, although
> not much, which could be unacceptable.  OTOH would
> this make it possible to use one memory management
> scheme throughout apache...
> 
> Maybe something to consider, maybe not.  I don't
> even know if these are the criteria or not ;)
> 
> 
> Sander
> 
> *) patch is against the recently posted possible
>    replacement code for pools.

Memory management requirements buckets/brigades, new pools

Posted by Sander Striker <st...@apache.org>.
Hi,

It seems that the memory management requirements for
buckets is that they have to be able to control their
own lifetime.  In other words, they need to be allocated
and freed on an individual basis.
It seems that their lifetime is bound by the lifetime
of the connection.

The above let me believe that buckets need a free
function to complement apr_palloc.  Hence the
attached patch that introduces apr_pfree *).  I know
this patch introduces some extra overhead, although
not much, which could be unacceptable.  OTOH would
this make it possible to use one memory management
scheme throughout apache...

Maybe something to consider, maybe not.  I don't
even know if these are the criteria or not ;)


Sander

*) patch is against the recently posted possible
   replacement code for pools.

RE: Review of possible replacement for pools

Posted by Dirk-Willem van Gulik <di...@covalent.net>.
> This _is_ a race condition.  I reported this at the end of june on list.
> Subject: Possible race condition in pools WAS: RE: Thoughts on locks.
> At the end of the thread the conclusion was to fix it, most likely
> through documentation.
>
> Personally I think that locking on each allocation will kill performance.
> Just never use a pool in two threads.

Whilst debugging some of my own code I found it usefull to add to each
pool a record of the thread id (which is very cheap to retrive) and do a
comparision each time you try to use it when a DEBUG flag is on. I found
that to be a practical compromize to spot the issue.

Dw


RE: Review of possible replacement for pools

Posted by Sander Striker <st...@apache.org>.
Hi,

> Enclosed is my review of your posted patch.  Actually, I probably would
> like to see Brian's patch again to compare it as I think they achieve
> similar goals.
>
> The important aspect of this patch is that it makes the pool code
> easier to understand.  I'll say that this would need comments.
> I'll volunteer to add comments that if this patch is accepted.

Yes.  Just for clarity: I started with an empty file and copied source
or concept over from the original pools code.  Then I added some of
my personal sause and this is the result.  So, similarity with the
original pools code is no coincedence.

> Here goes - my comments are prefaced with a > at the beginning of
> the line (the reverse of normal as I'm too lazy to insert > at the
> beginning of the source)...
>
> -------------
> #define MIN_ALLOC 8192
> #define MAX_INDEX   32
>
> #define BOUNDARY_INDEX 12
> #define BOUNDARY_SIZE (1 << BOUNDARY_INDEX)
>
>> Okay so that people have a clue what is going on here, Sander has
>> a freelist that is arranged by size.  It is based on the power of
>> 2 that it is closest to, IIRC.  The 0 element is reserved for items
>> bigger than 2^32.  Sander can correct me if I'm misremembering this.

Every node is a multiple of BOUNDARY_SIZE, which is in this case 4096.
So, when requesting 9034 bytes, we alloc a node of 12288 bytes and
serve it from that.  The remainder is saved for the next allocation,
just like the original pools code.  The minimal size of a node is
MIN_ALLOC, which is set to 8192.
On the free list, the nodes are indexed.  Or, nodes up to a certain
size are indexed, I should say, the rest is dumped in the first slot.
The indexing takes place to up to BOUNDARY_SIZE * MAX_INDEX node
sizes.  To see how it works, take a close look at node_malloc() and
node_free().

> #define ALIGN_DEFAULT(size) ALIGN(size, 8)
>
>> Why 8 as the default?

A guessed fixed number.  No science involved.  If someone has a better
way of doing this _without_ introduction of '/' and '*' I'll be happy.
If 8 is bogus, please suggest a better number.

> static APR_INLINE node_t *node_malloc(allocator_t *allocator,
> apr_size_t size)
> {
> >   ...blah, blah, blah...search for it on the indexed freelist and
> >   return it...if none available, call malloc...at this point, we are
> >   setting up the newly allocated node.
>     node->next = NULL;
>     node->index = index;
>     node->first_avail = (char *)node + SIZEOF_NODE_T;
>     node->endp = (char *)node + size;
>
>> Problem A.  You are adding the SIZEOF_NODE_T here that is implicitly
>> included in the size parameter (most calls to node_malloc has size +
>> SIZEOF_NODE_T).  Either move this portion out (not sure how you
>> would do this), or have node_malloc add the space required for the
>> node_t and don't call node_malloc with + SIZEOF_NODE_T.  However, that
>> strategy does present a problem in apr_pool_create_ex where you call
>> node_malloc with MIN_ALLOC as the size rather than
>> MIN_ALLOC+SIZEOF_NODE_T.  So, in order to truly get MIN_ALLOC, you'd
>> have to ask for MIN_ALLOC-SIZEOF_NODE_T.  Hmm.  But, I don't like
>> this strategy where you *must* pass in an extra SIZEOF_NODE_T bytes.
>> (Yes, this function isn't user-callable, but still...)

I was facing the same, this is what I chose to use.
Other solutions welcomed.  I somewhat like the MIN_ALLOC - SIZEOF_NODE_T
better in apr_pool_create_ex.  I'll put this in a seperate patch, for
people to choose (or someone can come up with something better :).
Keep in mind that this is an internal function and a little line at the
top of the function explaining what you need to pass in would be
satisfactory
too IMHO.

> static void node_free(allocator_t *allocator, node_t *node)
> {
>     node_t *next;
>     apr_uint32_t index, max_index;
>
>     if (allocator->lock)
>         apr_lock_acquire(allocator->lock);
>
>> This reminds me that we should have something like
>> apr_lock_acquire_opt(foo) which will not do anything if the passed in
>> parameter is NULL.  This would seriously cut down on the number of
>> places where this if (lock) lock_acquire syntax occurrs.  Someone
>> brought this up in a commit log recently.  It's a good point.  Hell,
>> even a #defined macro works well for this.

I'll put a simple macro solution in this code as a seperate patch.
We might consider exporting such a macro in apr_lock.h.

> APR_DECLARE(void *) apr_palloc(apr_pool_t *pool, apr_size_t size)
> {
>     node_t *active, *node;
>     void *mem;
>     char *endp;
>
>     size = ALIGN_DEFAULT(size);
>     active = pool->mactive;
>
>     endp = active->first_avail + size;
>     if (endp < active->endp) {
>         mem = active->first_avail;
>         active->first_avail = endp;
>         return mem;
>     }
>
>> I think this is a race condition.  This is also present in the current
>> pool code, so this isn't something that you introduced.  Consider the
>> following scenario:
>> Thread A has access to pool Z.
>> Thread B has access to pool Z as well.
>> Thread A calls apr_palloc, gets active, sees that there is enough
>> space, sets mem.
>> *CONTEXT SWITCH*
>> Thread B now calls apr_palloc, gets active, sess that there is enough
>> space, sets mem, updates first_avail, returns.
>> *CONTEXT SWITCH*
>> Thread A updates first_avail.  Oops.  =-)  However, I'd suggest an
>> option in apr_create_pool_ex that says that this pool is NOT shared
>> and doesn't need locking.  Otherwise, we must lock this case (not
>> an allocator lock, but a pool lock).  This seems to be the only case
>> where this could happen.  Seems a waste, but it definitely could happen.
>> This race doesn't matter in the apr_pool_clear or apr_pool_destroy
>> since two children don't try to clear the pool at the same time.

This _is_ a race condition.  I reported this at the end of june on list.
Subject: Possible race condition in pools WAS: RE: Thoughts on locks.
At the end of the thread the conclusion was to fix it, most likely
through documentation.

Personally I think that locking on each allocation will kill performance.
Just never use a pool in two threads.

Btw, similar race conditions are present in the cleanup code in pools.
I haven't touched those.  Take a look and conclude that no 2 threads
should ever register a cleanup at the same time. Or I must be missing
something...

If there are going to be two locks (I doubt it, but it might happen),
the pool lock should be used when adding/removing child pools to a
pool.  At the moment I am using the allocator lock for that.

>> I know the code doesn't crash under high load, so I know it has
>> a reasonable degree of quality and it is definitely easier to
>> understand.  Assuming the matters brought up above are addressed
>> to my satisfaction, I'll give this a +1.  -- justin

Thanks for the review, patches to last version and new version (which
is the old version, with patches applied), attached.

Sander

Review of possible replacement for pools

Posted by Justin Erenkrantz <je...@ebuilt.com>.
Enclosed is my review of your posted patch.  Actually, I probably would
like to see Brian's patch again to compare it as I think they achieve
similar goals.

The important aspect of this patch is that it makes the pool code
easier to understand.  I'll say that this would need comments.  
I'll volunteer to add comments that if this patch is accepted.

Here goes - my comments are prefaced with a > at the beginning of
the line (the reverse of normal as I'm too lazy to insert > at the
beginning of the source)...

-------------
#define MIN_ALLOC 8192
#define MAX_INDEX   32

#define BOUNDARY_INDEX 12
#define BOUNDARY_SIZE (1 << BOUNDARY_INDEX)

> Okay so that people have a clue what is going on here, Sander has
> a freelist that is arranged by size.  It is based on the power of
> 2 that it is closest to, IIRC.  The 0 element is reserved for items
> bigger than 2^32.  Sander can correct me if I'm misremembering this.

#define ALIGN_DEFAULT(size) ALIGN(size, 8)

> Why 8 as the default?

static APR_INLINE node_t *node_malloc(allocator_t *allocator, apr_size_t size)
{
>   ...blah, blah, blah...search for it on the indexed freelist and
>   return it...if none available, call malloc...at this point, we are
>   setting up the newly allocated node.
    node->next = NULL;
    node->index = index;
    node->first_avail = (char *)node + SIZEOF_NODE_T;
    node->endp = (char *)node + size;

> Problem A.  You are adding the SIZEOF_NODE_T here that is implicitly
> included in the size parameter (most calls to node_malloc has size +
> SIZEOF_NODE_T).  Either move this portion out (not sure how you
> would do this), or have node_malloc add the space required for the
> node_t and don't call node_malloc with + SIZEOF_NODE_T.  However, that 
> strategy does present a problem in apr_pool_create_ex where you call 
> node_malloc with MIN_ALLOC as the size rather than 
> MIN_ALLOC+SIZEOF_NODE_T.  So, in order to truly get MIN_ALLOC, you'd 
> have to ask for MIN_ALLOC-SIZEOF_NODE_T.  Hmm.  But, I don't like
> this strategy where you *must* pass in an extra SIZEOF_NODE_T bytes.
> (Yes, this function isn't user-callable, but still...)

static void node_free(allocator_t *allocator, node_t *node)
{
    node_t *next;
    apr_uint32_t index, max_index;

    if (allocator->lock)
        apr_lock_acquire(allocator->lock);

> This reminds me that we should have something like 
> apr_lock_acquire_opt(foo) which will not do anything if the passed in 
> parameter is NULL.  This would seriously cut down on the number of
> places where this if (lock) lock_acquire syntax occurrs.  Someone
> brought this up in a commit log recently.  It's a good point.  Hell, 
> even a #defined macro works well for this.  

APR_DECLARE(void *) apr_palloc(apr_pool_t *pool, apr_size_t size)
{
    node_t *active, *node;
    void *mem;
    char *endp;

    size = ALIGN_DEFAULT(size);
    active = pool->mactive;

    endp = active->first_avail + size;
    if (endp < active->endp) {
        mem = active->first_avail;
        active->first_avail = endp;
        return mem;
    }

> I think this is a race condition.  This is also present in the current
> pool code, so this isn't something that you introduced.  Consider the
> following scenario:
> Thread A has access to pool Z.
> Thread B has access to pool Z as well.
> Thread A calls apr_palloc, gets active, sees that there is enough
> space, sets mem.
> *CONTEXT SWITCH*
> Thread B now calls apr_palloc, gets active, sess that there is enough
> space, sets mem, updates first_avail, returns.
> *CONTEXT SWITCH*
> Thread A updates first_avail.  Oops.  =-)  However, I'd suggest an
> option in apr_create_pool_ex that says that this pool is NOT shared
> and doesn't need locking.  Otherwise, we must lock this case (not
> an allocator lock, but a pool lock).  This seems to be the only case 
> where this could happen.  Seems a waste, but it definitely could happen.
> This race doesn't matter in the apr_pool_clear or apr_pool_destroy
> since two children don't try to clear the pool at the same time.
>
> I know the code doesn't crash under high load, so I know it has
> a reasonable degree of quality and it is definitely easier to 
> understand.  Assuming the matters brought up above are addressed
> to my satisfaction, I'll give this a +1.  -- justin
>
> P.S. Oh, man.  This week's fun never ends.  My mail server that
> I receive email on had a full /var partition.  What is going on
> this week?  Well, I'm slowly getting queued-up emails now...I
> thought the list was awfully quiet.


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Justin Erenkrantz <je...@ebuilt.com>.
> The t/htdocs/modules/include/*.shtml stuff is probably the best at the
> moment... if you can run those many, many times with a lot of concurrency,
> that'd probably get you somewhere.

Thanks.  I didn't even think of using httpd-test's perl-framework (I'm 
not really familiar with it).  I snarfed in that directory and pointed
flood at big.  Now, I think we're getting somewhere - the CPU is maxed
out.  Cool.

> Throw mod_ssl into the equation and you might get even closer to something
> testable, though it'd arguably be better if the load were pool-use-heavy.

Well, I think with all of the benchmarks flying around, it'd be nice to
agree that "URL X, followed by URL Y, followed by URL Z" is at least
a decent coverage of httpd-2.0 code.  At least for determining relative
merits of code A vs. code B that affects the entire server.  I dunno,
it's a thought.  Right now, everybody just uses ab and points at /
or some small static file.  I think it'd be nice to standardize this
somewhat.

FWIW, the default httpd.conf is awful.  mod_include isn't allowed by
default anywhere (even in the manual).  That needs to be fixed.  It
took me 30 minutes to get mod_include to serve a shtml file.  Drats.
-- justin


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Cliff Woolley <cl...@yahoo.com>.
On Fri, 24 Aug 2001, Justin Erenkrantz wrote:

> On Fri, Aug 24, 2001 at 05:44:40PM -0700, Aaron Bannert wrote:
> > mod_include, PHP -- something dynamic? You just want to CPU-strap
> > some typical system usage, and then see what the maximum req/sec
> > throughput is you can get, right?
>
> Right, but I see no mod_include pages that are in the current CVS
> tree (FAQ.html looks likes it should be).  I'm trying to do
> something that is reproducible by other people.  =-)  -- justin

The t/htdocs/modules/include/*.shtml stuff is probably the best at the
moment... if you can run those many, many times with a lot of concurrency,
that'd probably get you somewhere.

Better yet, take all of those, make a bigass.shtml out of them with loads
of recursion and multiple includes of the same file over and over, etc.

Throw mod_ssl into the equation and you might get even closer to something
testable, though it'd arguably be better if the load were pool-use-heavy.

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Fri, Aug 24, 2001 at 05:44:40PM -0700, Aaron Bannert wrote:
> mod_include, PHP -- something dynamic? You just want to CPU-strap
> some typical system usage, and then see what the maximum req/sec
> throughput is you can get, right?

Right, but I see no mod_include pages that are in the current CVS
tree (FAQ.html looks likes it should be).  I'm trying to do 
something that is reproducible by other people.  =-)  -- justin


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Aaron Bannert <aa...@clove.org>.
On Fri, Aug 24, 2001 at 05:40:59PM -0700, Justin Erenkrantz wrote:
> On Fri, Aug 24, 2001 at 05:21:11PM -0700, Marc Slemko wrote:
> > On Fri, 24 Aug 2001, Justin Erenkrantz wrote:
> > 
> > > If you have the backplane (my guess would be Gigabit Ethernet) to
> > 
> > Well, or just a less powerful server.  The goal isn't the best raw
> > numbers, the goal is comparison.  Granted, that comparison can be
> > different on various sized machines (eg. SMP vs. not, etc.).
> 
> Well, part of the problem is that Sander's pool optimization is 
> intended to benefit MP boxes.  It adds thread-local free-lists - so
> the benefit is probably only noticable on MP boxes under load.
> 
> The problem is that I can't get enough out of our network (with
> the URLs I hit) to see if it improves the performance.  I will
> try to adjust the URLs I hit to see if I can move the bottleneck
> back to the CPU.  
> 
> Any suggestions here?  -- justin

mod_include, PHP -- something dynamic? You just want to CPU-strap
some typical system usage, and then see what the maximum req/sec
throughput is you can get, right?

-aaron

Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Fri, Aug 24, 2001 at 10:36:11PM -0700, Ian Holsman wrote:
> BTW can flood simulate a steady # of requests being attempted per
> second?  or does it just fire up N threads/processes and whack away.
> ( check out http://www.cs.rice.edu/CS/Systems/Web-measurement/ for more
> info)

Yeah, I read that paper last night and I believe flood can simulate
the S-Client under the following syntax:

<usefarmer count="10000" startdelay="1" startcount="2">Joe</usefarmer>

Flood will start two threads every second until there are 10000 active.
I suppose we could make it open-ended, but that complicates things.  
We could also make the resolution of the XML more precise (i.e. .5 
seconds), but Aaron and I disagreed on how to do this.  =)

In order to do the S-Client as described, you must have two processes 
with a domain socket and use event-based socket operations.  I don't 
feel that is very portable or scalable.  However, their idea of
continuing to ramp up the requests regardless of the server's response
time does make sense.  -- justin


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> BTW can flood simulate a steady # of requests being attempted per
> second? 
> or does it just fire up N threads/processes and whack away.
> ( check out http://www.cs.rice.edu/CS/Systems/Web-measurement/ for more
> info)

Heh, nice coincidence -- I printed that paper out earlier today and
gave Justin a copy.

Flood has settable delays between requests with (I think) pseudo-random
adjustments.  It can also blast away, if that's what is needed for a
given test.

....Roy


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> BTW can flood simulate a steady # of requests being attempted per
> second? 
> or does it just fire up N threads/processes and whack away.
> ( check out http://www.cs.rice.edu/CS/Systems/Web-measurement/ for more
> info)

Heh, nice coincidence -- I printed that paper out earlier today and
gave Justin a copy.

Flood has settable delays between requests with (I think) pseudo-random
adjustments.  It can also blast away, if that's what is needed for a
given test.

....Roy


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Ian Holsman <Ia...@cnet.com>.
On Fri, 2001-08-24 at 17:40, Justin Erenkrantz wrote:
> On Fri, Aug 24, 2001 at 05:21:11PM -0700, Marc Slemko wrote:
> > On Fri, 24 Aug 2001, Justin Erenkrantz wrote:
> > 
instead of looking at raw network thoughput look at heavy users of
pools, CPU utilization, and truss/prof output.
and compare these results with standard pool, and with brians patch.

I would concentrate more on getting 100 rps going through in the
following conditions:

* standard file no parsing (ie .. sendfile)
* mod include with 2 includes (header/footer) (as this REALLY uses
pools heavily)
* Keep-alives

I'll fire up the 8-way box on monday and compare sanders and brians
pool code against the 'stock' pool and see what I can see.

There was also the pool replay code I wrote a while back which could be
used to compare pool allocaters, as well as dreieds memory tests for the
SMS.

BTW can flood simulate a steady # of requests being attempted per
second? 
or does it just fire up N threads/processes and whack away.
( check out http://www.cs.rice.edu/CS/Systems/Web-measurement/ for more
info)
> > > If you have the backplane (my guess would be Gigabit Ethernet) to
> > 
> > Well, or just a less powerful server.  The goal isn't the best raw
> > numbers, the goal is comparison.  Granted, that comparison can be
> > different on various sized machines (eg. SMP vs. not, etc.).
> 
> Well, part of the problem is that Sander's pool optimization is 
> intended to benefit MP boxes.  It adds thread-local free-lists - so
> the benefit is probably only noticable on MP boxes under load.
> 
> The problem is that I can't get enough out of our network (with
> the URLs I hit) to see if it improves the performance.  I will
> try to adjust the URLs I hit to see if I can move the bottleneck
> back to the CPU.  
> 
> Any suggestions here?  -- justin
-- 
Ian Holsman
Performance Measurement & Analysis
CNET Networks    -    415 364-8608

Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Ian Holsman <Ia...@cnet.com>.
On Fri, 2001-08-24 at 17:40, Justin Erenkrantz wrote:
> On Fri, Aug 24, 2001 at 05:21:11PM -0700, Marc Slemko wrote:
> > On Fri, 24 Aug 2001, Justin Erenkrantz wrote:
> > 
instead of looking at raw network thoughput look at heavy users of
pools, CPU utilization, and truss/prof output.
and compare these results with standard pool, and with brians patch.

I would concentrate more on getting 100 rps going through in the
following conditions:

* standard file no parsing (ie .. sendfile)
* mod include with 2 includes (header/footer) (as this REALLY uses
pools heavily)
* Keep-alives

I'll fire up the 8-way box on monday and compare sanders and brians
pool code against the 'stock' pool and see what I can see.

There was also the pool replay code I wrote a while back which could be
used to compare pool allocaters, as well as dreieds memory tests for the
SMS.

BTW can flood simulate a steady # of requests being attempted per
second? 
or does it just fire up N threads/processes and whack away.
( check out http://www.cs.rice.edu/CS/Systems/Web-measurement/ for more
info)
> > > If you have the backplane (my guess would be Gigabit Ethernet) to
> > 
> > Well, or just a less powerful server.  The goal isn't the best raw
> > numbers, the goal is comparison.  Granted, that comparison can be
> > different on various sized machines (eg. SMP vs. not, etc.).
> 
> Well, part of the problem is that Sander's pool optimization is 
> intended to benefit MP boxes.  It adds thread-local free-lists - so
> the benefit is probably only noticable on MP boxes under load.
> 
> The problem is that I can't get enough out of our network (with
> the URLs I hit) to see if it improves the performance.  I will
> try to adjust the URLs I hit to see if I can move the bottleneck
> back to the CPU.  
> 
> Any suggestions here?  -- justin
-- 
Ian Holsman
Performance Measurement & Analysis
CNET Networks    -    415 364-8608

Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Fri, Aug 24, 2001 at 05:21:11PM -0700, Marc Slemko wrote:
> On Fri, 24 Aug 2001, Justin Erenkrantz wrote:
> 
> > If you have the backplane (my guess would be Gigabit Ethernet) to
> 
> Well, or just a less powerful server.  The goal isn't the best raw
> numbers, the goal is comparison.  Granted, that comparison can be
> different on various sized machines (eg. SMP vs. not, etc.).

Well, part of the problem is that Sander's pool optimization is 
intended to benefit MP boxes.  It adds thread-local free-lists - so
the benefit is probably only noticable on MP boxes under load.

The problem is that I can't get enough out of our network (with
the URLs I hit) to see if it improves the performance.  I will
try to adjust the URLs I hit to see if I can move the bottleneck
back to the CPU.  

Any suggestions here?  -- justin


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Fri, Aug 24, 2001 at 05:21:11PM -0700, Marc Slemko wrote:
> On Fri, 24 Aug 2001, Justin Erenkrantz wrote:
> 
> > If you have the backplane (my guess would be Gigabit Ethernet) to
> 
> Well, or just a less powerful server.  The goal isn't the best raw
> numbers, the goal is comparison.  Granted, that comparison can be
> different on various sized machines (eg. SMP vs. not, etc.).

Well, part of the problem is that Sander's pool optimization is 
intended to benefit MP boxes.  It adds thread-local free-lists - so
the benefit is probably only noticable on MP boxes under load.

The problem is that I can't get enough out of our network (with
the URLs I hit) to see if it improves the performance.  I will
try to adjust the URLs I hit to see if I can move the bottleneck
back to the CPU.  

Any suggestions here?  -- justin


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Marc Slemko <ma...@znep.com>.
On Fri, 24 Aug 2001, Justin Erenkrantz wrote:

> If you have the backplane (my guess would be Gigabit Ethernet) to

Well, or just a less powerful server.  The goal isn't the best raw
numbers, the goal is comparison.  Granted, that comparison can be
different on various sized machines (eg. SMP vs. not, etc.).

More important than just the performance of a given workload is an
understanding of what different types of things are slower or faster, and
what types of workloads are impacted the most by the changes.  And, of
course, relating those workloads to reality.

So I guess I'm just trying to remind people not to focus too much on
numbers generated by any particular benchmark since they can be misleading
at the best, and can cause "optimizations" that are actually more
expensive in the "real world"...


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Marc Slemko <ma...@znep.com>.
On Fri, 24 Aug 2001, Justin Erenkrantz wrote:

> If you have the backplane (my guess would be Gigabit Ethernet) to

Well, or just a less powerful server.  The goal isn't the best raw
numbers, the goal is comparison.  Granted, that comparison can be
different on various sized machines (eg. SMP vs. not, etc.).

More important than just the performance of a given workload is an
understanding of what different types of things are slower or faster, and
what types of workloads are impacted the most by the changes.  And, of
course, relating those workloads to reality.

So I guess I'm just trying to remind people not to focus too much on
numbers generated by any particular benchmark since they can be misleading
at the best, and can cause "optimizations" that are actually more
expensive in the "real world"...


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sat, Aug 25, 2001 at 12:23:26AM +0200, Sander Striker wrote:
> Justin will probably follow up later with test results.

Well, I did run the tests.  However, in all cases/combinations 
(new pool/old pool, threaded/prefork MPM), our limitation is the 
network bandwidth.  We're currently pulling about 70-75Mbps 
which seems to max out our Cisco Catalyst 2940 (100BaseT) 
switches.  This leads to about ~405rps for my test.

This is a *good* sign however - in the (recent) past with Apache 
2.0, I haven't been able to max out the network bandwidth.  Roy
has suggested using smaller files/requests - I'll try that
next week.  What I'll say is that Sander's new code doesn't
crash (it is cleaner code).  As far as performance goes, I can't
really judge that with the network we have - as we can't get 
much better without a network overhaul.  =)

If you have the backplane (my guess would be Gigabit Ethernet) to
support this, I can send anyone the flood configs I was using.
They are similar to the URLs I posted earlier as a suggestion 
for an attempt at standardizing benchmarks for common httpd-2.0 
code.

For the curious, the machines we were using were both dual 
PIII/550s with 1GB of RAM running Solaris (client is Sol7, 
server is Sol8).  They were yawning throughout the test (about 
50% idle on the server).  -- justin


Re: Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sat, Aug 25, 2001 at 12:23:26AM +0200, Sander Striker wrote:
> Justin will probably follow up later with test results.

Well, I did run the tests.  However, in all cases/combinations 
(new pool/old pool, threaded/prefork MPM), our limitation is the 
network bandwidth.  We're currently pulling about 70-75Mbps 
which seems to max out our Cisco Catalyst 2940 (100BaseT) 
switches.  This leads to about ~405rps for my test.

This is a *good* sign however - in the (recent) past with Apache 
2.0, I haven't been able to max out the network bandwidth.  Roy
has suggested using smaller files/requests - I'll try that
next week.  What I'll say is that Sander's new code doesn't
crash (it is cleaner code).  As far as performance goes, I can't
really judge that with the network we have - as we can't get 
much better without a network overhaul.  =)

If you have the backplane (my guess would be Gigabit Ethernet) to
support this, I can send anyone the flood configs I was using.
They are similar to the URLs I posted earlier as a suggestion 
for an attempt at standardizing benchmarks for common httpd-2.0 
code.

For the curious, the machines we were using were both dual 
PIII/550s with 1GB of RAM running Solaris (client is Sol7, 
server is Sol8).  They were yawning throughout the test (about 
50% idle on the server).  -- justin


Pools, possible replacement, WAS: RE: [Fwd: brianp patch Quantify results]

Posted by Sander Striker <st...@apache.org>.
Hi,

As Greg pointed out, I should have posted the code here sooner
for review instead of mailing it to others in private first.
I wanted to have the obvious bugs out before I posted though.
And, I wanted to know if it would actually perform instead of
being a lot of hot air.

Anyway, attached are a replacement apr_pools.h and apr_pools.c
(yes, attached, because I use outlook...).  I would like to
provide incremental patches as Greg suggested, but it is already
midnight again and I am tired.

Justin will probably follow up later with test results.

Disclaimer and changes:  

 - The allocation scheme (therefor apr_pool_t and friends,
   apr_palloc, apr_pcalloc, apr_pool_create *), apr_pool_clear,
   apr_pool_destroy and apr_pvsprintf (and its flush function).

   *) apr_pool_create is now a macro that calls apr_pool_create_ex
      with default arguments.

 - In addition, the way we keep track of free blocks is changed
   for faster allocations.

 - There is a way to ask for a new freelist for a pool instead
   of using its parents freelist, at creation time.

 - The code lacks good comments (I'll work on that, but not
   now).

 - There is still stuff in the original apr_pools.c that needs
   merging over (and maybe some rethought).  AFAIK that is all
   debug related, the server runs well without it.

 - The includes need cleaning up. I simply copied the original
   includes and that seems a bit too much.  IIRC it doesn't
   use APR_WANTxxx either.

 - The apr_pools.h file has some simple mods.  Instead it should
   be cleaned up.


Sander


Re: [Fwd: brianp patch Quantify results] (was thread-specific free listfor pools" patch )

Posted by Greg Stein <gs...@lyra.org>.
On Fri, Aug 24, 2001 at 01:58:02PM -0700, Justin Erenkrantz wrote:
> On Fri, Aug 24, 2001 at 01:48:52PM -0700, Ian Holsman wrote:
> > sander,
> > could your and brian's patches be merged, or do they both basiclaly
> > do the same thing.
> 
> I think they do essentially the same thing.  Sander's patch is a 
> rewrite of the pool code with improvements thrown in.  I'm 
> testing/hammering on it now.  It does seem to be a definite win
> from early testing though.  -- justin

If code is destined for the Apache tree, then it would be best to post that
code publicly, rather than sent privately to other people for testing.

Maybe nobody will do anything with it, but it gives them a chance to try it
and to review it. Behind the scenes work is detrimental to the group effort.

-g

-- 
Greg Stein, http://www.lyra.org/

Re: [Fwd: brianp patch Quantify results] (was thread-specific free listfor pools" patch )

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Fri, Aug 24, 2001 at 01:48:52PM -0700, Ian Holsman wrote:
> sander,
> could your and brian's patches be merged, or do they both basiclaly
> do the same thing.

I think they do essentially the same thing.  Sander's patch is a 
rewrite of the pool code with improvements thrown in.  I'm 
testing/hammering on it now.  It does seem to be a definite win
from early testing though.  -- justin


Re: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Greg Stein <gs...@lyra.org>.
On Fri, Aug 24, 2001 at 11:09:13PM +0200, Sander Striker wrote:
>...
> Mine takes it a bit further.  It essentially makes the freelist more
> efficient.  Adds the possibility to request a new freelist for a pool,
> which can optionally have locking.

Cool.

> Furthermore it resolves some
> issues with possible segfaults in the current pools code when running
> out of memory.

Not necessary :-)

> However, there is a downside.  I started with an empty slate and then
> started to add things in.  I still haven't merged some things over
> (the APR_POOL_DEBUG mode, USE_MALLOC and other fancy debug stuff).  Also,
> my code isn't documented very well at the moment (OTOH, the current
> pools code isn't exactly a quick read with all those #if[def] lines...).

For the pools code, it can only be patched. It is unacceptable to toss a
completely written-from-scratch replacement into the code base. If it takes
a sequence of 20 patches to reach the written-from-scratch stage, then
fine... but that means each step has been reviewable as you go along.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: [patch] apxs

Posted by Ryan Bloom <rb...@covalent.net>.
Committed.  Thanks.

Ryan

On Monday 27 August 2001 03:03, Stas Bekman wrote:
> I've started from the point where apxs was failing with the error:
>
> /home/stas/httpd-2.0/bin/apxs
> apxs:Error: Invalid query string `SHLTCFLAGS'
>
> because my httpd-2.0/build/config_vars.mk had:
> SHLTCFLAGS =
>
> I was fixing it and while doing that I've refactored some of the code:
>
> So this patch
> - allows empty $val from httpd-2.0/build/config_vars.mk
> - make code more perlish.
> - read and parse the config file only once
> - use a hash instead of array for the list of internal config vars to
>   test against
> - added -h option (just print usage)
> - wrapped numerous print STDERR calls into a simpler error() and
>   notice() subs
> - simplified some of the logic in if's using perl constructs.
>
> I didn't know how to extensively test whether I didn't break anything.
>
> BTW, according to the apxs manpage, the following command
> % /home/stas/httpd-2.0/bin/apxs -q -S PREFIX=bar INCLUDEDIR
> should print:
>
> /bar/include
>
> while it prints:
>
> /home/stas/httpd-2.0/include
>
> which seems to be wrong. This happens in the current apxs version, my
> patch doesn't change it.
>

______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

[patch] apxs

Posted by Stas Bekman <st...@stason.org>.
I've started from the point where apxs was failing with the error:

/home/stas/httpd-2.0/bin/apxs
apxs:Error: Invalid query string `SHLTCFLAGS'

because my httpd-2.0/build/config_vars.mk had:
SHLTCFLAGS =

I was fixing it and while doing that I've refactored some of the code:

So this patch
- allows empty $val from httpd-2.0/build/config_vars.mk
- make code more perlish.
- read and parse the config file only once
- use a hash instead of array for the list of internal config vars to
  test against
- added -h option (just print usage)
- wrapped numerous print STDERR calls into a simpler error() and
  notice() subs
- simplified some of the logic in if's using perl constructs.

I didn't know how to extensively test whether I didn't break anything.

BTW, according to the apxs manpage, the following command
% /home/stas/httpd-2.0/bin/apxs -q -S PREFIX=bar INCLUDEDIR
should print:

/bar/include

while it prints:

/home/stas/httpd-2.0/include

which seems to be wrong. This happens in the current apxs version, my
patch doesn't change it.


Index: apxs.in
===================================================================
RCS file: /home/cvspublic/httpd-2.0/support/apxs.in,v
retrieving revision 1.23
diff -u -r1.23 apxs.in
--- apxs.in	2001/08/26 16:28:19	1.23
+++ apxs.in	2001/08/27 09:56:52
@@ -1,4 +1,4 @@
-#!@perlbin@
+#!@perlbin@ -w
 # ====================================================================
 # The Apache Software License, Version 1.1
 #
@@ -62,7 +62,12 @@
 ##

 my $prefix         = "@prefix@";
-my $CFG_PREFIX     = "$prefix";
+my $CFG_PREFIX     = $prefix;
+
+# read the configuration variables once
+my %config_vars = ();
+get_config_vars("$prefix/build/config_vars.mk",\%config_vars);
+
 my $exec_prefix    = get_vars("exec_prefix");
 my $CFG_TARGET     = get_vars("progname");
 my $CFG_SYSCONFDIR = get_vars("sysconfdir");
@@ -72,9 +77,13 @@
 my $CFG_CC         = get_vars("CC");
 my $libexecdir     = get_vars("libexecdir");
 my $CFG_LIBEXECDIR = eval qq("$libexecdir");
-my $bindir        = get_vars("bindir");
+my $bindir         = get_vars("bindir");
 my $CFG_SBINDIR    = eval qq("$bindir");

+my %internal_vars = map {$_ => 1}
+    qw(TARGET CC CFLAGS CFLAGS_SHLIB LD_SHLIB LDFLAGS_SHLIB LIBS_SHLIB
+       PREFIX SBINDIR INCLUDEDIR LIBEXECDIR SYSCONFDIR);
+
 ##
 ##  parse argument line
 ##
@@ -95,43 +104,43 @@
 my $opt_a = 0;
 my $opt_A = 0;
 my $opt_q = 0;
+my $opt_h = 0;

 #   this subroutine is derived from Perl's getopts.pl with the enhancement of
 #   the "+" metacharater at the format string to allow a list to be build by
 #   subsequent occurance of the same option.
 sub Getopts {
     my ($argumentative, @ARGV) = @_;
-    my (@args, $first, $rest, $pos);
-    my ($errs) = 0;
-    local ($_);
-    local ($[) = 0;
-
-    @args = split( / */, $argumentative);
-    while(@ARGV && ($_ = $ARGV[0]) =~ /^-(.)(.*)/) {
-        ($first, $rest) = ($1,$2);
+    my $errs = 0;
+    local $_;
+    local $[ = 0;
+
+    my @args = split / */, $argumentative;
+    while (@ARGV && ($_ = $ARGV[0]) =~ /^-(.)(.*)/) {
+        my ($first, $rest) = ($1,$2);
         if ($_ =~ m|^--$|) {
-            shift(@ARGV);
+            shift @ARGV;
             last;
         }
-        $pos = index($argumentative,$first);
-        if($pos >= $[) {
-            if($args[$pos+1] eq ':') {
-                shift(@ARGV);
-                if($rest eq '') {
+        my $pos = index($argumentative,$first);
+        if ($pos >= $[) {
+            if ($args[$pos+1] eq ':') {
+                shift @ARGV;
+                if ($rest eq '') {
                     unless (@ARGV) {
-                        print STDERR "apxs:Error: Incomplete option: $first (needs an argument)\n";
-                        ++$errs;
+                        error("Incomplete option: $first (needs an argument)");
+                        $errs++;
                     }
                     $rest = shift(@ARGV);
                 }
                 eval "\$opt_$first = \$rest;";
             }
             elsif ($args[$pos+1] eq '+') {
-                shift(@ARGV);
-                if($rest eq '') {
+                shift @ARGV;
+                if ($rest eq '') {
                     unless (@ARGV) {
-                        print STDERR "apxs:Error: Incomplete option: $first (needs an argument)\n";
-                        ++$errs;
+                        error("Incomplete option: $first (needs an argument)");
+                        $errs++;
                     }
                     $rest = shift(@ARGV);
                 }
@@ -139,7 +148,7 @@
             }
             else {
                 eval "\$opt_$first = 1";
-                if($rest eq '') {
+                if ($rest eq '') {
                     shift(@ARGV);
                 }
                 else {
@@ -148,9 +157,9 @@
             }
         }
         else {
-            print STDERR "apxs:Error: Unknown option: $first\n";
-            ++$errs;
-            if($rest ne '') {
+            error("Unknown option: $first");
+            $errs++;
+            if ($rest ne '') {
                 $ARGV[0] = "-$rest";
             }
             else {
@@ -162,45 +171,47 @@
 }

 sub usage {
-    print STDERR "Usage: apxs -g [-S <var>=<val>] -n <modname>\n";
-    print STDERR "       apxs -q [-S <var>=<val>] <query> ...\n";
-    print STDERR "       apxs -c [-S <var>=<val>] [-o <dsofile>] [-D <name>[=<value>]]\n";
-    print STDERR "               [-I <incdir>] [-L <libdir>] [-l <libname>] [-Wc,<flags>]\n";
-    print STDERR "               [-Wl,<flags>] <files> ...\n";
-    print STDERR "       apxs -i [-S <var>=<val>] [-a] [-A] [-n <modname>] <dsofile> ...\n";
-    print STDERR "       apxs -e [-S <var>=<val>] [-a] [-A] [-n <modname>] <dsofile> ...\n";
-    exit(1);
+    print STDERR <<"__USAGE__";
+Usage: apxs -g [-S <var>=<val>] -n <modname>
+       apxs -q [-S <var>=<val>] <query> ...
+       apxs -c [-S <var>=<val>] [-o <dsofile>] [-D <name>[=<value>]]
+               [-I <incdir>] [-L <libdir>] [-l <libname>] [-Wc,<flags>]
+               [-Wl,<flags>] <files> ...
+       apxs -i [-S <var>=<val>] [-a] [-A] [-n <modname>] <dsofile> ...
+       apxs -e [-S <var>=<val>] [-a] [-A] [-n <modname>] <dsofile> ...
+       apxs -h
+__USAGE__
+    exit 1;
 }

 #   option handling
-my $rc;
-($rc, @ARGV) = &Getopts("qn:gco:I+D+L+l+W+S+eiaA", @ARGV);
-&usage if ($rc == 0);
-&usage if ($#ARGV == -1 and not $opt_g);
-&usage if (not $opt_q and not ($opt_g and $opt_n) and not $opt_i and not $opt_c and not $opt_e);
+my ($rc, @args) = Getopts("qhn:gco:I+D+L+l+W+S+eiaA", @ARGV);
+
+usage() if $opt_h;
+usage() unless $rc;
+usage() unless scalar @args or $opt_g;
+usage() unless $opt_q or ($opt_g and $opt_n) or $opt_i or $opt_c or $opt_e;

 #   argument handling
-my @args = @ARGV;
 my $name = 'unknown';
-$name = $opt_n if ($opt_n ne '');
+$name = $opt_n if $opt_n ne '';

 if (@opt_S) {
-    my ($opt_S);
+    my $opt_S;
     foreach $opt_S (@opt_S) {
 	if ($opt_S =~ m/^([^=]+)=(.*)$/) {
-	    my ($var) = $1;
-	    my ($val) = $2;
-	    my $oldval = eval "\$CFG_$var";
-
-	    unless ($var and $oldval) {
-		print STDERR "apxs:Error: no config variable $var\n";
-		&usage;
+            my ($var) = $1;
+            my ($val) = $2;
+            my $oldval = eval "\$CFG_$var";
+
+            unless ($oldval) {
+		error("no config variable CFG_$var");
+		usage();
 	    }
-
-	    eval "\$CFG_${var}=\"${val}\"";
+            eval "\$CFG_${var}=\"${val}\"";
 	} else {
-	    print STDERR "apxs:Error: malformatted -S option\n";
-	    &usage;
+	    error("malformatted -S option");
+	    usage();
 	}
     }
 }
@@ -208,68 +219,70 @@
 ##
 ##  Initial shared object support check
 ##
-my $exec_prefix = get_vars("exec_prefix");
 my $httpd = get_vars("bindir") . "/" . get_vars("progname");
-my $temp = eval qq("$httpd");
-my $httpd = eval qq("$temp");
+$httpd = eval qq("$httpd");
+$httpd = eval qq("$httpd");

 #allow apxs to be run from the source tree, before installation
 if ($0 =~ m:support/apxs$:) {
     ($httpd = $0) =~ s:support/apxs$::;
 }

-if (not -x "$httpd") {
-	print STDERR "apxs:Error: $httpd not found or not executable\n";
-	exit(1);
-}
-if (not grep(/mod_so/, `$httpd -l`)) {
-    print STDERR "apxs:Error: Sorry, no shared object support for Apache\n";
-    print STDERR "apxs:Error: available under your platform. Make sure\n";
-    print STDERR "apxs:Error: the Apache module mod_so is compiled into\n";
-    print STDERR "apxs:Error: your server binary `$httpd'.\n";
-    exit(1);
+unless (-x "$httpd") {
+	error("$httpd not found or not executable");
+	exit 1;
 }

+unless (grep /mod_so/, `$httpd -l`) {
+    error("Sorry, no shared object support for Apache");
+    error("available under your platform. Make sure");
+    error("the Apache module mod_so is compiled into");
+    error("your server binary `$httpd'.");
+    exit 1;
+}
+
+sub get_config_vars{
+    my ($file, $rh_config) = @_;
+
+    open IN, $file or die "cannot open $file: $!";
+    while (<IN>){
+        if (/^\s*(.*?)\s*=\s*(.*)$/){
+            $rh_config->{$1} = $2;
+        }
+    }
+    close IN;
+}
+
 sub get_vars {
     my $result = '';
-    my $arg;
     my $ok = 0;
+    my $arg;
     foreach $arg (@_) {
-        open IN, "$prefix/build/config_vars.mk" or die "open $prefix/build/config_vars.mk: $!";
-        while (<IN>) {
-            my $var;
-            my $val;
-            if (/(.*) = (.*)$/) {
-                $var = $1;
-                $val = $2;
-            }
-            next unless $var;
-            if ($arg eq $var or $arg eq lc($var)) {
-                $result .= "$val;;";
-                $ok = 1;
-                last;
-            }
+        if (exists $config_vars{$arg} or exists $config_vars{lc $arg}) {
+            my $val = exists $config_vars{$arg}
+                ? $config_vars{$arg}
+                : $config_vars{lc $arg};
+            $result .= eval qq("$val");
+            $result .= ";;";
+            $ok = 1;
         }
         if (not $ok) {
-            foreach $name (qw(
-                TARGET CC CFLAGS CFLAGS_SHLIB LD_SHLIB LDFLAGS_SHLIB LIBS_SHLIB
-                PREFIX SBINDIR INCLUDEDIR LIBEXECDIR SYSCONFDIR
-                )) {
-                if ($arg eq $name or $arg eq lc($name)) {
-                    my $val = eval "\$CFG_$name";
-                    $result .= eval qq("${val}") . ";;";
-                    $ok = 1;
-                }
+            if (exists $internal_vars{$arg} or exists $internal_vars{lc $arg}) {
+                my $val = exists $internal_vars{$arg} ? $arg : lc $arg;
+                $val = eval "\$CFG_$val";
+                $result .= eval qq("$val");
+                $result .= ";;";
+                $ok = 1;
             }
             if (not $ok) {
-                printf(STDERR "apxs:Error: Invalid query string `%s'\n", $arg);
+                error("Invalid query string `$arg'");
                 exit(1);
             }
         }
     }
     $result =~ s|;;$||;
     $result =~ s|:| |;
-    return("$result");
+    return $result;
 }

 ##
@@ -283,11 +296,11 @@
     my ($cmd, $rc);

     foreach $cmd (@cmds) {
-        print STDERR "$cmd\n";
-        $rc = system("$cmd");
-        if ($rc != 0) {
-            printf(STDERR "apxs:Break: Command failed with rc=%d\n", $rc << 8);
-            exit(1);
+        notice($cmd);
+        $rc = system $cmd;
+        if ($rc) {
+            error(sprintf "Command failed with rc=%d\n", $rc << 8);
+            exit 1 ;
         }
     }
 }
@@ -298,7 +311,7 @@
     ##

     if (-d $name) {
-        print STDERR "apxs:Error: Directory `$name' already exists. Remove first\n";
+        error("Directory `$name' already exists. Remove first");
         exit(1);
     }

@@ -308,21 +321,21 @@

     my ($mkf, $mods, $src) = ($data =~ m|^(.+)-=#=-\n(.+)-=#=-\n(.+)|s);

-    print STDERR "Creating [DIR]  $name\n";
+    notice("Creating [DIR]  $name");
     system("mkdir $name");
-    print STDERR "Creating [FILE] $name/Makefile\n";
+    notice("Creating [FILE] $name/Makefile");
     open(FP, ">${name}/Makefile") || die;
     print FP $mkf;
     close(FP);
-    print STDERR "Creating [FILE] $name/modules.mk\n";
+    notice("Creating [FILE] $name/modules.mk");
     open(FP, ">${name}/modules.mk") || die;
     print FP $mods;
     close(FP);
-    print STDERR "Creating [FILE] $name/mod_$name.c\n";
+    notice("Creating [FILE] $name/mod_$name.c");
     open(FP, ">${name}/mod_${name}.c") || die;
     print FP $src;
     close(FP);
-    print STDERR "Creating [FILE] $name/.deps\n";
+    notice("Creating [FILE] $name/.deps");
     system("touch ${name}/.deps");

     exit(0);
@@ -448,7 +461,7 @@
     my $f;
     foreach $f (@args) {
         if ($f !~ m#(\.so$|\.la$)#) {
-            print STDERR "apxs:Error: file $f is not a shared object\n";
+            error("file $f is not a shared object");
             exit(1);
         }
         my $t = $f;
@@ -482,8 +495,8 @@
                 }
             }
             if ($name eq '') {
-                print "apxs:Error: Sorry, cannot determine bootstrap symbol name\n";
-                print "apxs:Error: Please specify one with option `-n'\n";
+                error("Sorry, cannot determine bootstrap symbol name");
+                error("Please specify one with option `-n'");
                 exit(1);
             }
         }
@@ -504,7 +517,7 @@
     #   activate module via LoadModule/AddModule directive
     if ($opt_a or $opt_A) {
         if (not -f "$CFG_SYSCONFDIR/$CFG_TARGET.conf") {
-            print "apxs:Error: Config file $CFG_SYSCONFDIR/$CFG_TARGET.conf not found\n";
+            error("Config file $CFG_SYSCONFDIR/$CFG_TARGET.conf not found");
             exit(1);
         }

@@ -514,8 +527,8 @@
         close(FP);

         if ($content !~ m|\n#?\s*LoadModule\s+|) {
-            print STDERR "apxs:Error: Activation failed for custom $CFG_SYSCONFDIR/$CFG_TARGET.conf file.\n";
-            print STDERR "apxs:Error: At least one `LoadModule' directive already has to exist.\n";
+            error("Activation failed for custom $CFG_SYSCONFDIR/$CFG_TARGET.conf file.");
+            error("At least one `LoadModule' directive already has to exist.");
             exit(1);
         }

@@ -530,7 +543,7 @@
                  $content =~ s|^(.*\n)#?\s*$lmd[^\n]*\n|$1$c$lmd\n|sg;
             }
             $lmd =~ m|LoadModule\s+(.+?)_module.*|;
-            print STDERR "[$what module `$1' in $CFG_SYSCONFDIR/$CFG_TARGET.conf]\n";
+            notice("[$what module `$1' in $CFG_SYSCONFDIR/$CFG_TARGET.conf]");
         }
         my $amd;
         foreach $amd (@amd) {
@@ -548,10 +561,18 @@
                        "cp $CFG_SYSCONFDIR/$CFG_TARGET.conf.new $CFG_SYSCONFDIR/$CFG_TARGET.conf && " .
                        "rm $CFG_SYSCONFDIR/$CFG_TARGET.conf.new");
             } else {
-                print STDERR "unable to open configuration file\n";
+                notice("unable to open configuration file");
             }
 	}
     }
+}
+
+sub error{
+    print STDERR "apxs:Error: $_[0].\n";
+}
+
+sub notice{
+    print STDERR "$_[0]\n";
 }

 ##EOF##

_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://localhost/      http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



RE: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Sander Striker <st...@apache.org>.
> On Fri, 2001-08-24 at 13:50, Sander Striker wrote:
> > Please hlod off for a day.  I've done some pools coding
> > too again (and hopefully I can send in a patch tonight).
> > It should have even better performance.  Just ironing it
> > out.
> > 
> > Thanks,
> > 
> > Sander
> > 
> sander,
> could your and brian's patches be merged, or do they both basiclaly
> do the same thing.

Mine takes it a bit further.  It essentially makes the freelist more
efficient.  Adds the possibility to request a new freelist for a pool,
which can optionally have locking.  Furthermore it resolves some
issues with possible segfaults in the current pools code when running
out of memory.

However, there is a downside.  I started with an empty slate and then
started to add things in.  I still haven't merged some things over
(the APR_POOL_DEBUG mode, USE_MALLOC and other fancy debug stuff).  Also,
my code isn't documented very well at the moment (OTOH, the current
pools code isn't exactly a quick read with all those #if[def] lines...).

Sander


> > 
> > > -----Original Message-----
> > > From: Ian Holsman [mailto:ianh@cnet.com]
> > > Sent: 24 August 2001 21:14
> > > To: dev@httpd.apache.org
> > > Subject: [Fwd: brianp patch Quantify results] (was 
> thread-specific free
> > > listfor pools" patch )
> > > 
> > > 
> > > One of our other developers ran Brian Pane's 
> > > thread-specific free list for pools patch (posted ~1 week ago)
> > > 
> > > 
> > > 
> > > here are his results.
> > > ...Ian
> > > 
> > > -----Forwarded Message-----
> > > From: Blaise Tarr <XXXXXXXXXX>
> > > To: pxi-webserver@cnet.com
> > > Subject: brianp patch Quantify results
> > > 
> > > 
> > > Hey,
> > > 
> > > For the baseline I used the CVS version from yesterday (8/3) morning.
> > > Then I applied Brian's "thread-specific free list for pools" patch.
> > > 
> > > I used these configs:
> > >     StartServers         1 
> > >     MaxClients           1 
> > >     MinSpareThreads      5 
> > >     MaxSpareThreads     10 
> > >     ThreadsPerChild     25 
> > > 
> > > For the test, I requested 500 news.com pages that have 2 virtual
> > > includes.  The pages were copies of the same file but had different
> > > names.  (lynx -source http://hungry.cnet.com/2file/00${i}.html)
> > > 
> > > handle_include + descendants were 9.5% faster with Brian's patch, and
> > > accounted for 5.89% of the total time, as opposed to 6.28% of the
> > > total time for the baseline.  Overall, Brian's patch reduced the
> > > number of cycles by 3.74%.
> > > 
> > > Now, I must add that these are Quantify numbers, not real world
> > > numbers. 
> > > So, what's next?
> > > 
> > > -- 
> > > Blaise Tarr
> > > XXXXXXXXXXXXXXX         CNET.com
> > > 908.541.3771            The source for computers and technology.
> > > 
> > > 
> > > 
> > > 
> -- 
> Ian Holsman          IanH@cnet.com
> Performance Measurement & Analysis
> CNET Networks   -   (415) 364-8608
> 
> 
> 
> 

RE: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Sander Striker <st...@apache.org>.
> On Fri, 2001-08-24 at 13:50, Sander Striker wrote:
> > Please hlod off for a day.  I've done some pools coding
> > too again (and hopefully I can send in a patch tonight).
> > It should have even better performance.  Just ironing it
> > out.
> > 
> > Thanks,
> > 
> > Sander
> > 
> sander,
> could your and brian's patches be merged, or do they both basiclaly
> do the same thing.

Mine takes it a bit further.  It essentially makes the freelist more
efficient.  Adds the possibility to request a new freelist for a pool,
which can optionally have locking.  Furthermore it resolves some
issues with possible segfaults in the current pools code when running
out of memory.

However, there is a downside.  I started with an empty slate and then
started to add things in.  I still haven't merged some things over
(the APR_POOL_DEBUG mode, USE_MALLOC and other fancy debug stuff).  Also,
my code isn't documented very well at the moment (OTOH, the current
pools code isn't exactly a quick read with all those #if[def] lines...).

Sander


> > 
> > > -----Original Message-----
> > > From: Ian Holsman [mailto:ianh@cnet.com]
> > > Sent: 24 August 2001 21:14
> > > To: dev@httpd.apache.org
> > > Subject: [Fwd: brianp patch Quantify results] (was 
> thread-specific free
> > > listfor pools" patch )
> > > 
> > > 
> > > One of our other developers ran Brian Pane's 
> > > thread-specific free list for pools patch (posted ~1 week ago)
> > > 
> > > 
> > > 
> > > here are his results.
> > > ...Ian
> > > 
> > > -----Forwarded Message-----
> > > From: Blaise Tarr <XXXXXXXXXX>
> > > To: pxi-webserver@cnet.com
> > > Subject: brianp patch Quantify results
> > > 
> > > 
> > > Hey,
> > > 
> > > For the baseline I used the CVS version from yesterday (8/3) morning.
> > > Then I applied Brian's "thread-specific free list for pools" patch.
> > > 
> > > I used these configs:
> > >     StartServers         1 
> > >     MaxClients           1 
> > >     MinSpareThreads      5 
> > >     MaxSpareThreads     10 
> > >     ThreadsPerChild     25 
> > > 
> > > For the test, I requested 500 news.com pages that have 2 virtual
> > > includes.  The pages were copies of the same file but had different
> > > names.  (lynx -source http://hungry.cnet.com/2file/00${i}.html)
> > > 
> > > handle_include + descendants were 9.5% faster with Brian's patch, and
> > > accounted for 5.89% of the total time, as opposed to 6.28% of the
> > > total time for the baseline.  Overall, Brian's patch reduced the
> > > number of cycles by 3.74%.
> > > 
> > > Now, I must add that these are Quantify numbers, not real world
> > > numbers. 
> > > So, what's next?
> > > 
> > > -- 
> > > Blaise Tarr
> > > XXXXXXXXXXXXXXX         CNET.com
> > > 908.541.3771            The source for computers and technology.
> > > 
> > > 
> > > 
> > > 
> -- 
> Ian Holsman          IanH@cnet.com
> Performance Measurement & Analysis
> CNET Networks   -   (415) 364-8608
> 
> 
> 
> 

RE: [Fwd: brianp patch Quantify results] (was thread-specific free listfor pools" patch )

Posted by Ian Holsman <ia...@cnet.com>.
On Fri, 2001-08-24 at 13:50, Sander Striker wrote:
> Please hlod off for a day.  I've done some pools coding
> too again (and hopefully I can send in a patch tonight).
> It should have even better performance.  Just ironing it
> out.
> 
> Thanks,
> 
> Sander
> 
sander,
could your and brian's patches be merged, or do they both basiclaly
do the same thing.
> 
> > -----Original Message-----
> > From: Ian Holsman [mailto:ianh@cnet.com]
> > Sent: 24 August 2001 21:14
> > To: dev@httpd.apache.org
> > Subject: [Fwd: brianp patch Quantify results] (was thread-specific free
> > listfor pools" patch )
> > 
> > 
> > One of our other developers ran Brian Pane's 
> > thread-specific free list for pools patch (posted ~1 week ago)
> > 
> > 
> > 
> > here are his results.
> > ...Ian
> > 
> > -----Forwarded Message-----
> > From: Blaise Tarr <XXXXXXXXXX>
> > To: pxi-webserver@cnet.com
> > Subject: brianp patch Quantify results
> > 
> > 
> > Hey,
> > 
> > For the baseline I used the CVS version from yesterday (8/3) morning.
> > Then I applied Brian's "thread-specific free list for pools" patch.
> > 
> > I used these configs:
> >     StartServers         1 
> >     MaxClients           1 
> >     MinSpareThreads      5 
> >     MaxSpareThreads     10 
> >     ThreadsPerChild     25 
> > 
> > For the test, I requested 500 news.com pages that have 2 virtual
> > includes.  The pages were copies of the same file but had different
> > names.  (lynx -source http://hungry.cnet.com/2file/00${i}.html)
> > 
> > handle_include + descendants were 9.5% faster with Brian's patch, and
> > accounted for 5.89% of the total time, as opposed to 6.28% of the
> > total time for the baseline.  Overall, Brian's patch reduced the
> > number of cycles by 3.74%.
> > 
> > Now, I must add that these are Quantify numbers, not real world
> > numbers. 
> > So, what's next?
> > 
> > -- 
> > Blaise Tarr
> > XXXXXXXXXXXXXXX         CNET.com
> > 908.541.3771            The source for computers and technology.
> > 
> > 
> > 
> > 
-- 
Ian Holsman          IanH@cnet.com
Performance Measurement & Analysis
CNET Networks   -   (415) 364-8608


RE: [Fwd: brianp patch Quantify results] (was thread-specific free listfor pools" patch )

Posted by Ian Holsman <ia...@cnet.com>.
On Fri, 2001-08-24 at 13:50, Sander Striker wrote:
> Please hlod off for a day.  I've done some pools coding
> too again (and hopefully I can send in a patch tonight).
> It should have even better performance.  Just ironing it
> out.
> 
> Thanks,
> 
> Sander
> 
sander,
could your and brian's patches be merged, or do they both basiclaly
do the same thing.
> 
> > -----Original Message-----
> > From: Ian Holsman [mailto:ianh@cnet.com]
> > Sent: 24 August 2001 21:14
> > To: dev@httpd.apache.org
> > Subject: [Fwd: brianp patch Quantify results] (was thread-specific free
> > listfor pools" patch )
> > 
> > 
> > One of our other developers ran Brian Pane's 
> > thread-specific free list for pools patch (posted ~1 week ago)
> > 
> > 
> > 
> > here are his results.
> > ...Ian
> > 
> > -----Forwarded Message-----
> > From: Blaise Tarr <XXXXXXXXXX>
> > To: pxi-webserver@cnet.com
> > Subject: brianp patch Quantify results
> > 
> > 
> > Hey,
> > 
> > For the baseline I used the CVS version from yesterday (8/3) morning.
> > Then I applied Brian's "thread-specific free list for pools" patch.
> > 
> > I used these configs:
> >     StartServers         1 
> >     MaxClients           1 
> >     MinSpareThreads      5 
> >     MaxSpareThreads     10 
> >     ThreadsPerChild     25 
> > 
> > For the test, I requested 500 news.com pages that have 2 virtual
> > includes.  The pages were copies of the same file but had different
> > names.  (lynx -source http://hungry.cnet.com/2file/00${i}.html)
> > 
> > handle_include + descendants were 9.5% faster with Brian's patch, and
> > accounted for 5.89% of the total time, as opposed to 6.28% of the
> > total time for the baseline.  Overall, Brian's patch reduced the
> > number of cycles by 3.74%.
> > 
> > Now, I must add that these are Quantify numbers, not real world
> > numbers. 
> > So, what's next?
> > 
> > -- 
> > Blaise Tarr
> > XXXXXXXXXXXXXXX         CNET.com
> > 908.541.3771            The source for computers and technology.
> > 
> > 
> > 
> > 
-- 
Ian Holsman          IanH@cnet.com
Performance Measurement & Analysis
CNET Networks   -   (415) 364-8608


RE: [Fwd: brianp patch Quantify results] (was thread-specific free listfor pools" patch )

Posted by Sander Striker <st...@apache.org>.
Please hlod off for a day.  I've done some pools coding
too again (and hopefully I can send in a patch tonight).
It should have even better performance.  Just ironing it
out.

Thanks,

Sander


> -----Original Message-----
> From: Ian Holsman [mailto:ianh@cnet.com]
> Sent: 24 August 2001 21:14
> To: dev@httpd.apache.org
> Subject: [Fwd: brianp patch Quantify results] (was thread-specific free
> listfor pools" patch )
> 
> 
> One of our other developers ran Brian Pane's 
> thread-specific free list for pools patch (posted ~1 week ago)
> 
> 
> 
> here are his results.
> ...Ian
> 
> -----Forwarded Message-----
> From: Blaise Tarr <XXXXXXXXXX>
> To: pxi-webserver@cnet.com
> Subject: brianp patch Quantify results
> 
> 
> Hey,
> 
> For the baseline I used the CVS version from yesterday (8/3) morning.
> Then I applied Brian's "thread-specific free list for pools" patch.
> 
> I used these configs:
>     StartServers         1 
>     MaxClients           1 
>     MinSpareThreads      5 
>     MaxSpareThreads     10 
>     ThreadsPerChild     25 
> 
> For the test, I requested 500 news.com pages that have 2 virtual
> includes.  The pages were copies of the same file but had different
> names.  (lynx -source http://hungry.cnet.com/2file/00${i}.html)
> 
> handle_include + descendants were 9.5% faster with Brian's patch, and
> accounted for 5.89% of the total time, as opposed to 6.28% of the
> total time for the baseline.  Overall, Brian's patch reduced the
> number of cycles by 3.74%.
> 
> Now, I must add that these are Quantify numbers, not real world
> numbers. 
> So, what's next?
> 
> -- 
> Blaise Tarr
> XXXXXXXXXXXXXXX         CNET.com
> 908.541.3771            The source for computers and technology.
> 
> 
> 
> 

RE: [Fwd: brianp patch Quantify results] (was thread-specific free listfor pools" patch )

Posted by Sander Striker <st...@apache.org>.
Please hlod off for a day.  I've done some pools coding
too again (and hopefully I can send in a patch tonight).
It should have even better performance.  Just ironing it
out.

Thanks,

Sander


> -----Original Message-----
> From: Ian Holsman [mailto:ianh@cnet.com]
> Sent: 24 August 2001 21:14
> To: dev@httpd.apache.org
> Subject: [Fwd: brianp patch Quantify results] (was thread-specific free
> listfor pools" patch )
> 
> 
> One of our other developers ran Brian Pane's 
> thread-specific free list for pools patch (posted ~1 week ago)
> 
> 
> 
> here are his results.
> ...Ian
> 
> -----Forwarded Message-----
> From: Blaise Tarr <XXXXXXXXXX>
> To: pxi-webserver@cnet.com
> Subject: brianp patch Quantify results
> 
> 
> Hey,
> 
> For the baseline I used the CVS version from yesterday (8/3) morning.
> Then I applied Brian's "thread-specific free list for pools" patch.
> 
> I used these configs:
>     StartServers         1 
>     MaxClients           1 
>     MinSpareThreads      5 
>     MaxSpareThreads     10 
>     ThreadsPerChild     25 
> 
> For the test, I requested 500 news.com pages that have 2 virtual
> includes.  The pages were copies of the same file but had different
> names.  (lynx -source http://hungry.cnet.com/2file/00${i}.html)
> 
> handle_include + descendants were 9.5% faster with Brian's patch, and
> accounted for 5.89% of the total time, as opposed to 6.28% of the
> total time for the baseline.  Overall, Brian's patch reduced the
> number of cycles by 3.74%.
> 
> Now, I must add that these are Quantify numbers, not real world
> numbers. 
> So, what's next?
> 
> -- 
> Blaise Tarr
> XXXXXXXXXXXXXXX         CNET.com
> 908.541.3771            The source for computers and technology.
> 
> 
> 
>