On 08.05.2011 18:40, Daniel Shahaf wrote:
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 ?
For speed. This is on a critical path (cache lookups are frequent
and modulo is a very expensive operation). The previous code
also used the bitwise operator because the compiler knew that
CACHE_SEGMENTS was a power of 2.
/* 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.
There was a less obvious issue behind that as segment_count_shift
being 32 bits makes the whole left-hand side 32 bits only.
Fixed in r1101091.
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?)
Exactly.
@@ -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.
Fixed in r1101091.
-- Stefan^2.
