You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Justin Erenkrantz <je...@ebuilt.com> on 2001/12/05 03:56:11 UTC
Re: Pools rewrite [2]
On Fri, Nov 30, 2001 at 06:27:14PM +0100, Sander Striker wrote:
> Hi,
>
> This is my second go at the pools code.
> I incorporated some suggestions made by Brian Pane.
>
> Please review,
General comments:
1) Lose the tabs.
2) It probably needs to be modified to use the new locking API.
I don't see this as a showstopper, but I think it's something
that should happen and may help with the locking performance.
Aaron can speak more about this...
Rest of my comments are inline (denoted by <comment>). (Are you
using Outlook? If not, please inline your patches...)
My comments aren't really about the core logic though. I know I
reviewed the overall logic and found it correct before. I'm going
to assume that the core memory logic hasn't changed. And, Brian
and Ian's testing prove that it works in practice, so I'll look at
the logic again when I have more time. -- justin
------
<snip, snip>
/*
* Magic numbers
*/
#define MIN_ALLOC 8192
#define MAX_INDEX 16
#define BOUNDARY_INDEX 12
#define BOUNDARY_SIZE (1 << BOUNDARY_INDEX)
<comment>
I think I mentioned it before, but it'd probably be good to
indicate rationale why we think these values are good. I'm
not disagreeing with them, but I think it might be helpful to
explain how they fit in the overall scheme of things.
And, if this goes in, I'll probably do the same thing I did
with SMS and make a pass that adds source code comments. That
won't happen until next week at the earliest.
</comment>
/*
* Macros and defines
*/
/* ALIGN() is only to be used to align on a power of 2 boundary */
#define ALIGN(size, boundary) \
(((size) + ((boundary) - 1)) & ~((boundary) - 1))
#define ALIGN_DEFAULT(size) ALIGN(size, 8)
#define LOCK(mutex) \
if (mutex) \
apr_thread_mutex_lock(mutex);
#define UNLOCK(mutex) \
if (mutex) \
apr_thread_mutex_unlock(mutex);
<comment>
Ack. No. Declare a new scope like so:
#define UNLOCK(mutex) \
do { \
if (mutex) \
apr_thread_mutex_unlock(mutex); \
} while(0)
Calling it like UNLOCK(mutex) (i.e. no semicolon) is just badness
and will shoot us down the road later.
</comment>
<snip, snip>
/* apr_pcalloc is almost exactly the same as apr_palloc, except for
* a few memset()s. This saves an extra function call though, which
* is enough justification for this code duplication IMO.
*/
<comment>
On a thought from Dean Gaudet, how would performance be helped if
we #define'd apr_pcalloc to be:
#define apr_pcalloc(pool, size) memset(apr_palloc(pool, size), '\0', size);
The key idea is that Dean pointed out that the compiler can do a
*much* better job of optimizing if we don't stick the memset deep
down in a function somewhere. I'd be curious to see how this
optimization would help. And, I think code duplication is evil
anyway. This has the potential to be faster than apr_pcalloc and
use the same code.
</comment>
APR_DECLARE(void *) apr_pcalloc(apr_pool_t *pool, apr_size_t size)
{
<snip, snip>
/*
* Initialization
*/
APR_DECLARE(apr_status_t) apr_pool_alloc_init(apr_pool_t *global)
{
apr_status_t rv;
if (global_allocator_initialized)
return APR_SUCCESS; /* Is this correct? */
<comment>
Sure. apr_thread_once is probably a much better way to handle this
though I think.
</comment>
memset(&global_allocator, 0, SIZEOF_ALLOCATOR_T);
if ((rv = apr_thread_mutex_create(&global_allocator.mutex,
APR_THREAD_MUTEX_DEFAULT, global)) != APR_SUCCESS) {
return rv;
}
global_allocator.owner = global;
global_pool = global;
global_allocator_initialized = 1;
return APR_SUCCESS;
}
<snip, snip>
Re: Pools rewrite [2]
Posted by Aaron Bannert <aa...@clove.org>.
On Tue, Dec 11, 2001 at 11:27:25AM +0100, Sander Striker wrote:
> A thread doesn't own any pools nor does it know about specific pools,
> the same goes for the other way around.
Actually, there is a pool created and associated to a thread when that
thread is created. See APR_POOL_IMPLEMENT_ACCESSOR_X in thread.c.
> The idea is to have to option to optimize for the situation where
> you _know_ that a pool will only be accessed by one thread at a time.
Back in June or July I advocated removing the thread's pool from the
actual thread structure, and just letting the app pass whatever pool
contexts they needed through the opaque thread data, but that didn't
happen. So now I'd expect someone who wanted to use a special allocator
to first call the accessor (apr_thread_pool_get(thd) implemented with
APR_POOL_IMPLEMENT_ACCESSOR) and then create another subpool that has
private free lists and no global locks.
> The app is in control. If you're not sure if you are using a pool
> in more than one thread at a time, don't disable locking for that
> pools allocator.
Absolutely. :)
-aaron
RE: Pools rewrite [2]
Posted by Sander Striker <st...@apache.org>.
> From: Sander Striker [mailto:striker@apache.org]
> Sent: 11 December 2001 11:27
> > From: Christian Gross [mailto:christiangross@yahoo.de]
> > Sent: 11 December 2001 11:19
>
> > At 22:12 04/12/2001 -0500, Cliff Woolley wrote:
> >
> > > > This is my second go at the pools code.
> > >
> > >A potential snag with the thread-specific pools idea was brought up today
> > >when I was talking with FirstBill and some of the others. What is this
> > >going to do to us a little ways down the road when we try to implement
> > >async I/O, and all of the sudden requests are jumping from one thread to
> > >another? Sounds like a big problem if thread-specific pools are in
> > >place... is there an easy answer?
> > >
> > >--Cliff
> >
> > Hi
> >
> > I know I am harping in a bit late in the conversation... BUT I really
> > wanted to add something here. COM started with the same thing. In COM
> > there are different threading models, which were created out of the need to
> > synchronize access and define ownership. Likewise in Windows GUI code you
> > had the same situation. In either case the problem's of having thread
> > owned stuff is that things become "quirky". When I mean "quirky", I mean
> > gee it worked then, but not now.
>
> A thread doesn't own any pools nor does it know about specific pools,
> the same goes for the other way around.
>
> The idea is to have to option to optimize for the situation where
^^ the
*sigh*
> you _know_ that a pool will only be accessed by one thread at a time.
>
> The app is in control. If you're not sure if you are using a pool
> in more than one thread at a time, don't disable locking for that
> pools allocator.
>
> > So sorry about being a bit late on the topic...
>
> No prob,
>
> > Christian Gross
Sander
RE: Pools rewrite [2]
Posted by Sander Striker <st...@apache.org>.
> From: Christian Gross [mailto:christiangross@yahoo.de]
> Sent: 11 December 2001 11:19
> At 22:12 04/12/2001 -0500, Cliff Woolley wrote:
>
> > > This is my second go at the pools code.
> >
> >A potential snag with the thread-specific pools idea was brought up today
> >when I was talking with FirstBill and some of the others. What is this
> >going to do to us a little ways down the road when we try to implement
> >async I/O, and all of the sudden requests are jumping from one thread to
> >another? Sounds like a big problem if thread-specific pools are in
> >place... is there an easy answer?
> >
> >--Cliff
>
> Hi
>
> I know I am harping in a bit late in the conversation... BUT I really
> wanted to add something here. COM started with the same thing. In COM
> there are different threading models, which were created out of the need to
> synchronize access and define ownership. Likewise in Windows GUI code you
> had the same situation. In either case the problem's of having thread
> owned stuff is that things become "quirky". When I mean "quirky", I mean
> gee it worked then, but not now.
A thread doesn't own any pools nor does it know about specific pools,
the same goes for the other way around.
The idea is to have to option to optimize for the situation where
you _know_ that a pool will only be accessed by one thread at a time.
The app is in control. If you're not sure if you are using a pool
in more than one thread at a time, don't disable locking for that
pools allocator.
> So sorry about being a bit late on the topic...
No prob,
> Christian Gross
Sander
Re: Pools rewrite [2]
Posted by Christian Gross <ch...@yahoo.de>.
At 22:12 04/12/2001 -0500, Cliff Woolley wrote:
> > This is my second go at the pools code.
>
>A potential snag with the thread-specific pools idea was brought up today
>when I was talking with FirstBill and some of the others. What is this
>going to do to us a little ways down the road when we try to implement
>async I/O, and all of the sudden requests are jumping from one thread to
>another? Sounds like a big problem if thread-specific pools are in
>place... is there an easy answer?
>
>--Cliff
>
>--------------------------------------------------------------
> Cliff Woolley
> cliffwoolley@yahoo.com
> Charlottesville, VA
Hi
I know I am harping in a bit late in the conversation... BUT I really
wanted to add something here. COM started with the same thing. In COM
there are different threading models, which were created out of the need to
synchronize access and define ownership. Likewise in Windows GUI code you
had the same situation. In either case the problem's of having thread
owned stuff is that things become "quirky". When I mean "quirky", I mean
gee it worked then, but not now.
So sorry about being a bit late on the topic...
Christian Gross
Re: Pools rewrite [2]
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Wed, Dec 05, 2001 at 10:14:20AM -0800, Aaron Bannert wrote:
> No, it'll be something like this:
>
> #ifdef APR_HAS_THREADS
> /* do stuff to prevent deadlocks on threaded builds, like: */
> apr_thread_mutex_lock(mutex);
> #else
> /* I can't imagine what you'd need to do special to treat the
> * non-threaded case, so the "#else" case probably won't exist. */
> #endif
>
> Pools don't need cross-process locks, right? In any case, the new lock
> api is supposed to be a full replacement for the old one.
Right, pools are at the very least contained to a single memory
space. So, there should be no need for locks when we don't have
threads. (They aren't shared memory pools...)
FWIW, that should be
#if APR_HAS_THREADS
=) -- justin
Re: Pools rewrite [2]
Posted by Aaron Bannert <aa...@clove.org>.
On Wed, Dec 05, 2001 at 06:06:28PM +0100, Sander Striker wrote:
> > IMHO, I don't see the use of a macro for this kind of thing in the first
> > place, because it's a simple enough construct. Also, we might want to check
> > for errors on that lock call (up to you).
>
> For one, use of this macro let me convert to the new locking api rather
> quickly.
That's fine, it's up to you. If it bothers me enough to change it I'll
post a patch. :) We're going to have to add some more stuff in there
anyway (like a return for status or something) which wouldn't make sense
to have in a macro. Justification follows...
> [...]
> > > Isn't apr_thread_once restricted to platforms where we actually have
> > > threads? Hmmm, I now start to wonder about my apr_thread_mutex_xxx
> > > calls. Aaron?
> >
> > apr_thread_once seems to be only usable on platforms where APR_HAS_THREADS
> > is defined. On platforms where there are no threads, apr_thread_once
> > can just be an int flag to make sure it doesn't get called twice.
>
> Could someone provide a patch that works in both cases?
I'm suggesting that either you protect the apr_thread_once call in #ifdef's,
or we write an apr_thread_once that works regardless of if APR_HAS_THREADS
is defined.
> > As for the locking functions, the old way was that the locking routines
> > were always available on platforms with thread support or not, but at
> > runtime we'd get an APR_ENOTIMPL on platforms that didn't support it. If
> > we run across a platform that doesn't support apr_thread_mutex_t right now,
> > we'll at least get a compile-time error. Since that is undesirable, all
> > uses of apr_thread_mutex_t should be protected in APR_HAS_THREADS #ifdefs.
>
> Hmmm, so, actually we should use the old locking API (for cross process)
> if !APR_HAS_THREADS and the apr_thread_mutex_xxx API when APR_HAS_THREADS?
No, it'll be something like this:
#ifdef APR_HAS_THREADS
/* do stuff to prevent deadlocks on threaded builds, like: */
apr_thread_mutex_lock(mutex);
#else
/* I can't imagine what you'd need to do special to treat the
* non-threaded case, so the "#else" case probably won't exist. */
#endif
Pools don't need cross-process locks, right? In any case, the new lock
api is supposed to be a full replacement for the old one.
-aaron
Re: Pools rewrite [2]
Posted by Greg Marr <gr...@alum.wpi.edu>.
At 11:44 AM 12/05/2001, Brian Pane wrote:
>Aaron Bannert wrote:
>
>>On Wed, Dec 05, 2001 at 12:35:00PM +0100, Sander Striker wrote:
>>
>>>><comment>
>>>>Ack. No. Declare a new scope like so:
>>>>#define UNLOCK(mutex) \
>>>> do { \
>>>> if (mutex) \
>>>> apr_thread_mutex_unlock(mutex); \
>>>> } while(0)
>>>>
>>>>Calling it like UNLOCK(mutex) (i.e. no semicolon) is just badness
>>>>and will shoot us down the road later.
>>>></comment>
>>>Ok, will change that.
>>
>>If the semicolon is the only problem, just remove the semicolon from
>>the macro:
>>
>>#define UNLOCK(mutex) \
>> if (mutex) \
>> apr_thread_mutex_unlock(mutex)
>
>I recommend still creating a scope around the if-statement:
>#define UNLOCK(mutex) \
> { \
> if (mutex) \
> apr_thread_mutex_unlock(mutex); \
> }
>
>That will guard against unintended effects when the macro
>is used in code like this:
>
> if (condition)
> UNLOCK(mutex)
> else
> ...
That will fail if it's called like this:
if (condition)
UNLOCK(mutex);
else
...
since it will expand to:
if (condition)
{
if (mutex)
apr_thread_mutex_unlock(mutex);
};
else
...
Note the ; after the close brace, that makes the else illegal.
The only truly safe construct that I've seen is the do/while
recommended above.
--
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"
Re: Pools rewrite [2]
Posted by Ryan Bloom <rb...@covalent.net>.
The original code was 100% correct. It is the model that we use
through out the code, and it is the model that we should continue
to use. It allows people to do
UNLOCK(mutex);
which is what most programmers expect.
Ryan
On Wednesday 05 December 2001 08:44 am, Brian Pane wrote:
> Aaron Bannert wrote:
> >On Wed, Dec 05, 2001 at 12:35:00PM +0100, Sander Striker wrote:
> >>><comment>
> >>>Ack. No. Declare a new scope like so:
> >>>#define UNLOCK(mutex) \
> >>> do { \
> >>> if (mutex) \
> >>> apr_thread_mutex_unlock(mutex); \
> >>> } while(0)
> >>>
> >>>Calling it like UNLOCK(mutex) (i.e. no semicolon) is just badness
> >>>and will shoot us down the road later.
> >>></comment>
> >>
> >>Ok, will change that.
> >
> >If the semicolon is the only problem, just remove the semicolon from
> >the macro:
> >
> >#define UNLOCK(mutex) \
> > if (mutex) \
> > apr_thread_mutex_unlock(mutex)
>
> I recommend still creating a scope around the if-statement:
> #define UNLOCK(mutex) \
> { \
> if (mutex) \
> apr_thread_mutex_unlock(mutex); \
> }
>
> That will guard against unintended effects when the macro
> is used in code like this:
>
> if (condition)
> UNLOCK(mutex)
> else
> ...
>
> --Brian
--
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: Pools rewrite [2]
Posted by Brian Pane <bp...@pacbell.net>.
Aaron Bannert wrote:
>On Wed, Dec 05, 2001 at 12:35:00PM +0100, Sander Striker wrote:
>
>>><comment>
>>>Ack. No. Declare a new scope like so:
>>>#define UNLOCK(mutex) \
>>> do { \
>>> if (mutex) \
>>> apr_thread_mutex_unlock(mutex); \
>>> } while(0)
>>>
>>>Calling it like UNLOCK(mutex) (i.e. no semicolon) is just badness
>>>and will shoot us down the road later.
>>></comment>
>>>
>>Ok, will change that.
>>
>
>If the semicolon is the only problem, just remove the semicolon from
>the macro:
>
>#define UNLOCK(mutex) \
> if (mutex) \
> apr_thread_mutex_unlock(mutex)
>
I recommend still creating a scope around the if-statement:
#define UNLOCK(mutex) \
{ \
if (mutex) \
apr_thread_mutex_unlock(mutex); \
}
That will guard against unintended effects when the macro
is used in code like this:
if (condition)
UNLOCK(mutex)
else
...
--Brian
RE: Pools rewrite [2]
Posted by Sander Striker <st...@apache.org>.
> From: Aaron Bannert [mailto:aaron@clove.org]
> Sent: 05 December 2001 17:32
> On Wed, Dec 05, 2001 at 12:35:00PM +0100, Sander Striker wrote:
> > > <comment>
> > > Ack. No. Declare a new scope like so:
> > > #define UNLOCK(mutex) \
> > > do { \
> > > if (mutex) \
> > > apr_thread_mutex_unlock(mutex); \
> > > } while(0)
> > >
> > > Calling it like UNLOCK(mutex) (i.e. no semicolon) is just badness
> > > and will shoot us down the road later.
> > > </comment>
> >
> > Ok, will change that.
>
> If the semicolon is the only problem, just remove the semicolon from
> the macro:
>
> #define UNLOCK(mutex) \
> if (mutex) \
> apr_thread_mutex_unlock(mutex)
>
>
> IMHO, I don't see the use of a macro for this kind of thing in the first
> place, because it's a simple enough construct. Also, we might want to check
> for errors on that lock call (up to you).
For one, use of this macro let me convert to the new locking api rather
quickly.
[...]
> > Isn't apr_thread_once restricted to platforms where we actually have
> > threads? Hmmm, I now start to wonder about my apr_thread_mutex_xxx
> > calls. Aaron?
>
> apr_thread_once seems to be only usable on platforms where APR_HAS_THREADS
> is defined. On platforms where there are no threads, apr_thread_once
> can just be an int flag to make sure it doesn't get called twice.
Could someone provide a patch that works in both cases?
> As for the locking functions, the old way was that the locking routines
> were always available on platforms with thread support or not, but at
> runtime we'd get an APR_ENOTIMPL on platforms that didn't support it. If
> we run across a platform that doesn't support apr_thread_mutex_t right now,
> we'll at least get a compile-time error. Since that is undesirable, all
> uses of apr_thread_mutex_t should be protected in APR_HAS_THREADS #ifdefs.
Hmmm, so, actually we should use the old locking API (for cross process)
if !APR_HAS_THREADS and the apr_thread_mutex_xxx API when APR_HAS_THREADS?
> -aaron
Sander
Re: Pools rewrite [2]
Posted by Aaron Bannert <aa...@clove.org>.
On Wed, Dec 05, 2001 at 12:35:00PM +0100, Sander Striker wrote:
> > <comment>
> > Ack. No. Declare a new scope like so:
> > #define UNLOCK(mutex) \
> > do { \
> > if (mutex) \
> > apr_thread_mutex_unlock(mutex); \
> > } while(0)
> >
> > Calling it like UNLOCK(mutex) (i.e. no semicolon) is just badness
> > and will shoot us down the road later.
> > </comment>
>
> Ok, will change that.
If the semicolon is the only problem, just remove the semicolon from
the macro:
#define UNLOCK(mutex) \
if (mutex) \
apr_thread_mutex_unlock(mutex)
IMHO, I don't see the use of a macro for this kind of thing in the first
place, because it's a simple enough construct. Also, we might want to check
for errors on that lock call (up to you).
[snip]
> >
> > /*
> > * Initialization
> > */
> >
> > APR_DECLARE(apr_status_t) apr_pool_alloc_init(apr_pool_t *global)
> > {
> > apr_status_t rv;
> >
> > if (global_allocator_initialized)
> > return APR_SUCCESS; /* Is this correct? */
> >
> > <comment>
> > Sure. apr_thread_once is probably a much better way to handle this
> > though I think.
> > </comment>
>
> My point was if it was correct to return APR_SUCCESS. People are
> initializing twice if they reach that line of code.
>
> Isn't apr_thread_once restricted to platforms where we actually have
> threads? Hmmm, I now start to wonder about my apr_thread_mutex_xxx
> calls. Aaron?
apr_thread_once seems to be only usable on platforms where APR_HAS_THREADS
is defined. On platforms where there are no threads, apr_thread_once
can just be an int flag to make sure it doesn't get called twice.
As for the locking functions, the old way was that the locking routines
were always available on platforms with thread support or not, but at
runtime we'd get an APR_ENOTIMPL on platforms that didn't support it. If
we run across a platform that doesn't support apr_thread_mutex_t right now,
we'll at least get a compile-time error. Since that is undesirable, all
uses of apr_thread_mutex_t should be protected in APR_HAS_THREADS #ifdefs.
[snip]
-aaron
RE: Pools rewrite [2]
Posted by Sander Striker <st...@apache.org>.
> From: Roy T. Fielding [mailto:fielding@ebuilt.com]
> Sent: 05 December 2001 04:20
> On Tue, Dec 04, 2001 at 10:12:20PM -0500, Cliff Woolley wrote:
> >
> > > This is my second go at the pools code.
> >
> > A potential snag with the thread-specific pools idea was brought up today
> > when I was talking with FirstBill and some of the others. What is this
> > going to do to us a little ways down the road when we try to implement
> > async I/O, and all of the sudden requests are jumping from one thread to
> > another? Sounds like a big problem if thread-specific pools are in
> > place... is there an easy answer?
>
> Are they thread-specific, or simply "owned" by one thread at a time?
Simply "owned" by one thread at a time.
> .....Roy
Sander
Re: Pools rewrite [2]
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
On Tue, Dec 04, 2001 at 10:12:20PM -0500, Cliff Woolley wrote:
>
> > This is my second go at the pools code.
>
> A potential snag with the thread-specific pools idea was brought up today
> when I was talking with FirstBill and some of the others. What is this
> going to do to us a little ways down the road when we try to implement
> async I/O, and all of the sudden requests are jumping from one thread to
> another? Sounds like a big problem if thread-specific pools are in
> place... is there an easy answer?
Are they thread-specific, or simply "owned" by one thread at a time?
....Roy
Re: Async IO was Re: Pools rewrite [2]
Posted by Bill Stoddard <bi...@wstoddard.com>.
Yep, my thoughts exactly. I am really interested in working on an Async MPM (in fact, I
have a windows MPM that does async writes. I have done one that does async reads. Now just
need to combine the two :-)
The big change that needs to happen is to enable setting a network layer nonblocking
attribute at the top of a filter stack, at the handler. The handler must make the decision
whether it wants to do async/non-blocking i/o or not. Probably also need to establish the
completion context at the top of the filter stack as well (the structure that enables you
to regain context after the pending i/o completes)
Oh, and we need to handle the fact that async i/o and non-blocking i/o are really
different things.
Windows uses async i/o rather than non-blocking i/o. That is, a windows network call that
doesn't complete immediately is taken over by the kernel and a kernel thread will do the
i/o on behalf of the application thread. When the i/o is complete, the completionport
associated with the socket is posted to wake up an application thread to complete the
work. The application must be damn careful not to muck with the storage passed to the
kernel on an async i/o.
/dev/poll, kqueue, et. al. (the typical Unix solutions) use non-blocking i/o. If a network
call does not complete immediately, the kernel does NOT do the i/o on behalf of the
application. Instead, a worker thread (blocked on the kqueue, select, whatever) is
awakened when the i/o can be reissued with the expectation of completing. The application
must reissue the i/o.
Async i/o 'saves' reissuing the i/o call, but the complexity with true async i/o as
compared to non-blocking i/o make async a PITA to deal with. I really don't like Windows
async i/o; it's too damn complicated.
If you ever feel up to poking around in the windows MPM, you will see a lot of the
infrastructure needed to make it an async MPM. I'm thinking I will just add a config
option to the MPM to make it run in async mode. Someday when I have free time :-)
The ultimate in coolness would be to create an APR API that hides all the nasty
implenmentation details under the covers.
Bill
> On Tuesday 04 December 2001 10:47 pm, Justin Erenkrantz wrote:
>
> It has always been a problem of timing. After 2.0 is released, I plan
> to look at creating an MPM for async I/O. And unless this requires
> massive code changes, I would expect it to go in a 2.0 release.
>
> This is the type of thing that we should be doing after 2.0 is finally
> made a GA release, look for cool ways to use the current architecture
> to do new things.
>
> Ryan
>
> > [ Moving this to dev@httpd since I think this is not yet an
> > APR issue. The app needs to drive this, not the portability
> > library. ]
> >
> > On Tue, Dec 04, 2001 at 11:09:11PM -0500, Bill Stoddard wrote:
> > > The idea is to never allow your threads to block doing network i/o. Do
> > > all your reads/writes with non-blocking (or asynchronous) i/o.
> >
> > Considering I'm a newbie here, I've just spent a lot of time in
> > the archives looking at what you guys have said before about
> > trying to do async I/O in httpd.
> >
> > So, can someone tell me why we haven't done this before? It
> > sure isn't because no one has wanted to. It keeps popping up
> > every year or so since 1997. What has been the killer?
> >
> > I see a tremendous amount of posts around June/July 1999 about
> > work on async-server hybrid (ASH MPM). However, I can't find
> > what happened to the bugger. Dean kept talking about it, but
> > obviously something happened to halt it.
> >
> > <Pi...@twinlark.arctic.org>
> > http://www.apachelabs.org/apache-mbox/199906.mbox/%3cPine.LNX.3.96dg4.99061
> >8090647.27639I-100000@twinlark.arctic.org%3e
> >
> > <Pi...@twinlark.arctic.org>
> > http://www.apachelabs.org/apache-mbox/199906.mbox/%3cPine.LNX.3.96dg4.99061
> >9160004.27639b-100000@twinlark.arctic.org%3e
> >
> > I wish we had a web page that listed "frequent topics" and
> > their point/counterpoints. I don't want to go down any road
> > that has already been covered. And, seeing the posts in this
> > timeframe leads me to believe that almost every point has been
> > covered. -- justin
>
> --
>
> ______________________________________________________________
> Ryan Bloom rbb@apache.org
> Covalent Technologies rbb@covalent.net
> --------------------------------------------------------------
>
Re: Async IO was Re: Pools rewrite [2]
Posted by Ryan Bloom <rb...@covalent.net>.
On Tuesday 04 December 2001 10:47 pm, Justin Erenkrantz wrote:
It has always been a problem of timing. After 2.0 is released, I plan
to look at creating an MPM for async I/O. And unless this requires
massive code changes, I would expect it to go in a 2.0 release.
This is the type of thing that we should be doing after 2.0 is finally
made a GA release, look for cool ways to use the current architecture
to do new things.
Ryan
> [ Moving this to dev@httpd since I think this is not yet an
> APR issue. The app needs to drive this, not the portability
> library. ]
>
> On Tue, Dec 04, 2001 at 11:09:11PM -0500, Bill Stoddard wrote:
> > The idea is to never allow your threads to block doing network i/o. Do
> > all your reads/writes with non-blocking (or asynchronous) i/o.
>
> Considering I'm a newbie here, I've just spent a lot of time in
> the archives looking at what you guys have said before about
> trying to do async I/O in httpd.
>
> So, can someone tell me why we haven't done this before? It
> sure isn't because no one has wanted to. It keeps popping up
> every year or so since 1997. What has been the killer?
>
> I see a tremendous amount of posts around June/July 1999 about
> work on async-server hybrid (ASH MPM). However, I can't find
> what happened to the bugger. Dean kept talking about it, but
> obviously something happened to halt it.
>
> <Pi...@twinlark.arctic.org>
> http://www.apachelabs.org/apache-mbox/199906.mbox/%3cPine.LNX.3.96dg4.99061
>8090647.27639I-100000@twinlark.arctic.org%3e
>
> <Pi...@twinlark.arctic.org>
> http://www.apachelabs.org/apache-mbox/199906.mbox/%3cPine.LNX.3.96dg4.99061
>9160004.27639b-100000@twinlark.arctic.org%3e
>
> I wish we had a web page that listed "frequent topics" and
> their point/counterpoints. I don't want to go down any road
> that has already been covered. And, seeing the posts in this
> timeframe leads me to believe that almost every point has been
> covered. -- justin
--
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Async IO was Re: Pools rewrite [2]
Posted by Justin Erenkrantz <je...@ebuilt.com>.
[ Moving this to dev@httpd since I think this is not yet an
APR issue. The app needs to drive this, not the portability
library. ]
On Tue, Dec 04, 2001 at 11:09:11PM -0500, Bill Stoddard wrote:
> The idea is to never allow your threads to block doing network i/o. Do all your
> reads/writes with non-blocking (or asynchronous) i/o.
Considering I'm a newbie here, I've just spent a lot of time in
the archives looking at what you guys have said before about
trying to do async I/O in httpd.
So, can someone tell me why we haven't done this before? It
sure isn't because no one has wanted to. It keeps popping up
every year or so since 1997. What has been the killer?
I see a tremendous amount of posts around June/July 1999 about
work on async-server hybrid (ASH MPM). However, I can't find
what happened to the bugger. Dean kept talking about it, but
obviously something happened to halt it.
<Pi...@twinlark.arctic.org>
http://www.apachelabs.org/apache-mbox/199906.mbox/%3cPine.LNX.3.96dg4.990618090647.27639I-100000@twinlark.arctic.org%3e
<Pi...@twinlark.arctic.org>
http://www.apachelabs.org/apache-mbox/199906.mbox/%3cPine.LNX.3.96dg4.990619160004.27639b-100000@twinlark.arctic.org%3e
I wish we had a web page that listed "frequent topics" and
their point/counterpoints. I don't want to go down any road
that has already been covered. And, seeing the posts in this
timeframe leads me to believe that almost every point has been
covered. -- justin
Re: Pools rewrite [2]
Posted by Bill Stoddard <bi...@wstoddard.com>.
> On Tue, Dec 04, 2001 at 10:12:20PM -0500, Cliff Woolley wrote:
> >
> > > This is my second go at the pools code.
> >
> > A potential snag with the thread-specific pools idea was brought up today
> > when I was talking with FirstBill and some of the others. What is this
> > going to do to us a little ways down the road when we try to implement
> > async I/O, and all of the sudden requests are jumping from one thread to
> > another? Sounds like a big problem if thread-specific pools are in
> > place... is there an easy answer?
>
> My guess would be not to pass POOL_FNEW_ALLOCATOR to apr_pool_create.
> It's an option passed when we create the pool, so if you can't
> guarantee the thread-locality of the pool, it would make sense not to
> split the pool off from its parent.
>
> But, the problem you are describing isn't really a concern to the
> pools - it's the use of the pools. All this does is allow threads
> in the same process to allocate memory at the same time (the current
> design has a bottleneck where all threads share a common per-process
> alloc mutex). If you control the usage of the pools so that a
> specific pool is only "owned" by one thread at a time, then you
> should be fine even with a per-thread allocator.
>
> I'd really appreciate some explanation on what this async I/O is
> and how it fits into the grand scheme of things. There are lots
> of assumptions within httpd about things being linear. -- justin
The idea is to never allow your threads to block doing network i/o. Do all your
reads/writes with non-blocking (or asynchronous) i/o. Your worker threads are blocked on a
queue (BSD KQueue, Solaris /dev/poll or Windows CompletionPort) waiting for i/o completion
notifications. When an i/o completes, the queue dispatches one of your threads, you
recover the request context (conn_rec, request_rec, et. al.) with a look-up function keyed
to the file descriptor (or other keys supported by the particular implementation), and
pick up where you left off. Details of how to recover context, how threads are awakened,
etc. are a bit different for each OS but the general idea is the same. I expect it is
possible to create an APR API that hides most if not all of the OS specific implementation
details.
Since your worker threads never block on network i/o, they are free to handle other work,
thus you can handle more work with fewer threads (and all the good things that implies).
Bill
RE: Pools rewrite [2]
Posted by Sander Striker <st...@apache.org>.
> From: Justin Erenkrantz [mailto:jerenkrantz@ebuilt.com]
> Sent: 05 December 2001 04:25
> On Tue, Dec 04, 2001 at 10:12:20PM -0500, Cliff Woolley wrote:
> >
> > > This is my second go at the pools code.
> >
> > A potential snag with the thread-specific pools idea was brought up today
> > when I was talking with FirstBill and some of the others. What is this
> > going to do to us a little ways down the road when we try to implement
> > async I/O, and all of the sudden requests are jumping from one thread to
> > another? Sounds like a big problem if thread-specific pools are in
> > place... is there an easy answer?
>
> My guess would be not to pass POOL_FNEW_ALLOCATOR to apr_pool_create.
> It's an option passed when we create the pool, so if you can't
> guarantee the thread-locality of the pool, it would make sense not to
> split the pool off from its parent.
Actually, that depends. One option is to not specify POOL_FNEW_ALLOCATOR
at all. The second option, if you know the pool is going to be shared by
threads but not much, pass POOL_FNEW_ALLOCATOR|POOL_FLOCK. Now, the pool
will have its own allocator (which subpools will inherit) which is protected
by a mutex. This is a different mutex than the one on the parent pool,
so 'fighting over' mutexes would be less likely.
> But, the problem you are describing isn't really a concern to the
> pools - it's the use of the pools. All this does is allow threads
> in the same process to allocate memory at the same time (the current
> design has a bottleneck where all threads share a common per-process
> alloc mutex). If you control the usage of the pools so that a
> specific pool is only "owned" by one thread at a time, then you
> should be fine even with a per-thread allocator.
Indeed. Currently the allocator is hidden away behind the pools API,
we could opt to expose it.
> I'd really appreciate some explanation on what this async I/O is
> and how it fits into the grand scheme of things. There are lots
> of assumptions within httpd about things being linear. -- justin
Sander
Re: Pools rewrite [2]
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Dec 04, 2001 at 10:12:20PM -0500, Cliff Woolley wrote:
>
> > This is my second go at the pools code.
>
> A potential snag with the thread-specific pools idea was brought up today
> when I was talking with FirstBill and some of the others. What is this
> going to do to us a little ways down the road when we try to implement
> async I/O, and all of the sudden requests are jumping from one thread to
> another? Sounds like a big problem if thread-specific pools are in
> place... is there an easy answer?
My guess would be not to pass POOL_FNEW_ALLOCATOR to apr_pool_create.
It's an option passed when we create the pool, so if you can't
guarantee the thread-locality of the pool, it would make sense not to
split the pool off from its parent.
But, the problem you are describing isn't really a concern to the
pools - it's the use of the pools. All this does is allow threads
in the same process to allocate memory at the same time (the current
design has a bottleneck where all threads share a common per-process
alloc mutex). If you control the usage of the pools so that a
specific pool is only "owned" by one thread at a time, then you
should be fine even with a per-thread allocator.
I'd really appreciate some explanation on what this async I/O is
and how it fits into the grand scheme of things. There are lots
of assumptions within httpd about things being linear. -- justin
Re: Pools rewrite [2]
Posted by Brian Pane <bp...@pacbell.net>.
Aaron Bannert wrote:
>On Tue, Dec 04, 2001 at 06:56:11PM -0800, Justin Erenkrantz wrote:
>[...]
>
>>2) It probably needs to be modified to use the new locking API.
>> I don't see this as a showstopper, but I think it's something
>> that should happen and may help with the locking performance.
>> Aaron can speak more about this...
>>
>[...]
>
>It's not a problem if this committed with the old lock API, I don't
>mind converting it.
>
It's using the new lock API already
--Brian
Re: Pools rewrite [2]
Posted by Aaron Bannert <aa...@clove.org>.
On Tue, Dec 04, 2001 at 06:56:11PM -0800, Justin Erenkrantz wrote:
[...]
> 2) It probably needs to be modified to use the new locking API.
> I don't see this as a showstopper, but I think it's something
> that should happen and may help with the locking performance.
> Aaron can speak more about this...
[...]
It's not a problem if this committed with the old lock API, I don't
mind converting it.
-aaron
Re: Pools rewrite [2]
Posted by Bill Stoddard <bi...@wstoddard.com>.
> From: "Cliff Woolley" <cl...@yahoo.com>
> Sent: Tuesday, December 04, 2001 9:12 PM
>
>
> > > This is my second go at the pools code.
> >
> > A potential snag with the thread-specific pools idea was brought up today
> > when I was talking with FirstBill and some of the others. What is this
> > going to do to us a little ways down the road when we try to implement
> > async I/O, and all of the sudden requests are jumping from one thread to
> > another? Sounds like a big problem if thread-specific pools are in
> > place... is there an easy answer?
>
> It certainly better not be a problem, or that asych would be borked.
>
> Only one thread -at-a-time- will be processing a request. Unlike 2.0, there
> might be no thread processing a request at a given time, but the request
> and connection pools can safely be assumed to be thread-safe at any given
> moment, if we do the async right.
>
> Of course, if you implement multi-thread/connection duplexing, then we get
> back to the same code we started with. And probably loose most of the
> async performance benefits to pool locks.
If the server is doing async network i/o, you will have far fewer threads to contend for
the lock. And the win for an async network i/o server is being able to efficiently handle
large numbers of concurrent clients with just a few threads.
Bill
Bill
>
>
>
>
>
Re: Pools rewrite [2]
Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Cliff Woolley" <cl...@yahoo.com>
Sent: Tuesday, December 04, 2001 9:12 PM
> > This is my second go at the pools code.
>
> A potential snag with the thread-specific pools idea was brought up today
> when I was talking with FirstBill and some of the others. What is this
> going to do to us a little ways down the road when we try to implement
> async I/O, and all of the sudden requests are jumping from one thread to
> another? Sounds like a big problem if thread-specific pools are in
> place... is there an easy answer?
It certainly better not be a problem, or that asych would be borked.
Only one thread -at-a-time- will be processing a request. Unlike 2.0, there
might be no thread processing a request at a given time, but the request
and connection pools can safely be assumed to be thread-safe at any given
moment, if we do the async right.
Of course, if you implement multi-thread/connection duplexing, then we get
back to the same code we started with. And probably loose most of the
async performance benefits to pool locks.
Re: Pools rewrite [2]
Posted by Brian Pane <br...@cnet.com>.
Bill Stoddard wrote:
[...]
>>If a future async implementation has this same property--i.e.,
>>pools can be "passed" from one thread to another, but a given
>>pool can only have its methods invoked from one thread at a
>>time--then we shouldn't have any problems.
>>
>>--Brian
>>
>
>What happens if a thread exits (under shutdown for instance) and requests are still being
>handled by other threads asynchrounously? Looks as if that threads cleanup will free the
>pool out from under the thread using the pool at the time.
>
Right, although that problem isn't specific to the freelist. In
the scenario you're describing, any resource inside the pool, like
an mmap or a file descriptor, could get clobbered by the thread
that owns the pool while another thread is simultaneously trying to
complete the request associated with the pool.
I think the implication of this is that, in the async model, the
thread that creates a pool for a transaction shouldn't be considered
the owner of the pool--and thus shouldn't take responsibility for
destroying the pool during shutdown. I look at that as a design
challenge for an async MPM, rather than something that the pools
themselves can solve.
--Brian
Re: Pools rewrite [2]
Posted by Ryan Bloom <rb...@covalent.net>.
On Tuesday 04 December 2001 07:53 pm, Bill Stoddard wrote:
> > Cliff Woolley wrote:
> > >>This is my second go at the pools code.
> > >
> > >A potential snag with the thread-specific pools idea was brought up
> > > today when I was talking with FirstBill and some of the others. What
> > > is this going to do to us a little ways down the road when we try to
> > > implement async I/O, and all of the sudden requests are jumping from
> > > one thread to another? Sounds like a big problem if thread-specific
> > > pools are in place... is there an easy answer?
> >
> > I think there's an easy answer.
> >
> > The thread-specific pools in this implementation are really
> > just pools that happen to own their own free lists. If those
> > pools get moved from one thread to another, that's okay, as
> > long as only one thread at a time is accessing a given pool
> > (which has always been a restriction of the pool API, because
> > apr_palloc() isn't thread-safe). In fact, we have this scenario
> > already in the worker MPM with Sander's pool code: the
> > thread-specific pool for each request gets created by the
> > listener thread and handed off to a worker thread.
> >
> > If a future async implementation has this same property--i.e.,
> > pools can be "passed" from one thread to another, but a given
> > pool can only have its methods invoked from one thread at a
> > time--then we shouldn't have any problems.
> >
> > --Brian
>
> What happens if a thread exits (under shutdown for instance) and requests
> are still being handled by other threads asynchrounously? Looks as if that
> threads cleanup will free the pool out from under the thread using the pool
> at the time.
Yeah, but that is only because the pool was allocated out of the thread pool,
and if we are really working on async, it would have to be allocated out
of the child_pool. That would solve that problem.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: Pools rewrite [2]
Posted by Bill Stoddard <bi...@wstoddard.com>.
> Cliff Woolley wrote:
>
> >>This is my second go at the pools code.
> >>
> >
> >A potential snag with the thread-specific pools idea was brought up today
> >when I was talking with FirstBill and some of the others. What is this
> >going to do to us a little ways down the road when we try to implement
> >async I/O, and all of the sudden requests are jumping from one thread to
> >another? Sounds like a big problem if thread-specific pools are in
> >place... is there an easy answer?
> >
>
> I think there's an easy answer.
>
> The thread-specific pools in this implementation are really
> just pools that happen to own their own free lists. If those
> pools get moved from one thread to another, that's okay, as
> long as only one thread at a time is accessing a given pool
> (which has always been a restriction of the pool API, because
> apr_palloc() isn't thread-safe). In fact, we have this scenario
> already in the worker MPM with Sander's pool code: the
> thread-specific pool for each request gets created by the
> listener thread and handed off to a worker thread.
>
> If a future async implementation has this same property--i.e.,
> pools can be "passed" from one thread to another, but a given
> pool can only have its methods invoked from one thread at a
> time--then we shouldn't have any problems.
>
> --Brian
What happens if a thread exits (under shutdown for instance) and requests are still being
handled by other threads asynchrounously? Looks as if that threads cleanup will free the
pool out from under the thread using the pool at the time.
Bill
Re: Pools rewrite [2]
Posted by Brian Pane <br...@cnet.com>.
Cliff Woolley wrote:
>Okay. The situation (real or imagined) I was leery of was not allocation
>but pool destruction... what happens when you have a subpool of a
>one-thread-at-a-time pool that was created in one thread and gets
>destroyed in another pool, if the parent pool is still active in the other
>thread somehow? I don't have a specific case I can name where this would
>happen, but it seems possible.
>
Fortunately, this scenario doesn't happen in the Apache worker MPM.
The only place where we have a parent-child relationship between pools
in different threads is between the pool in the listener thread and
the transaction pools in each worker. But the pool in the listener
thread is designated as "shared among threads" so that creation or
deletion of its children always requires locking.
It *is* possible for a hypothetical APR app to have the problem
you've described. However, consider what has to happen in order
for this problem to occur: The app has to create a pool, designate
it as one-thread-at-a-time, and then create another object (the
subpool) that has the potential to modify the pool at any time (by
detaching itself from the parent pool). I call that an application
design bug. The best way I can think of to guard against this is
to put a prominent note in the API documentation that says (in
effect):
If you're going to declare a pool as one-thread-at-a-time,
then your application is responsible for ensuring that no
subpools of this pool can be created or destroyed in other
threads.
--Brian
Re: Pools rewrite [2]
Posted by Cliff Woolley <cl...@yahoo.com>.
On Tue, 4 Dec 2001, Brian Pane wrote:
> I think there's an easy answer.
>
> The thread-specific pools in this implementation are really
> just pools that happen to own their own free lists.
>...
>
> If a future async implementation has this same property--i.e.,
> pools can be "passed" from one thread to another, but a given
> pool can only have its methods invoked from one thread at a
> time--then we shouldn't have any problems.
Okay. The situation (real or imagined) I was leery of was not allocation
but pool destruction... what happens when you have a subpool of a
one-thread-at-a-time pool that was created in one thread and gets
destroyed in another pool, if the parent pool is still active in the other
thread somehow? I don't have a specific case I can name where this would
happen, but it seems possible.
[I'm perfectly happy to be told I'm imagining things and this just isn't a
problem... if that's the case, great!]
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: Pools rewrite [2]
Posted by Brian Pane <br...@cnet.com>.
Cliff Woolley wrote:
>>This is my second go at the pools code.
>>
>
>A potential snag with the thread-specific pools idea was brought up today
>when I was talking with FirstBill and some of the others. What is this
>going to do to us a little ways down the road when we try to implement
>async I/O, and all of the sudden requests are jumping from one thread to
>another? Sounds like a big problem if thread-specific pools are in
>place... is there an easy answer?
>
I think there's an easy answer.
The thread-specific pools in this implementation are really
just pools that happen to own their own free lists. If those
pools get moved from one thread to another, that's okay, as
long as only one thread at a time is accessing a given pool
(which has always been a restriction of the pool API, because
apr_palloc() isn't thread-safe). In fact, we have this scenario
already in the worker MPM with Sander's pool code: the
thread-specific pool for each request gets created by the
listener thread and handed off to a worker thread.
If a future async implementation has this same property--i.e.,
pools can be "passed" from one thread to another, but a given
pool can only have its methods invoked from one thread at a
time--then we shouldn't have any problems.
--Brian
RE: Pools rewrite [2]
Posted by Sander Striker <st...@apache.org>.
> From: Cliff Woolley [mailto:cliffwoolley@yahoo.com]
> Sent: 05 December 2001 04:12
>> This is my second go at the pools code.
>
> A potential snag with the thread-specific pools idea was brought up today
> when I was talking with FirstBill and some of the others. What is this
> going to do to us a little ways down the road when we try to implement
> async I/O, and all of the sudden requests are jumping from one thread to
> another? Sounds like a big problem if thread-specific pools are in
> place... is there an easy answer?
>
> --Cliff
The app decides if a pool is thread specific, or put differently, if
a pool will be used only by one thread at a time. The default behaviour
of the new pools code is to behave the exact same way as the original
pools code.
Sander
Re: Pools rewrite [2]
Posted by Cliff Woolley <cl...@yahoo.com>.
> This is my second go at the pools code.
A potential snag with the thread-specific pools idea was brought up today
when I was talking with FirstBill and some of the others. What is this
going to do to us a little ways down the road when we try to implement
async I/O, and all of the sudden requests are jumping from one thread to
another? Sounds like a big problem if thread-specific pools are in
place... is there an easy answer?
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
RE: Pools rewrite [2]
Posted by Sander Striker <st...@apache.org>.
> From: Ryan Bloom [mailto:rbb@covalent.net]
> Sent: 05 December 2001 15:42
> On Wednesday 05 December 2001 03:35 am, Sander Striker wrote:
[...]
>> Damn, I really need to find a way for me not to hit that
>> tab key. Sorry about that, force of habbit that is a)
>> hard to change (working on it though) b) doesn't easily
>> show on (my) screen.
>
> Don't learn how not to hit tab, configure your editor to
> use 4 spaces instead of tab. :-) Every major editor
> can do that.
I did once. And cursed because I couldn't insert tabs into
Makefiles anymore. Also, the sourcecode formatting for
a commercial product I'm working on does have tabs, so that
means switching the config. Next project I'll just force
the apache coding style to be used :)
> Ryan
Sander
Re: Pools rewrite [2]
Posted by Ryan Bloom <rb...@covalent.net>.
On Wednesday 05 December 2001 03:35 am, Sander Striker wrote:
> > From: Justin Erenkrantz [mailto:jerenkrantz@ebuilt.com]
> > Sent: 05 December 2001 03:56
> >
> > On Fri, Nov 30, 2001 at 06:27:14PM +0100, Sander Striker wrote:
> > > Hi,
> > >
> > > This is my second go at the pools code.
> > > I incorporated some suggestions made by Brian Pane.
> > >
> > > Please review,
> >
> > General comments:
> > 1) Lose the tabs.
>
> Damn, I really need to find a way for me not to hit that
> tab key. Sorry about that, force of habbit that is a)
> hard to change (working on it though) b) doesn't easily
> show on (my) screen.
Don't learn how not to hit tab, configure your editor to
use 4 spaces instead of tab. :-) Every major editor
can do that.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: Pools rewrite [2]
Posted by Brian Pane <bp...@pacbell.net>.
Sander Striker wrote:
>So, actually, there isn't much difference between the plain apr_pcalloc
>implementation and the apr_pcalloc macro? (other than code duplication)
>
>The real solution seems to be eliminating apr_pcalloc calls when possible
>(and sensible). Maybe we can have another quantify run to identify the
>most prominent left? Brian?
>
I've done some profiling recently, and there aren't any obvious
candidates for apr_pcalloc elimination left. We've fixed the big
ones already, and the cases that remain are apr_pcallocs of relatively
large structures where only a small subset of the fields are overwritten
right after the pcalloc. The creation of request_recs is a good
example.
--Brian
RE: Pools rewrite [2]
Posted by Sander Striker <st...@apache.org>.
> On Wed, 5 Dec 2001, Sander Striker wrote:
>
> > Right, ok. The 8192 was a number used in the original pools code,
> > I just ripped it :). The BOUNDARY_SIZE is set to be 4096, which
> > is the size of a page on most systems.
>
> i recall doing some statistics gathering and trying to get a single block
> to handle many common requests ...
>
> > > On a thought from Dean Gaudet, how would performance be helped if
> > > we #define'd apr_pcalloc to be:
> > > #define apr_pcalloc(pool, size) memset(apr_palloc(pool, size), '\0', size);
> >
> > When out of mem, this will segfault at the point where the
> > apr_pcalloc macro is used.
>
> on linux (and anything else with optimistic memory allocation) you can't
> check for out of memory just by checking for a NULL. segfault is how you
> find out you ran out of memory. this thread has come up many times ;)
Damn, yes! Keep banging my head on my desk everytime you repeat it :)
> so it's pretty much impossible to do portable out of memory checks.
>
> (if this is puzzling, think about copy-on-write fork semantics... and how
> a pessimistic OS like solaris requires GB of swap which are never used.)
>
> > Secondly, the size is aligned to the next multiple of 8 bytes within
> > apr_pcalloc, this to the advantage of memset. I wonder if taking
> > the memset out of the function will improve performance. I personally
> > doubt it. Dean, care to enlighten me?
>
> i doubt there are compilers that can figure out that the size has been
> aligned inside the apr_pcalloc function.
>
> whereas in the macro version, structure sizes are always naturally aligned
> for the target processor, and the compiler knows that as long as it has
> the constant.
>
> the macro won't really help performance 'cause the cases where calloc is a
> perf problem are typically because the memset itself is overkill.
> (there's a great example of this in EAPI, or at least there was in the
> EAPI that came with the apache on redhat 6.x... i haven't looked to see if
> it was ever fixed. memset of 8KB of otherwise unused memory for every
> request.)
>
> -dean
So, actually, there isn't much difference between the plain apr_pcalloc
implementation and the apr_pcalloc macro? (other than code duplication)
The real solution seems to be eliminating apr_pcalloc calls when possible
(and sensible). Maybe we can have another quantify run to identify the
most prominent left? Brian?
Sander
RE: Pools rewrite [2]
Posted by dean gaudet <de...@arctic.org>.
[thanks for the bcc, sander]
On Wed, 5 Dec 2001, Sander Striker wrote:
> Right, ok. The 8192 was a number used in the original pools code,
> I just ripped it :). The BOUNDARY_SIZE is set to be 4096, which
> is the size of a page on most systems.
i recall doing some statistics gathering and trying to get a single block
to handle many common requests ...
> > On a thought from Dean Gaudet, how would performance be helped if
> > we #define'd apr_pcalloc to be:
> > #define apr_pcalloc(pool, size) memset(apr_palloc(pool, size), '\0', size);
>
> When out of mem, this will segfault at the point where the
> apr_pcalloc macro is used.
on linux (and anything else with optimistic memory allocation) you can't
check for out of memory just by checking for a NULL. segfault is how you
find out you ran out of memory. this thread has come up many times ;)
so it's pretty much impossible to do portable out of memory checks.
(if this is puzzling, think about copy-on-write fork semantics... and how
a pessimistic OS like solaris requires GB of swap which are never used.)
> Secondly, the size is aligned to the next multiple of 8 bytes within
> apr_pcalloc, this to the advantage of memset. I wonder if taking
> the memset out of the function will improve performance. I personally
> doubt it. Dean, care to enlighten me?
i doubt there are compilers that can figure out that the size has been
aligned inside the apr_pcalloc function.
whereas in the macro version, structure sizes are always naturally aligned
for the target processor, and the compiler knows that as long as it has
the constant.
the macro won't really help performance 'cause the cases where calloc is a
perf problem are typically because the memset itself is overkill.
(there's a great example of this in EAPI, or at least there was in the
EAPI that came with the apache on redhat 6.x... i haven't looked to see if
it was ever fixed. memset of 8KB of otherwise unused memory for every
request.)
-dean
RE: Pools rewrite [2]
Posted by Sander Striker <st...@apache.org>.
> From: Justin Erenkrantz [mailto:jerenkrantz@ebuilt.com]
> Sent: 05 December 2001 03:56
> On Fri, Nov 30, 2001 at 06:27:14PM +0100, Sander Striker wrote:
> > Hi,
> >
> > This is my second go at the pools code.
> > I incorporated some suggestions made by Brian Pane.
> >
> > Please review,
>
> General comments:
> 1) Lose the tabs.
Damn, I really need to find a way for me not to hit that
tab key. Sorry about that, force of habbit that is a)
hard to change (working on it though) b) doesn't easily
show on (my) screen.
> 2) It probably needs to be modified to use the new locking API.
> I don't see this as a showstopper, but I think it's something
> that should happen and may help with the locking performance.
> Aaron can speak more about this...
It already does per Brians suggestion after my first post.
> Rest of my comments are inline (denoted by <comment>). (Are you
> using Outlook? If not, please inline your patches...)
I'm using LookOut :). I'm going to inline and attach (as an experiment).
There is an option in outlook that lets it wrap lines at col 132 instead
of the default 76 or 78. For the interested:
Tools -> Options -> Mail Format -> Settings (Message Format).
> My comments aren't really about the core logic though. I know I
> reviewed the overall logic and found it correct before. I'm going
> to assume that the core memory logic hasn't changed.
Not much anyway AFAIK, it's been a while :)
> And, Brian and Ian's testing prove that it works in practice, so I'll
> look at the logic again when I have more time. -- justin
Ack.
> ------
> <snip, snip>
> /*
> * Magic numbers
> */
>
> #define MIN_ALLOC 8192
> #define MAX_INDEX 16
>
> #define BOUNDARY_INDEX 12
> #define BOUNDARY_SIZE (1 << BOUNDARY_INDEX)
>
> <comment>
> I think I mentioned it before, but it'd probably be good to
> indicate rationale why we think these values are good. I'm
> not disagreeing with them, but I think it might be helpful to
> explain how they fit in the overall scheme of things.
>
> And, if this goes in, I'll probably do the same thing I did
> with SMS and make a pass that adds source code comments. That
> won't happen until next week at the earliest.
> </comment>
Right, ok. The 8192 was a number used in the original pools code,
I just ripped it :). The BOUNDARY_SIZE is set to be 4096, which
is the size of a page on most systems.
> /*
> * Macros and defines
> */
>
> /* ALIGN() is only to be used to align on a power of 2 boundary */
> #define ALIGN(size, boundary) \
> (((size) + ((boundary) - 1)) & ~((boundary) - 1))
>
> #define ALIGN_DEFAULT(size) ALIGN(size, 8)
>
> #define LOCK(mutex) \
> if (mutex) \
> apr_thread_mutex_lock(mutex);
>
> #define UNLOCK(mutex) \
> if (mutex) \
> apr_thread_mutex_unlock(mutex);
>
> <comment>
> Ack. No. Declare a new scope like so:
> #define UNLOCK(mutex) \
> do { \
> if (mutex) \
> apr_thread_mutex_unlock(mutex); \
> } while(0)
>
> Calling it like UNLOCK(mutex) (i.e. no semicolon) is just badness
> and will shoot us down the road later.
> </comment>
Ok, will change that.
> <snip, snip>
> /* apr_pcalloc is almost exactly the same as apr_palloc, except for
> * a few memset()s. This saves an extra function call though, which
> * is enough justification for this code duplication IMO.
> */
>
> <comment>
> On a thought from Dean Gaudet, how would performance be helped if
> we #define'd apr_pcalloc to be:
> #define apr_pcalloc(pool, size) memset(apr_palloc(pool, size), '\0', size);
>
> The key idea is that Dean pointed out that the compiler can do a
> *much* better job of optimizing if we don't stick the memset deep
> down in a function somewhere. I'd be curious to see how this
> optimization would help. And, I think code duplication is evil
> anyway. This has the potential to be faster than apr_pcalloc and
> use the same code.
> </comment>
When out of mem, this will segfault at the point where the
apr_pcalloc macro is used.
Secondly, the size is aligned to the next multiple of 8 bytes within
apr_pcalloc, this to the advantage of memset. I wonder if taking
the memset out of the function will improve performance. I personally
doubt it. Dean, care to enlighten me?
> APR_DECLARE(void *) apr_pcalloc(apr_pool_t *pool, apr_size_t size)
> {
>
> <snip, snip>
>
> /*
> * Initialization
> */
>
> APR_DECLARE(apr_status_t) apr_pool_alloc_init(apr_pool_t *global)
> {
> apr_status_t rv;
>
> if (global_allocator_initialized)
> return APR_SUCCESS; /* Is this correct? */
>
> <comment>
> Sure. apr_thread_once is probably a much better way to handle this
> though I think.
> </comment>
My point was if it was correct to return APR_SUCCESS. People are
initializing twice if they reach that line of code.
Isn't apr_thread_once restricted to platforms where we actually have
threads? Hmmm, I now start to wonder about my apr_thread_mutex_xxx
calls. Aaron?
> memset(&global_allocator, 0, SIZEOF_ALLOCATOR_T);
>
> if ((rv = apr_thread_mutex_create(&global_allocator.mutex,
> APR_THREAD_MUTEX_DEFAULT, global)) != APR_SUCCESS) {
> return rv;
> }
>
> global_allocator.owner = global;
> global_pool = global;
> global_allocator_initialized = 1;
>
> return APR_SUCCESS;
> }
>
> <snip, snip>
APR_DECLARE(void) apr_pool_alloc_term(apr_pool_t *pool)
{
/*
* XXX: What happens when global_pool != pool?
* IMHO apr_pool_alloc_term should take void, not an
* apr_pool_t *.
*/
This one bothered me personally. The current pools implementation
assumes that pool == permanent_pool IIRC. Could apr_pool_alloc_term()
be changed to just take void? [lousy wording, hope you know what I
mean].
Sander