A few questions:

stef...@apache.org wrote on Sun, May 08, 2011 at 13:48:33 -0000:
> Author: stefan2
> Date: Sun May  8 13:48:33 2011
> New Revision: 1100738
> 
> URL: http://svn.apache.org/viewvc?rev=1100738&view=rev
> Log:
> Make membuffer cache segmentation dynamic, i.e. don't segment
> smaller caches. That will allow for much larger objects to be cached
> such as large directories. 
> 
> Remember: max cachable item size = cache size / (4 * #segments)
> 
> * subversion/libsvn_subr/cache-membuffer.c
>   (CACHE_SEGMENTS): drop constant
>   (MIN_SEGMENT_SIZE): introduce segmentation threshold constant
>   (svn_membuffer_t): add segment count variable
>   (get_group_index): adapt cache segment selection
>   (svn_cache__membuffer_cache_create): determine number of segments 
> dynamically
>   (svn_membuffer_cache_get_info): adapt to variable segment count
> 
> Modified:
>     subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> 
> Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1100738&r1=1100737&r2=1100738&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Sun May  8 
> 13:48:33 2011
> @@ -619,7 +621,7 @@ get_group_index(svn_membuffer_t **cache,
>    memcpy(to_find, checksum->digest, APR_MD5_DIGESTSIZE);
>  
>    /* select the cache segment to use */
> -  *cache = &(*cache)[to_find[0] % CACHE_SEGMENTS];
> +  *cache = &(*cache)[to_find[0] & ((*cache)->segment_count -1)];
>  

Why did you switch from modulo to bitwise-and ?

>    /* Get the group that *must* contain the entry. Fold the hash value 
>     * just to be sure (it should not be necessary for perfect hashes).
> @@ -903,16 +905,38 @@ svn_cache__membuffer_cache_create(svn_me
>                                    svn_boolean_t thread_safe,
>                                    apr_pool_t *pool)
>  {
> -  /* allocate cache as an array of segments / cache objects */
> -  svn_membuffer_t *c = apr_palloc(pool, CACHE_SEGMENTS * sizeof(*c));
> +  svn_membuffer_t *c;
> +
> +  apr_uint32_t segment_count_shift = 0;
> +  apr_uint32_t segment_count = 1;
> +
>    apr_uint32_t seg;
>    apr_uint32_t group_count;
>    apr_uint64_t data_size;
>  
> +  /* Determine a reasonable number of cache segments. Segmentation is
> +   * only useful for multi-threaded / multi-core servers as it reduces
> +   * lock contention on these systems.
> +   *
> +   * But on these systems, we can assume that ample memory has been
> +   * allocated to this cache. Smaller caches should not be segmented
> +   * as this severely limites the maximum size of cachable items.
> +   *
> +   * Segments should not be smaller than 32MB and max. cachable item
> +   * size should grow as fast as segmentation.
> +   */
> +  while ((2 * MIN_SEGMENT_SIZE << (2 * segment_count_shift)) < total_size)
> +    ++segment_count_shift;
> +

x * y << z, confusing to parse for precedence, but the result is the same 
either way.

As to shifting by 2*segment_count_shift... if I understand correctly
that is in order to balance the size of a segment[1] v. the number of segments?

[1]  (and via that the maximum cacheable item size?)

> @@ -942,10 +966,12 @@ svn_cache__membuffer_cache_create(svn_me
>        directory_size = group_count * sizeof(entry_group_t);
>      }
>  
> -  for (seg = 0; seg < CACHE_SEGMENTS; ++seg)
> +    for (seg = 0; seg < segment_count; ++seg)
>      {

Wrong indentation change.

> @@ -1654,7 +1680,7 @@ svn_membuffer_cache_get_info(void *cache
>    info->used_entries = 0;
>    info->total_entries = 0;
>  
> -  for (i = 0; i < CACHE_SEGMENTS; ++i)
> +  for (i = 0; i < cache->membuffer->segment_count; ++i)
>      {
>        svn_membuffer_t *segment = cache->membuffer + i;
>  
> 
> 

Thanks,

Daniel

Reply via email to