You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Joe Orton <jo...@redhat.com> on 2008/04/14 21:43:57 UTC

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

On Sat, Apr 12, 2008 at 08:42:59AM -0000, Mladen Turk wrote:
> Author: mturk
> Date: Sat Apr 12 01:42:51 2008
> New Revision: 647394
> 
> URL: http://svn.apache.org/viewvc?rev=647394&view=rev
> Log:
> Introduce apr_pool_sys_allocator_set

Why can't you just use a custom allocator for whatever the problem is 
here?  That's what the allocator abstraction is *for*, right?  Adding 
yet another abstraction above the allocator seems really wrong; 
especially since it introduces a bunch of global state and *everybody* 
suffers the overhead of the additional function calls and conditionals.

joe

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Mladen Turk <mt...@apache.org>.
William A. Rowe, Jr. wrote:
> Ruediger Pluem wrote:
>>
>> But now that we have branched 1.3.0 we have to live with it until 2.0, 
>> right?
>> Maybe branching just 48 hours after these commits wasn't such a good idea
>> especially with these 48 hours being a weekend after ApacheCon.
> 
> We have *branched* 1.3.x, we have not *tagged* 1.3.0.
> 
> I'm going to hold off several days until the 1.3.0 api is settled, there's
> obvious issues for some of the new features. 

Think I'll revert the apr_pool_sys_allocator_set both from
1.3.x branch and from the head.

I admit it was done in a hurry so both API and
intended usage is not polished. The proper memory profiling
and custom system allocation API needs to be done much
better then my patch does.
I'll continue development in head, but probably by using
compile time define APR_POOL_PROFILE (similar to APR_POOL_DEBUG)

Regards
--
(TM)

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ruediger Pluem wrote:
> 
> But now that we have branched 1.3.0 we have to live with it until 2.0, 
> right?
> Maybe branching just 48 hours after these commits wasn't such a good idea
> especially with these 48 hours being a weekend after ApacheCon.

We have *branched* 1.3.x, we have not *tagged* 1.3.0.

I'm going to hold off several days until the 1.3.0 api is settled, there's
obvious issues for some of the new features.  So these can be reverted on
1.3.x branch, and kept on trunk/1.4 for further development (e.g. to resolve
each of Joe's objections to the ssl/evp code - provided he's ok with leaving
it to the trunk for the time being?)

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 04/14/2008 10:52 PM, Joe Orton wrote:
> On Mon, Apr 14, 2008 at 10:44:09PM +0200, Ruediger Pluem wrote:
>> But now that we have branched 1.3.0 we have to live with it until 2.0, right?
>> Maybe branching just 48 hours after these commits wasn't such a good idea
>> especially with these 48 hours being a weekend after ApacheCon.
> 
> Nothing is fixed in stone on the 1.3.x branch until a 1.3.0 release is 
> made and approved.  Having the 1.3.x branch there is fine and useful.

Then everything is fine and I agree that it is *very* useful to have the
branch just to iron out such problems. As mentioned I thought and was worried
that we would have to live for a long time we such issues that did not
have enough discussion.

Regards

RĂ¼diger

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Mon, Apr 14, 2008 at 09:52:13PM +0100, Joe Orton wrote:
> On Mon, Apr 14, 2008 at 10:44:09PM +0200, Ruediger Pluem wrote:
> > But now that we have branched 1.3.0 we have to live with it until 2.0, right?
> > Maybe branching just 48 hours after these commits wasn't such a good idea
> > especially with these 48 hours being a weekend after ApacheCon.
> 
> Nothing is fixed in stone on the 1.3.x branch until a 1.3.0 release is 
> made and approved.  Having the 1.3.x branch there is fine and useful.
> 

+1. And we shouldn't feel rushed in any way by what is going
on in httpd-land either :)
-- 
===========================================================================
   Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
        "Great is the guilt of an unnecessary war"  ~ John Adams

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Apr 14, 2008 at 10:44:09PM +0200, Ruediger Pluem wrote:
> But now that we have branched 1.3.0 we have to live with it until 2.0, right?
> Maybe branching just 48 hours after these commits wasn't such a good idea
> especially with these 48 hours being a weekend after ApacheCon.

Nothing is fixed in stone on the 1.3.x branch until a 1.3.0 release is 
made and approved.  Having the 1.3.x branch there is fine and useful.

joe

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 04/14/2008 10:05 PM, Sander Striker wrote:
> On Mon, Apr 14, 2008 at 9:43 PM, Joe Orton <jo...@redhat.com> wrote:
>> On Sat, Apr 12, 2008 at 08:42:59AM -0000, Mladen Turk wrote:
>>  > Author: mturk
>>  > Date: Sat Apr 12 01:42:51 2008
>>  > New Revision: 647394
>>  >
>>  > URL: http://svn.apache.org/viewvc?rev=647394&view=rev
>>  > Log:
>>  > Introduce apr_pool_sys_allocator_set
>>
>>  Why can't you just use a custom allocator for whatever the problem is
>>  here?  That's what the allocator abstraction is *for*, right?  Adding
>>  yet another abstraction above the allocator seems really wrong;
>>  especially since it introduces a bunch of global state and *everybody*
>>  suffers the overhead of the additional function calls and conditionals.
> 
> Indeed.  We went through quite a few iterations of performance testing
> and tweaks to get to where we are.  Note that we introduced a memory
> allocation system with more abstractions before... we punted on that once
> we found out when it caused too much overhead.

But now that we have branched 1.3.0 we have to live with it until 2.0, right?
Maybe branching just 48 hours after these commits wasn't such a good idea
especially with these 48 hours being a weekend after ApacheCon.

Regards

RĂ¼diger



Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Sander Striker <st...@apache.org>.
On Mon, Apr 14, 2008 at 9:43 PM, Joe Orton <jo...@redhat.com> wrote:
> On Sat, Apr 12, 2008 at 08:42:59AM -0000, Mladen Turk wrote:
>  > Author: mturk
>  > Date: Sat Apr 12 01:42:51 2008
>  > New Revision: 647394
>  >
>  > URL: http://svn.apache.org/viewvc?rev=647394&view=rev
>  > Log:
>  > Introduce apr_pool_sys_allocator_set
>
>  Why can't you just use a custom allocator for whatever the problem is
>  here?  That's what the allocator abstraction is *for*, right?  Adding
>  yet another abstraction above the allocator seems really wrong;
>  especially since it introduces a bunch of global state and *everybody*
>  suffers the overhead of the additional function calls and conditionals.

Indeed.  We went through quite a few iterations of performance testing
and tweaks to get to where we are.  Note that we introduced a memory
allocation system with more abstractions before... we punted on that once
we found out when it caused too much overhead.

Cheers,

Sander

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Mladen Turk <mt...@apache.org>.
Joe Orton wrote:
> 
> Can you explain some of those other things might be?  There are multiple 
> ways you can hook into a profiling toolkit without requiring source code 
> changes:
> 
> 1) compile-time hacks to #define malloc
> 2) at link-time link against some library which provides malloc etc 
> before libc
> 3) at load-time LD_PRELOAD a library which overrides libc malloc etc
> 

In multiplatform environment only the 1) is valid
option. However it lacks API for profiling, so basically
you cannot hook into apr from your application unless
you recompile your application as well and like you
said do some hacks.

Further more, profiling is just one of the use cases.
Other is application provided memory allocation not
known at the apr compile time. I reverted the patch
because it was incomplete. It should address both
internal usage of malloc, per allocator memory
callbacks (similar how malloc calls sbrk) and statistical
data if APR_POOL_PROFILE was defined in apr compile time.

Regards
--
(TM)




Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Gonzalo Arana <go...@gmail.com>.
Hi,

On Wed, Apr 16, 2008 at 11:00 AM, Joe Orton <jo...@redhat.com> wrote:
> On Tue, Apr 15, 2008 at 06:48:23AM +0200, Mladen Turk wrote:
>  > Joe Orton wrote:
>  > > On Sat, Apr 12, 2008 at 08:42:59AM -0000, Mladen Turk wrote:
>  > >> Author: mturk
>  > >> Date: Sat Apr 12 01:42:51 2008
>  > >> New Revision: 647394
>  > >>
>  > >> URL: http://svn.apache.org/viewvc?rev=647394&view=rev
>  > >> Log:
>  > >> Introduce apr_pool_sys_allocator_set
>  > >
>  > > Why can't you just use a custom allocator for whatever the problem
>  > > is here?  That's what the allocator abstraction is *for*, right?
>  > > Adding yet another abstraction above the allocator seems really
>  > > wrong; especially since it introduces a bunch of global state and
>  > > *everybody* suffers the overhead of the additional function calls
>  > > and conditionals.
>  >
>  > How can I use custom allocator from outside apr?
>  >
>  > The allocator is private, so cannot be customized from application.
>
>  Good point.
>
>
>  > It relies on malloc/free, and the point of patch is to abstract those
>  > calls. Patch itself only sets the function pointer, and since its
>  > warper is inline function the proper compiler should make no
>  > overhead except one JNZ instruction.
>  >
>  > The point is that apr internally uses system allocator and
>  > we presume this should be malloc/free. Any other mechanism
>  > is ruled out by that presumption. malloc/free might not be the
>  > most effective way to allocate the memory from the system in
>  > all use cases. It also doesn't allow things like profiling
>  > and all other sort of things the application might have.
>
>  Can you explain some of those other things might be?  There are multiple
>  ways you can hook into a profiling toolkit without requiring source code
>  changes:
>
>  1) compile-time hacks to #define malloc
>  2) at link-time link against some library which provides malloc etc
>  before libc
>  3) at load-time LD_PRELOAD a library which overrides libc malloc etc

There is one possible "other thing": you may want to use an mmap'ed
based allocator for the request pool & it's children, and a
malloc/free based for other, long term pools.

I may be wrong about this, but I believe a mmap/munmap based allocator
would allow to reduce the simultaneous memory usage, since after a
request is processed, the memory would be actually unmaped.  Calling
free(3) does not always reduce the process memory usage (on Linux at
least).

Regards,

-- 
Gonzalo A. Arana

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Joe Orton <jo...@redhat.com>.
On Tue, Apr 15, 2008 at 06:48:23AM +0200, Mladen Turk wrote:
> Joe Orton wrote:
> > On Sat, Apr 12, 2008 at 08:42:59AM -0000, Mladen Turk wrote:
> >> Author: mturk
> >> Date: Sat Apr 12 01:42:51 2008
> >> New Revision: 647394
> >>
> >> URL: http://svn.apache.org/viewvc?rev=647394&view=rev
> >> Log:
> >> Introduce apr_pool_sys_allocator_set
> >
> > Why can't you just use a custom allocator for whatever the problem 
> > is here?  That's what the allocator abstraction is *for*, right? 
> > Adding yet another abstraction above the allocator seems really 
> > wrong; especially since it introduces a bunch of global state and 
> > *everybody* suffers the overhead of the additional function calls 
> > and conditionals.
>
> How can I use custom allocator from outside apr?
>
> The allocator is private, so cannot be customized from application.

Good point.

> It relies on malloc/free, and the point of patch is to abstract those
> calls. Patch itself only sets the function pointer, and since its
> warper is inline function the proper compiler should make no
> overhead except one JNZ instruction.
>
> The point is that apr internally uses system allocator and
> we presume this should be malloc/free. Any other mechanism
> is ruled out by that presumption. malloc/free might not be the
> most effective way to allocate the memory from the system in
> all use cases. It also doesn't allow things like profiling
> and all other sort of things the application might have.

Can you explain some of those other things might be?  There are multiple 
ways you can hook into a profiling toolkit without requiring source code 
changes:

1) compile-time hacks to #define malloc
2) at link-time link against some library which provides malloc etc 
before libc
3) at load-time LD_PRELOAD a library which overrides libc malloc etc

Regards,

joe

Re: svn commit: r647394 - in /apr/apr/trunk: CHANGES include/apr_pools.h memory/unix/apr_pools.c

Posted by Mladen Turk <mt...@apache.org>.
Joe Orton wrote:
 > On Sat, Apr 12, 2008 at 08:42:59AM -0000, Mladen Turk wrote:
 >> Author: mturk
 >> Date: Sat Apr 12 01:42:51 2008
 >> New Revision: 647394
 >>
 >> URL: http://svn.apache.org/viewvc?rev=647394&view=rev
 >> Log:
 >> Introduce apr_pool_sys_allocator_set
 >
 > Why can't you just use a custom allocator for whatever the problem is 
here?  That's what the allocator abstraction is *for*, right?  Adding 
yet another abstraction above the allocator seems really wrong; 
especially since it introduces a bunch of global state and *everybody* 
suffers the overhead of the additional function calls and conditionals.
 >

How can I use custom allocator from outside apr?
The allocator is private, so cannot be customized from application.
It relies on malloc/free, and the point of patch is to abstract those
calls. Patch itself only sets the function pointer, and since its
warper is inline function the proper compiler should make no
overhead except one JNZ instruction.

The point is that apr internally uses system allocator and
we presume this should be malloc/free. Any other mechanism
is ruled out by that presumption. malloc/free might not be the
most effective way to allocate the memory from the system in
all use cases. It also doesn't allow things like profiling
and all other sort of things the application might have.
Once you create an allocator you don't have a track what's
going on beneath. You don't know how apr internally allocates
the memory from the system. Those callbacks if set are the
solution.

However if you really think this is an too high overhead
we might add compile time switch (APR_POOL_PROFILE) that
if not set will skip out this single compare instruction.

Regards
-- 
(TM)