You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by ja...@apache.org on 2017/06/02 19:58:55 UTC

svn commit: r1797422 - /apr/apr/trunk/memcache/apr_memcache.c

Author: jailletc36
Date: Fri Jun  2 19:58:55 2017
New Revision: 1797422

URL: http://svn.apache.org/viewvc?rev=1797422&view=rev
Log:
Save some cycles by not copying the pollfds.

Modified:
    apr/apr/trunk/memcache/apr_memcache.c

Modified: apr/apr/trunk/memcache/apr_memcache.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/memcache/apr_memcache.c?rev=1797422&r1=1797421&r2=1797422&view=diff
==============================================================================
--- apr/apr/trunk/memcache/apr_memcache.c (original)
+++ apr/apr/trunk/memcache/apr_memcache.c Fri Jun  2 19:58:55 2017
@@ -1296,7 +1296,8 @@ apr_memcache_multgetp(apr_memcache_t *mc
     /* create polling structures */
     pollfds = apr_pcalloc(temp_pool, apr_hash_count(server_queries) * sizeof(apr_pollfd_t));
     
-    rv = apr_pollset_create(&pollset, apr_hash_count(server_queries), temp_pool, 0);
+    rv = apr_pollset_create(&pollset, apr_hash_count(server_queries), temp_pool,
+                            APR_POLLSET_NOCOPY);
 
     if (rv != APR_SUCCESS) {
         query_hash_index = apr_hash_first(temp_pool, server_queries);



Re: svn commit: r1797422 - /apr/apr/trunk/memcache/apr_memcache.c

Posted by Eric Covener <co...@gmail.com>.
On Mon, Jun 5, 2017 at 4:32 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> What happens if two different threads attempt this poll in parallel? I
> presumed pollsets are copied to prevent two threads from clashing.

>>      pollfds = apr_pcalloc(temp_pool, apr_hash_count(server_queries) *
>> sizeof(apr_pollfd_t));
>>
>> -    rv = apr_pollset_create(&pollset, apr_hash_count(server_queries),
>> temp_pool, 0);
>> +    rv = apr_pollset_create(&pollset, apr_hash_count(server_queries),
>> temp_pool,
>> +                            APR_POLLSET_NOCOPY);

Looks kosher, the pollfds that will be added to this pollset have the
same lifetime / same pool (beginning of the diff context), and the
pollset does not escape this function.

Re: svn commit: r1797422 - /apr/apr/trunk/memcache/apr_memcache.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jun 5, 2017 at 10:32 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> What happens if two different threads attempt this poll in parallel? I
> presumed pollsets are copied to prevent two threads from clashing.

APR_POLLSET_NOCOPY is about the lifetime of the pollfds, while
thread-safety should be ensured by APR_POLLSET_THREADSAFE (using
no/zero flags as before was actually not thread-safe already).

BTW, if the OS/native pollset is already threadsafe and
APR_POLLSET_NOCOPY is specified, APR_POLLSET_THREADSAFE can sometimes
be omitted/ignored (this the case for e.g. epoll, but not for kqueue
whereas it possibly could be).
Hence if we really need to be thread-safe here we should use
APR_POLLSET_THREADSAFE explicitely.

Re: svn commit: r1797422 - /apr/apr/trunk/memcache/apr_memcache.c

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
What happens if two different threads attempt this poll in parallel? I
presumed pollsets are copied to prevent two threads from clashing.


On Jun 2, 2017 14:58, <ja...@apache.org> wrote:

> Author: jailletc36
> Date: Fri Jun  2 19:58:55 2017
> New Revision: 1797422
>
> URL: http://svn.apache.org/viewvc?rev=1797422&view=rev
> Log:
> Save some cycles by not copying the pollfds.
>
> Modified:
>     apr/apr/trunk/memcache/apr_memcache.c
>
> Modified: apr/apr/trunk/memcache/apr_memcache.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/memcache/apr_
> memcache.c?rev=1797422&r1=1797421&r2=1797422&view=diff
> ============================================================
> ==================
> --- apr/apr/trunk/memcache/apr_memcache.c (original)
> +++ apr/apr/trunk/memcache/apr_memcache.c Fri Jun  2 19:58:55 2017
> @@ -1296,7 +1296,8 @@ apr_memcache_multgetp(apr_memcache_t *mc
>      /* create polling structures */
>      pollfds = apr_pcalloc(temp_pool, apr_hash_count(server_queries) *
> sizeof(apr_pollfd_t));
>
> -    rv = apr_pollset_create(&pollset, apr_hash_count(server_queries),
> temp_pool, 0);
> +    rv = apr_pollset_create(&pollset, apr_hash_count(server_queries),
> temp_pool,
> +                            APR_POLLSET_NOCOPY);
>
>      if (rv != APR_SUCCESS) {
>          query_hash_index = apr_hash_first(temp_pool, server_queries);
>
>
>