httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marion & Christophe JAILLET <christophe.jail...@wanadoo.fr>
Subject Re: [PATCH] Reduce memory footprint in mod_dav's property code
Date Tue, 22 Dec 2015 07:10:03 GMT
You can also have a look at 
https://bz.apache.org/bugzilla/show_bug.cgi?id=48130

Le 22/12/2015 01:13, Stefan Fuhrmann a écrit :
> Hi,
>
> I stumbled over this while investigation an OOM report from a
> Subversion user [1].  Due to unfortunate circumstances [2], I've
> seen directory listings with a few 10000 entries eat 10s of GB
> of RAM.  Those circumstances are being addressed but the root
> cause is in mod_dav and the patch below fixes it.
>
> The problem is that dav_open_propdb creates a pool that
> dav_close_propdb can't destroy.  So, for every property, at least
> 8 KB of memory will be allocated.  If we simply use the parent
> pool as is, only a few 100 bytes will be used.
>
> Moreover, if the APR allocator should return a larger block once
> in a while, that block will get used up nicely before more memory
> is allocated.  So with this patch, the worst case will still use
> only about to 1/10th of the current best case for large collections.
>
> -- Stefan^2.
>
> [1] 
> http://mail-archives.apache.org/mod_mbox/subversion-users/201512.mbox/%3C1CEE115D02633942A40D49D447DCF46E432F3A32%40SD01CFMM0202.OMEGA.DCE-EIR.NET%3E
> [2] 
> http://mail-archives.apache.org/mod_mbox/subversion-dev/201512.mbox/%3C5676A54F.9040609%40apache.org%3E
>
>
> [[[
> Fix a pool handling asymmetry in mod_dav that caused inefficient
> memory usage with collection properties.
>
> If we can't clear nor destroy the local pool in dav_propdb, there
> is no point in creating it as a sub-pool of its parent.
>
> * modules/dav/main/props.c
>   (DAV_PROPDB_LOCAL_POOL): New compile-time switch replacing the
>                            plain "#if 0" in dav_close_propdb.  It
>                            ensures consistent behaviour between
>                            open and close in the future.
>   (dav_open_propdb): If we can't clear the pool, we may as well use
>                      its parent directly.
>   (dav_close_propdb): Use the new define.
> ]]]
>
> [[[
> Index: modules/dav/main/props.c
> ===================================================================
> --- modules/dav/main/props.c    (revision 1721073)
> +++ modules/dav/main/props.c    (working copy)
> @@ -167,6 +167,16 @@
>
>  #define DAV_EMPTY_VALUE                "\0"    /* TWO null terms */
>
> +/*
> +** Currently, mod_dav's pool usage doesn't allow clearing the pool
> +** at dav_propdb.p . Therefore, we won't create a sub-pool for it
> +** and use the request's pool directly instead.
> +**
> +** Once the pool usage issue has been fixed, set this to 1 for optimal
> +** memory usage.
> +*/
> +#define DAV_PROPDB_LOCAL_POOL          0
> +
>  struct dav_propdb {
>      apr_pool_t *p;                /* the pool we should use */
>      request_rec *r;               /* the request record */
> @@ -537,7 +547,11 @@
>  #endif
>
>      propdb->r = r;
> +#if DAV_PROPDB_LOCAL_POOL
>      apr_pool_create(&propdb->p, r->pool);
> +#else
> +    propdb->p = r->pool;
> +#endif
>      propdb->resource = resource;
>      propdb->ns_xlate = ns_xlate;
>
> @@ -562,8 +576,7 @@
>          (*propdb->db_hooks->close)(propdb->db);
>      }
>
> -    /* Currently, mod_dav's pool usage doesn't allow clearing this 
> pool. */
> -#if 0
> +#if DAV_PROPDB_LOCAL_POOL
>      apr_pool_destroy(propdb->p);
>  #endif
>  }
> ]]]
>


Mime
View raw message