You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by rb...@covalent.net on 2000/12/01 15:38:23 UTC
Re: cvs commit: apr/locks/unix locks.c
> Modified: . configure.in hints.m4 CHANGES
> include apr.h.in apr.hw
> locks/unix locks.c
> Log:
> New config variable apr_process_lock_is_global specifies that the selected
> inter-process lock method is sufficient for APR_LOCKALL (i.e., it blocks
> all threads and processes). For now, hints.m4 turns on this flag for
> OS/390.
MHO this was put in the wrong place. This is an internal APR flag, and
should not be in apr.h at all. Really what this is, is an optimization,
so that we don't create an intraprocess lock if the cross-process lock
will lock the threads. Locks and how they are setup was an area of great
contention between Manoj and myself when I was designing them, because of
lockall and cross-process locks.
I am favor of this concept, but I would like the definition hidden from
the program using APR, because I think it just confuses things. The
program should feel free to choose the correct lock type based on need, so
it they need lockall, then they should just choose lockall, and not try to
second guess APR, that cross-process would also work. This patch opens
the way for people to try to second guess the code.
Just to be clear again, ++1 on the concept just wouldd like the def moved
from apr.h to apr_private.h (also requires backing out the Apache change).
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: apr/locks/unix locks.c
Posted by rb...@covalent.net.
> > Then please just leave it, and I'll implement the whole thing myself. The
> > other option is for you to implement it, and I'll go in immediately and
> > change it.
>
> Why is it that two people ask for the simple one, don't want to do the
> harder one (and don't want to see that complexity in there), yet you will
> just go and do it anyways?
Because APR was designed to be small pieces that don't necessarily know
about each other. There is a minimum amount of code that creates
cross-dependancies between different sections of APR. It is not that much
more complex, and it helps to maintain the abstraction. It is important
to me to ensure that each piece of APR can build individually without
losing much of it's functionality. By putting all of this into one big
function in misc/unix/start.c, you remove that ability, because we rely on
misc to report on file_io.
Now, go back and read Jeff's reply:
> > That is for damn sure :)^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H
> > I'll start out with all the logic right in cfg.c and as other folks
> > have time/inclination they can consider other implementations.
I have the time/inclination.
That is why I am willing to implement something that I believe is the
correct implementation. Personally, I think it is a bit less complex than
a single function that is basically one large:
#if APR_HAS_THREADS
return APR_THREADS_PTHREADS
#endif
#if APR_HAS_LOCKS
return APR_LOCKS_FOOBAR
#endif
...
I also know that I will be taking small pieces of APR and putting them in
my own projects. I will most likely want to have the reporting features
that we are talking about.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: apr/locks/unix locks.c
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Dec 01, 2000 at 02:29:43PM -0800, rbb@covalent.net wrote:
>
> > > I think that is overkill for now. All of these flags are available in
> > > apr_private.h. If it starts to get unwieldy or something, then we can split.
> > > But let's start simple, make it harder when that is needed.
> >
> > That is for damn sure :)^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H
> > I'll start out with all the logic right in cfg.c and as other folks
> > have time/inclination they can consider other implementations.
>
> Then please just leave it, and I'll implement the whole thing myself. The
> other option is for you to implement it, and I'll go in immediately and
> change it.
Why is it that two people ask for the simple one, don't want to do the
harder one (and don't want to see that complexity in there), yet you will
just go and do it anyways?
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apr/locks/unix locks.c
Posted by rb...@covalent.net.
> > I think that is overkill for now. All of these flags are available in
> > apr_private.h. If it starts to get unwieldy or something, then we can split.
> > But let's start simple, make it harder when that is needed.
>
> That is for damn sure :)^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H
> I'll start out with all the logic right in cfg.c and as other folks
> have time/inclination they can consider other implementations.
Then please just leave it, and I'll implement the whole thing myself. The
other option is for you to implement it, and I'll go in immediately and
change it.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: apr/locks/unix locks.c
Posted by Jeff Trawick <tr...@bellsouth.net>.
Greg Stein <gs...@lyra.org> writes:
> On Fri, Dec 01, 2000 at 08:44:14AM -0800, rbb@covalent.net wrote:
> >
> > This should work the same way apr_initialize works, so that it calls into
> > the APR sub-libraries with a function like "apr_report_lock" or
> > "apr_report_threads". This would make it very extensible, and as we build
> > APR with or without different components, we would just pick up the right
> > information.
>
> I think that is overkill for now. All of these flags are available in
> apr_private.h. If it starts to get unwieldy or something, then we can split.
> But let's start simple, make it harder when that is needed.
That is for damn sure :)^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H^H
I'll start out with all the logic right in cfg.c and as other folks
have time/inclination they can consider other implementations.
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: cvs commit: apr/locks/unix locks.c
Posted by rb...@covalent.net.
> > This should work the same way apr_initialize works, so that it calls into
> > the APR sub-libraries with a function like "apr_report_lock" or
> > "apr_report_threads". This would make it very extensible, and as we build
> > APR with or without different components, we would just pick up the right
> > information.
>
> I think that is overkill for now. All of these flags are available in
> apr_private.h. If it starts to get unwieldy or something, then we can split.
> But let's start simple, make it harder when that is needed.
I really disagree. Each section of APR is segregated for a reason. Let
those sections handle this themselves. This won't be hard to code, and in
general it will make the code much cleaner and easier to deal with later.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: apr/locks/unix locks.c
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Dec 01, 2000 at 08:44:14AM -0800, rbb@covalent.net wrote:
>
> > Why don't we add something like apr_get_config_string() which returns
> > a string like the "httpd -V" output (but of course only builds the APR
> > part)?
>
> How many +1s can I give this? this would be incredibly useful.
+1 here, too.
> > include/apr_general.h:
> >
> > apr_status_t apr_get_config_string(char **cfg, apr_pool_t *cont);
> >
> > misc/unix/cfg.c:
> >
> > put the function here; any other functions dealing with config tests
> > could go here too
+1
> This should work the same way apr_initialize works, so that it calls into
> the APR sub-libraries with a function like "apr_report_lock" or
> "apr_report_threads". This would make it very extensible, and as we build
> APR with or without different components, we would just pick up the right
> information.
I think that is overkill for now. All of these flags are available in
apr_private.h. If it starts to get unwieldy or something, then we can split.
But let's start simple, make it harder when that is needed.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apr/locks/unix locks.c
Posted by rb...@covalent.net.
> Why don't we add something like apr_get_config_string() which returns
> a string like the "httpd -V" output (but of course only builds the APR
> part)?
How many +1s can I give this? this would be incredibly useful.
> include/apr_general.h:
>
> apr_status_t apr_get_config_string(char **cfg, apr_pool_t *cont);
>
> misc/unix/cfg.c:
>
> put the function here; any other functions dealing with config tests
> could go here too
This should work the same way apr_initialize works, so that it calls into
the APR sub-libraries with a function like "apr_report_lock" or
"apr_report_threads". This would make it very extensible, and as we build
APR with or without different components, we would just pick up the right
information.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: apr/locks/unix locks.c
Posted by Jeff Trawick <tr...@bellsouth.net>.
rbb@covalent.net writes:
> > Modified: . configure.in hints.m4 CHANGES
> > include apr.h.in apr.hw
> > locks/unix locks.c
> > Log:
> > New config variable apr_process_lock_is_global specifies that the selected
> > inter-process lock method is sufficient for APR_LOCKALL (i.e., it blocks
> > all threads and processes). For now, hints.m4 turns on this flag for
> > OS/390.
>
> MHO this was put in the wrong place. This is an internal APR flag, and
> should not be in apr.h at all. Really what this is, is an optimization,
> so that we don't create an intraprocess lock if the cross-process lock
> will lock the threads. Locks and how they are setup was an area of great
> contention between Manoj and myself when I was designing them, because of
> lockall and cross-process locks.
I agree 99%. App code shouldn't give a !@#$. The remaining 1% is
that it can be extremely useful for apps to tell their users how they
were built.
Forget APR_PROCESS_LOCK_IS_GLOBAL for a sec and consider
APR_USE_xyz_SERIALIZE. Apache code doesn't give a rip about it except
for generating the "htpd -V" output. That has been quite useful
because of the large number of times the lock mechanism in use comes
up in discussions of performance and to a lesser extent brokenness. I
consider APR_PROCESS_LOCK_IS_GLOBAL to be important too.
Back to reality though:
Why don't we add something like apr_get_config_string() which returns
a string like the "httpd -V" output (but of course only builds the APR
part)?
include/apr_general.h:
apr_status_t apr_get_config_string(char **cfg, apr_pool_t *cont);
misc/unix/cfg.c:
put the function here; any other functions dealing with config tests
could go here too
With this in hand, we can decide that certain feature tests macros
aren't needed by apps and move them out of apr.h.
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...