You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by yl...@apache.org on 2018/04/03 22:00:10 UTC
svn commit: r1828289 - in /apr/apr/trunk: CHANGES include/apr_reslist.h
util-misc/apr_reslist.c
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 Thu, Apr 5, 2018 at 2:25 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Thu, Apr 5, 2018 at 2:02 AM, Eric Covener <co...@gmail.com> wrote:
>> Would this help the mod_proxy CLOSE_WAIT issue in BZ?
>
> Yes I think so (when MaxSpareThreads/Servers are too high to do their
> job, for some reason, otherwise it's not an issue IMHO).
>
> I was also thinking of a new apr_reslist_checker_set() too, where
> setting checker callback (like the current constructor and destructor)
> could be used to determine whether an (about-to-be-)acquired resource
> is still valid before returning it to the caller (otherwise destroy it
> and return the next one).
> In the mod_proxy case, the checker could be based on
> ap_proxy_is_socket_connected() or (better)
> ap_proxy_check_connection()..
But anyway, in the best case, it'd require APR-1.7...
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 Thu, Apr 5, 2018 at 2:02 AM, Eric Covener <co...@gmail.com> wrote:
> Would this help the mod_proxy CLOSE_WAIT issue in BZ?
Yes I think so (when MaxSpareThreads/Servers are too high to do their
job, for some reason, otherwise it's not an issue IMHO).
I was also thinking of a new apr_reslist_checker_set() too, where
setting checker callback (like the current constructor and destructor)
could be used to determine whether an (about-to-be-)acquired resource
is still valid before returning it to the caller (otherwise destroy it
and return the next one).
In the mod_proxy case, the checker could be based on
ap_proxy_is_socket_connected() or (better)
ap_proxy_check_connection()..
Fwd: svn commit: r1828289 - in /apr/apr/trunk: CHANGES
include/apr_reslist.h util-misc/apr_reslist.c
Posted by Eric Covener <co...@gmail.com>.
Would this help the mod_proxy CLOSE_WAIT issue in BZ?
---------- Forwarded message ----------
From: <yl...@apache.org>
Date: Tue, Apr 3, 2018 at 6:00 PM
Subject: svn commit: r1828289 - in /apr/apr/trunk: CHANGES
include/apr_reslist.h util-misc/apr_reslist.c
To: commits@apr.apache.org
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;
--
Eric Covener
covener@gmail.com
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.
Re: svn commit: r1828289 - in /apr/apr/trunk: CHANGES
include/apr_reslist.h util-misc/apr_reslist.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
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;
>
>