You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by William A Rowe Jr <wr...@rowe-clan.net> on 2018/04/04 16:38:24 UTC
Re: svn commit: r1828289 - in /apr/apr/trunk: CHANGES
include/apr_reslist.h util-misc/apr_reslist.c
One question here;
isn't it more efficient to offer push_resource_fifo() and push_resource_lifo()
API's to the user, as opposed to extra conditionals? In theory, the consumer
developer knows which ring they want.
On Tue, Apr 3, 2018 at 5:00 PM, <yl...@apache.org> wrote:
> Author: ylavic
> Date: Tue Apr 3 22:00:10 2018
> New Revision: 1828289
>
> URL: http://svn.apache.org/viewvc?rev=1828289&view=rev
> Log:
> reslist: Add apr_reslist_fifo_set().
>
> This allows to reuse resources in FIFO mode instead of the default LIFO mode.
>
>
> Modified:
> apr/apr/trunk/CHANGES
> apr/apr/trunk/include/apr_reslist.h
> apr/apr/trunk/util-misc/apr_reslist.c
>
> Modified: apr/apr/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/CHANGES?rev=1828289&r1=1828288&r2=1828289&view=diff
> ==============================================================================
> --- apr/apr/trunk/CHANGES [utf-8] (original)
> +++ apr/apr/trunk/CHANGES [utf-8] Tue Apr 3 22:00:10 2018
> @@ -1,6 +1,9 @@
> -*- coding: utf-8 -*-
> Changes for APR 2.0.0
>
> + *) reslist: Add apr_reslist_fifo_set() to allow for reusing resources
> + in FIFO mode instead of the default LIFO mode. [Yann Ylavic]
> +
> *) Add apr_pool_tag_get to retrieve the pool tag name. [Joe Orton]
>
> *) Add apr_sockaddr_zone_set, apr_sockaddr_zone_set to set and retrieve
>
> Modified: apr/apr/trunk/include/apr_reslist.h
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/apr_reslist.h?rev=1828289&r1=1828288&r2=1828289&view=diff
> ==============================================================================
> --- apr/apr/trunk/include/apr_reslist.h (original)
> +++ apr/apr/trunk/include/apr_reslist.h Tue Apr 3 22:00:10 2018
> @@ -134,6 +134,13 @@ APR_DECLARE(apr_status_t) apr_reslist_re
> */
> APR_DECLARE(void) apr_reslist_timeout_set(apr_reslist_t *reslist,
> apr_interval_time_t timeout);
> +/**
> + * Set whether the reslist reuses resources as FIFO (First In First Out) or
> + * LIFO (Last In First Out).
> + * @param reslist The resource list.
> + * @param fifo Set as FIFO (non zero) or LIFO (zero).
> + */
> +APR_DECLARE(void) apr_reslist_fifo_set(apr_reslist_t *reslist, int to);
>
> /**
> * Return the number of outstanding resources.
>
> Modified: apr/apr/trunk/util-misc/apr_reslist.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/util-misc/apr_reslist.c?rev=1828289&r1=1828288&r2=1828289&view=diff
> ==============================================================================
> --- apr/apr/trunk/util-misc/apr_reslist.c (original)
> +++ apr/apr/trunk/util-misc/apr_reslist.c Tue Apr 3 22:00:10 2018
> @@ -58,6 +58,7 @@ struct apr_reslist_t {
> apr_thread_mutex_t *listlock;
> apr_thread_cond_t *avail;
> #endif
> + int fifo;
> };
>
> /**
> @@ -80,7 +81,12 @@ static apr_res_t *pop_resource(apr_resli
> */
> static void push_resource(apr_reslist_t *reslist, apr_res_t *resource)
> {
> - APR_RING_INSERT_HEAD(&reslist->avail_list, resource, apr_res_t, link);
> + if (reslist->fifo) {
> + APR_RING_INSERT_TAIL(&reslist->avail_list, resource, apr_res_t, link);
> + }
> + else {
> + APR_RING_INSERT_HEAD(&reslist->avail_list, resource, apr_res_t, link);
> + }
> resource->freed = apr_time_now();
> reslist->nidle++;
> }
> @@ -434,6 +440,11 @@ APR_DECLARE(void) apr_reslist_timeout_se
> reslist->timeout = timeout;
> }
>
> +APR_DECLARE(void) apr_reslist_fifo_set(apr_reslist_t *reslist, int to)
> +{
> + reslist->fifo = to;
> +}
> +
> APR_DECLARE(apr_uint32_t) apr_reslist_acquired_count(apr_reslist_t *reslist)
> {
> apr_uint32_t count;
>
>
Re: svn commit: r1828289 - in /apr/apr/trunk: CHANGES
include/apr_reslist.h util-misc/apr_reslist.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Apr 4, 2018 at 6:57 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Wed, Apr 4, 2018 at 6:38 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>> One question here;
>>
>> isn't it more efficient to offer push_resource_fifo() and push_resource_lifo()
>> API's to the user, as opposed to extra conditionals? In theory, the consumer
>> developer knows which ring they want.
>
> So we'd have two more _fifo versions of apr_reslist_release() and
> apr_reslist_maintain() if we wanted to avoid the test somewhere.
> Given that we already acquire the "listlock" mutex and signal the
> "avail" condvar, this new test is pretty much in the noise already,
> it's not worth duplicating the above functions IMHO.
Not to talk about apr_time_now() called on every push_resource()..
Re: svn commit: r1828289 - in /apr/apr/trunk: CHANGES
include/apr_reslist.h util-misc/apr_reslist.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Apr 4, 2018 at 6:38 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> One question here;
>
> isn't it more efficient to offer push_resource_fifo() and push_resource_lifo()
> API's to the user, as opposed to extra conditionals? In theory, the consumer
> developer knows which ring they want.
So we'd have two more _fifo versions of apr_reslist_release() and
apr_reslist_maintain() if we wanted to avoid the test somewhere.
Given that we already acquire the "listlock" mutex and signal the
"avail" condvar, this new test is pretty much in the noise already,
it's not worth duplicating the above functions IMHO.