You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/04/16 15:06:07 UTC
svn commit: r1093983 - in /subversion/trunk/subversion:
include/private/svn_cache.h libsvn_fs_fs/caching.c
libsvn_subr/cache-inprocess.c libsvn_subr/cache-membuffer.c
libsvn_subr/cache-memcache.c libsvn_subr/cache.c libsvn_subr/cache.h
Author: stefan2
Date: Sat Apr 16 13:06:07 2011
New Revision: 1093983
URL: http://svn.apache.org/viewvc?rev=1093983&view=rev
Log:
In symmetry to svn_cache__get_partial, introduce a partial
setter / modifier API function that enabled the caller to
modify changed items without the full deserialization -
modification - serialization chain. Therefore, adding entries
to e.g. directories becomes O(1) instead of O(n).
* subversion/include/private/svn_cache.h
(svn_cache__partial_setter_func_t): declare new callback
(svn_cache__set_partial): declare new API function
* subversion/libsvn_subr/cache.h
(svn_cache__vtable_t): add entry for new API function
* subversion/libsvn_subr/cache.c
(svn_cache__set_partial): implement
* subversion/libsvn_subr/cache-inprocess.c
(inprocess_cache_set_partial): implement for inprocess cache
(inprocess_cache_vtable): extend vtable
* subversion/libsvn_subr/cache-memcache.c
(memcache_set_partial): implement for memcache cache
(memcache_vtable): extend vtable
* subversion/libsvn_subr/cache-membuffer.c
(svn_membuffer_cache_set_partial, membuffer_cache_set_partial):
implement for membuffer cache
(membuffer_cache_vtable): extend vtable
Modified:
subversion/trunk/subversion/include/private/svn_cache.h
subversion/trunk/subversion/libsvn_fs_fs/caching.c
subversion/trunk/subversion/libsvn_subr/cache-inprocess.c
subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
subversion/trunk/subversion/libsvn_subr/cache-memcache.c
subversion/trunk/subversion/libsvn_subr/cache.c
subversion/trunk/subversion/libsvn_subr/cache.h
Modified: subversion/trunk/subversion/include/private/svn_cache.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_cache.h?rev=1093983&r1=1093982&r2=1093983&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_cache.h (original)
+++ subversion/trunk/subversion/include/private/svn_cache.h Sat Apr 16 13:06:07 2011
@@ -71,6 +71,19 @@ typedef svn_error_t *(*svn_cache__partia
apr_pool_t *pool);
/**
+ * A function type for modifying an already deserialized in the @a *data
+ * buffer of length @a *data_len. Additional information of the modification
+ * to do will be provided in @a baton. The function may change the size of
+ * data buffer and may re-allocate it if necessary. In that case, the new
+ * values must be passed back in @a *data_len and @a *data, respectively.
+ * Allocations will be done from @a pool.
+ */
+typedef svn_error_t *(*svn_cache__partial_setter_func_t)(char **data,
+ apr_size_t *data_len,
+ void *baton,
+ apr_pool_t *pool);
+
+/**
* A function type for serializing an object @a in into bytes. The
* function should allocate the serialized value in @a pool, set
* @a *data to the serialized value, and set @a *data_len to its length.
@@ -395,6 +408,20 @@ svn_cache__get_partial(void **value,
apr_pool_t *scratch_pool);
/**
+ * Find the item identified by @a key in the @a cache. If it has been found,
+ * call @a func for it and @a baton to potentially modify the data. Changed
+ * data will be written back to the cache. If the item cannot be found,
+ * @a func does not get called. @a scratch_pool is used for temporary
+ * allocations.
+ */
+svn_error_t *
+svn_cache__set_partial(svn_cache__t *cache,
+ const void *key,
+ svn_cache__partial_setter_func_t func,
+ void *baton,
+ apr_pool_t *scratch_pool);
+
+/**
* Collect all available usage statistics on the cache instance @a cache
* and write the data into @a info. If @a reset has been set, access
* counters will be reset right after copying the statistics info.
Modified: subversion/trunk/subversion/libsvn_fs_fs/caching.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/caching.c?rev=1093983&r1=1093982&r2=1093983&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/caching.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/caching.c Sat Apr 16 13:06:07 2011
@@ -31,6 +31,7 @@
#include "svn_cmdline.h"
#include "svn_private_config.h"
+#include <valgrind/callgrind.h>
/* Return a memcache in *MEMCACHE_P for FS if it's configured to use
memcached, or NULL otherwise. Also, sets *FAIL_STOP to a boolean
@@ -157,6 +158,13 @@ svn_fs_fs__initialize_caches(svn_fs_t *f
svn_memcache_t *memcache;
svn_boolean_t no_handler;
+ static int runCount = 0;
+
+ if (++runCount == 2)
+ {
+ CALLGRIND_START_INSTRUMENTATION
+ }
+
SVN_ERR(read_config(&memcache, &no_handler, fs, pool));
/* Make the cache for revision roots. For the vast majority of
Modified: subversion/trunk/subversion/libsvn_subr/cache-inprocess.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-inprocess.c?rev=1093983&r1=1093982&r2=1093983&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cache-inprocess.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cache-inprocess.c Sat Apr 16 13:06:07 2011
@@ -481,6 +481,35 @@ inprocess_cache_get_partial(void **value
func(value_p, entry->value, entry->size, baton, pool));
}
+static svn_error_t *
+inprocess_cache_set_partial(void *cache_void,
+ const void *key,
+ svn_cache__partial_setter_func_t func,
+ void *baton,
+ apr_pool_t *pool)
+{
+ inprocess_cache_t *cache = cache_void;
+ struct cache_entry *entry;
+ svn_error_t *err = SVN_NO_ERROR;
+
+ SVN_ERR(lock_cache(cache));
+
+ entry = apr_hash_get(cache->hash, key, cache->klen);
+ if (! entry)
+ return unlock_cache(cache, err);
+
+ move_page_to_front(cache, entry->page);
+
+ cache->data_size -= entry->size;
+ err = func((char **)&entry->value,
+ &entry->size,
+ baton,
+ entry->page->page_pool);
+ cache->data_size += entry->size;
+
+ return unlock_cache(cache, err);
+}
+
static svn_boolean_t
inprocess_cache_is_cachable(void *cache_void, apr_size_t size)
{
@@ -525,6 +554,7 @@ static svn_cache__vtable_t inprocess_cac
inprocess_cache_iter,
inprocess_cache_is_cachable,
inprocess_cache_get_partial,
+ inprocess_cache_set_partial,
inprocess_cache_get_info
};
Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1093983&r1=1093982&r2=1093983&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Sat Apr 16 13:06:07 2011
@@ -1174,6 +1174,12 @@ membuffer_cache_get(svn_membuffer_t *cac
return deserializer(item, buffer, entry->size, pool);
}
+/* Look for the cache entry identified by KEY and KEY_LEN. FOUND indicates
+ * whether that entry exists. If not found, *ITEM will be NULL. Otherwise,
+ * the DESERIALIZER is called with that entry and the BATON provided
+ * and will extract the desired information. The result is set in *ITEM.
+ * Allocations will be done in POOL.
+ */
static svn_error_t *
membuffer_cache_get_partial(svn_membuffer_t *cache,
const void *key,
@@ -1239,6 +1245,105 @@ membuffer_cache_get_partial(svn_membuffe
return unlock_cache(cache, err);
}
+/* Look for the cache entry identified by KEY and KEY_LEN. If no entry
+ * has been found, the function returns without modifying the cache.
+ * Otherwise, FUNC is called with that entry and the BATON provided
+ * and may modify the cache entry. Allocations will be done in POOL.
+ */
+static svn_error_t *
+membuffer_cache_set_partial(svn_membuffer_t *cache,
+ const void *key,
+ apr_size_t key_len,
+ svn_cache__partial_setter_func_t func,
+ void *baton,
+ DEBUG_CACHE_MEMBUFFER_TAG_ARG
+ apr_pool_t *pool)
+{
+ apr_uint32_t group_index;
+ unsigned char to_find[KEY_SIZE];
+ entry_t *entry;
+ svn_error_t *err = SVN_NO_ERROR;
+
+ /* cache item lookup
+ */
+ group_index = get_group_index(&cache, key, key_len, to_find, pool);
+
+ SVN_ERR(lock_cache(cache));
+
+ entry = find_entry(cache, group_index, to_find, FALSE);
+ cache->total_reads++;
+
+ /* this function is a no-op if the item is not in cache
+ */
+ if (entry != NULL)
+ {
+ /* access the serialized cache item */
+ char *data = (char*)cache->data + entry->offset;
+ char *orig_data = data;
+ apr_size_t size = entry->size;
+
+ entry->hit_count++;
+ cache->hit_count++;
+ cache->total_writes++;
+
+#ifdef DEBUG_CACHE_MEMBUFFER
+
+ /* Check for overlapping entries.
+ */
+ SVN_ERR_ASSERT(entry->next == NO_INDEX ||
+ entry->offset + size
+ <= get_entry(cache, entry->next)->offset);
+
+ /* Compare original content, type and key (hashes)
+ */
+ SVN_ERR(store_content_part(tag, data, size, pool));
+ SVN_ERR(assert_equal_tags(&entry->tag, tag));
+
+#endif
+
+ /* modify it, preferrably in-situ.
+ */
+ err = func(&data, &size, baton, pool);
+
+ /* if modification caused a re-allocation, we need to remove the old
+ * entry and to copy the new data back into cache.
+ */
+ if (data != orig_data)
+ {
+ /* Remove the old entry and try to make space for the new one.
+ */
+ drop_entry(cache, entry);
+ if ( (cache->data_size / 4 > size)
+ && ensure_data_insertable(cache, size))
+ {
+ /* Write the new entry.
+ */
+ entry->size = size;
+ entry->offset = cache->current_data;
+ if (size)
+ memcpy(cache->data + entry->offset, data, size);
+
+ /* Link the entry properly.
+ */
+ insert_entry(cache, entry);
+
+#ifdef DEBUG_CACHE_MEMBUFFER
+
+ /* Remember original content, type and key (hashes)
+ */
+ SVN_ERR(store_content_part(tag, data, size, pool));
+ memcpy(&entry->tag, tag, sizeof(*tag));
+
+#endif
+ }
+ }
+ }
+
+ /* done here -> unlock the cache
+ */
+ return unlock_cache(cache, err);
+}
+
/* Implement the svn_cache__t interface on top of a shared membuffer cache.
*
* Because membuffer caches tend to be very large, there will be rather few
@@ -1482,6 +1587,40 @@ svn_membuffer_cache_get_partial(void **v
return SVN_NO_ERROR;
}
+static svn_error_t *
+svn_membuffer_cache_set_partial(void *cache_void,
+ const void *key,
+ svn_cache__partial_setter_func_t func,
+ void *baton,
+ apr_pool_t *pool)
+{
+ svn_membuffer_cache_t *cache = cache_void;
+
+ void *full_key;
+ apr_size_t full_key_len;
+
+ DEBUG_CACHE_MEMBUFFER_INIT_TAG
+
+ combine_key(cache->prefix,
+ sizeof(cache->prefix),
+ key,
+ cache->key_len,
+ &full_key,
+ &full_key_len,
+ pool);
+
+ SVN_ERR(membuffer_cache_set_partial(cache->membuffer,
+ full_key,
+ full_key_len,
+ func,
+ baton,
+ DEBUG_CACHE_MEMBUFFER_TAG
+ pool));
+// printf("set partial %s \n", key);
+
+ return SVN_NO_ERROR;
+}
+
static svn_boolean_t
svn_membuffer_cache_is_cachable(void *cache_void, apr_size_t size)
{
@@ -1545,6 +1684,7 @@ static svn_cache__vtable_t membuffer_cac
svn_membuffer_cache_iter,
svn_membuffer_cache_is_cachable,
svn_membuffer_cache_get_partial,
+ svn_membuffer_cache_set_partial,
svn_membuffer_cache_get_info
};
Modified: subversion/trunk/subversion/libsvn_subr/cache-memcache.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-memcache.c?rev=1093983&r1=1093982&r2=1093983&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cache-memcache.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cache-memcache.c Sat Apr 16 13:06:07 2011
@@ -288,6 +288,38 @@ memcache_get_partial(void **value_p,
static svn_error_t *
+memcache_set_partial(void *cache_void,
+ const void *key,
+ svn_cache__partial_setter_func_t func,
+ void *baton,
+ apr_pool_t *pool)
+{
+ svn_error_t *err = SVN_NO_ERROR;
+
+ char *data;
+ apr_size_t size;
+
+ apr_pool_t *subpool = svn_pool_create(poo);
+ SVN_ERR(memcache_internal_get(&data,
+ &size,
+ found,
+ cache_void,
+ key,
+ pool));
+
+ /* If we found it, modify it and write it back to cache */
+ if (*found)
+ {
+ SVN_ERR(func(&data, &size, baton, subpool));
+ err = memcache_internal_set(cache_void, key, data, data_len, subpool);
+ }
+
+ svn_pool_destroy(subpool);
+ return err;
+}
+
+
+static svn_error_t *
memcache_iter(svn_boolean_t *completed,
void *cache_void,
svn_iter_apr_hash_cb_t user_cb,
@@ -336,6 +368,7 @@ static svn_cache__vtable_t memcache_vtab
memcache_iter,
memcache_is_cachable,
memcache_get_partial,
+ memcache_set_partial,
memcache_get_info
};
Modified: subversion/trunk/subversion/libsvn_subr/cache.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache.c?rev=1093983&r1=1093982&r2=1093983&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cache.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cache.c Sat Apr 16 13:06:07 2011
@@ -154,6 +154,23 @@ svn_cache__get_partial(void **value,
}
svn_error_t *
+svn_cache__set_partial(svn_cache__t *cache,
+ const void *key,
+ svn_cache__partial_setter_func_t func,
+ void *baton,
+ apr_pool_t *scratch_pool)
+{
+ cache->writes++;
+ return handle_error(cache,
+ (cache->vtable->set_partial)(cache->cache_internal,
+ key,
+ func,
+ baton,
+ scratch_pool),
+ scratch_pool);
+}
+
+svn_error_t *
svn_cache__get_info(svn_cache__t *cache,
svn_cache__info_t *info,
svn_boolean_t reset,
Modified: subversion/trunk/subversion/libsvn_subr/cache.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache.h?rev=1093983&r1=1093982&r2=1093983&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cache.h (original)
+++ subversion/trunk/subversion/libsvn_subr/cache.h Sat Apr 16 13:06:07 2011
@@ -58,6 +58,11 @@ typedef struct svn_cache__vtable_t {
svn_cache__partial_getter_func_t func,
void *baton,
apr_pool_t *pool);
+ svn_error_t *(*set_partial)(void *cache_implementation,
+ const void *key,
+ svn_cache__partial_setter_func_t func,
+ void *baton,
+ apr_pool_t *scratch_pool);
svn_error_t *(*get_info)(void *cache_implementation,
svn_cache__info_t *info,
Re: svn commit: r1093983 - in /subversion/trunk/subversion: include/private/svn_cache.h
libsvn_fs_fs/caching.c libsvn_subr/cache-inprocess.c libsvn_subr/cache-membuffer.c
libsvn_subr/cache-memcache.c libsvn_subr/cache.c libsvn_subr/cache.h
Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
On 14.05.2011 03:35, Daniel Shahaf wrote:
> stefan2@apache.org wrote on Sat, Apr 16, 2011 at 13:06:07 -0000:
>> Author: stefan2
>> Date: Sat Apr 16 13:06:07 2011
>> New Revision: 1093983
>>
>> URL: http://svn.apache.org/viewvc?rev=1093983&view=rev
>> Log:
>> In symmetry to svn_cache__get_partial, introduce a partial
>> setter / modifier API function that enabled the caller to
>> modify changed items without the full deserialization -
>> modification - serialization chain. Therefore, adding entries
>> to e.g. directories becomes O(1) instead of O(n).
>>
> ...
>> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Sat Apr 16 13:06:07 2011
>> @@ -1174,6 +1174,12 @@ membuffer_cache_get(svn_membuffer_t *cac
>> return deserializer(item, buffer, entry->size, pool);
>> }
>>
>> +/* Look for the cache entry identified by KEY and KEY_LEN. FOUND indicates
>> + * whether that entry exists. If not found, *ITEM will be NULL. Otherwise,
>> + * the DESERIALIZER is called with that entry and the BATON provided
>> + * and will extract the desired information. The result is set in *ITEM.
>> + * Allocations will be done in POOL.
>> + */
>> static svn_error_t *
>> membuffer_cache_get_partial(svn_membuffer_t *cache,
>> const void *key,
>> @@ -1239,6 +1245,105 @@ membuffer_cache_get_partial(svn_membuffe
>> return unlock_cache(cache, err);
>> }
>>
>> +/* Look for the cache entry identified by KEY and KEY_LEN. If no entry
>> + * has been found, the function returns without modifying the cache.
>> + * Otherwise, FUNC is called with that entry and the BATON provided
>> + * and may modify the cache entry. Allocations will be done in POOL.
>> + */
>> +static svn_error_t *
>> +membuffer_cache_set_partial(svn_membuffer_t *cache,
>> + const void *key,
>> + apr_size_t key_len,
>> + svn_cache__partial_setter_func_t func,
>> + void *baton,
>> + DEBUG_CACHE_MEMBUFFER_TAG_ARG
>> + apr_pool_t *pool)
>> +{
>> + apr_uint32_t group_index;
>> + unsigned char to_find[KEY_SIZE];
>> + entry_t *entry;
>> + svn_error_t *err = SVN_NO_ERROR;
>> +
>> + /* cache item lookup
>> + */
>> + group_index = get_group_index(&cache, key, key_len, to_find, pool);
>> +
>> + SVN_ERR(lock_cache(cache));
>> +
>> + entry = find_entry(cache, group_index, to_find, FALSE);
>> + cache->total_reads++;
>> +
>> + /* this function is a no-op if the item is not in cache
>> + */
>> + if (entry != NULL)
>> + {
>> + /* access the serialized cache item */
>> + char *data = (char*)cache->data + entry->offset;
>> + char *orig_data = data;
>> + apr_size_t size = entry->size;
>> +
>> + entry->hit_count++;
>> + cache->hit_count++;
>> + cache->total_writes++;
>> +
>> +#ifdef DEBUG_CACHE_MEMBUFFER
>> +
>> + /* Check for overlapping entries.
>> + */
>> + SVN_ERR_ASSERT(entry->next == NO_INDEX ||
>> + entry->offset + size
>> +<= get_entry(cache, entry->next)->offset);
>> +
>> + /* Compare original content, type and key (hashes)
>> + */
>> + SVN_ERR(store_content_part(tag, data, size, pool));
>> + SVN_ERR(assert_equal_tags(&entry->tag, tag));
>> +
>> +#endif
>> +
>> + /* modify it, preferrably in-situ.
>> + */
>> + err = func(&data,&size, baton, pool);
>> +
>> + /* if modification caused a re-allocation, we need to remove the old
>> + * entry and to copy the new data back into cache.
>> + */
>> + if (data != orig_data)
> Shouldn't you be checking ERR before storing the change made by FUNC?
>
You are right! Fixed in r1103391.
Thanks for the review!
-- Stefan^2.
Re: svn commit: r1093983 - in /subversion/trunk/subversion:
include/private/svn_cache.h libsvn_fs_fs/caching.c
libsvn_subr/cache-inprocess.c libsvn_subr/cache-membuffer.c
libsvn_subr/cache-memcache.c libsvn_subr/cache.c libsvn_subr/cache.h
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stefan2@apache.org wrote on Sat, Apr 16, 2011 at 13:06:07 -0000:
> Author: stefan2
> Date: Sat Apr 16 13:06:07 2011
> New Revision: 1093983
>
> URL: http://svn.apache.org/viewvc?rev=1093983&view=rev
> Log:
> In symmetry to svn_cache__get_partial, introduce a partial
> setter / modifier API function that enabled the caller to
> modify changed items without the full deserialization -
> modification - serialization chain. Therefore, adding entries
> to e.g. directories becomes O(1) instead of O(n).
>
...
> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Sat Apr 16 13:06:07 2011
> @@ -1174,6 +1174,12 @@ membuffer_cache_get(svn_membuffer_t *cac
> return deserializer(item, buffer, entry->size, pool);
> }
>
> +/* Look for the cache entry identified by KEY and KEY_LEN. FOUND indicates
> + * whether that entry exists. If not found, *ITEM will be NULL. Otherwise,
> + * the DESERIALIZER is called with that entry and the BATON provided
> + * and will extract the desired information. The result is set in *ITEM.
> + * Allocations will be done in POOL.
> + */
> static svn_error_t *
> membuffer_cache_get_partial(svn_membuffer_t *cache,
> const void *key,
> @@ -1239,6 +1245,105 @@ membuffer_cache_get_partial(svn_membuffe
> return unlock_cache(cache, err);
> }
>
> +/* Look for the cache entry identified by KEY and KEY_LEN. If no entry
> + * has been found, the function returns without modifying the cache.
> + * Otherwise, FUNC is called with that entry and the BATON provided
> + * and may modify the cache entry. Allocations will be done in POOL.
> + */
> +static svn_error_t *
> +membuffer_cache_set_partial(svn_membuffer_t *cache,
> + const void *key,
> + apr_size_t key_len,
> + svn_cache__partial_setter_func_t func,
> + void *baton,
> + DEBUG_CACHE_MEMBUFFER_TAG_ARG
> + apr_pool_t *pool)
> +{
> + apr_uint32_t group_index;
> + unsigned char to_find[KEY_SIZE];
> + entry_t *entry;
> + svn_error_t *err = SVN_NO_ERROR;
> +
> + /* cache item lookup
> + */
> + group_index = get_group_index(&cache, key, key_len, to_find, pool);
> +
> + SVN_ERR(lock_cache(cache));
> +
> + entry = find_entry(cache, group_index, to_find, FALSE);
> + cache->total_reads++;
> +
> + /* this function is a no-op if the item is not in cache
> + */
> + if (entry != NULL)
> + {
> + /* access the serialized cache item */
> + char *data = (char*)cache->data + entry->offset;
> + char *orig_data = data;
> + apr_size_t size = entry->size;
> +
> + entry->hit_count++;
> + cache->hit_count++;
> + cache->total_writes++;
> +
> +#ifdef DEBUG_CACHE_MEMBUFFER
> +
> + /* Check for overlapping entries.
> + */
> + SVN_ERR_ASSERT(entry->next == NO_INDEX ||
> + entry->offset + size
> + <= get_entry(cache, entry->next)->offset);
> +
> + /* Compare original content, type and key (hashes)
> + */
> + SVN_ERR(store_content_part(tag, data, size, pool));
> + SVN_ERR(assert_equal_tags(&entry->tag, tag));
> +
> +#endif
> +
> + /* modify it, preferrably in-situ.
> + */
> + err = func(&data, &size, baton, pool);
> +
> + /* if modification caused a re-allocation, we need to remove the old
> + * entry and to copy the new data back into cache.
> + */
> + if (data != orig_data)
Shouldn't you be checking ERR before storing the change made by FUNC?
> + {
> + /* Remove the old entry and try to make space for the new one.
> + */
> + drop_entry(cache, entry);
> + if ( (cache->data_size / 4 > size)
> + && ensure_data_insertable(cache, size))
> + {
> + /* Write the new entry.
> + */
> + entry->size = size;
> + entry->offset = cache->current_data;
> + if (size)
> + memcpy(cache->data + entry->offset, data, size);
> +
> + /* Link the entry properly.
> + */
> + insert_entry(cache, entry);
> +
> +#ifdef DEBUG_CACHE_MEMBUFFER
> +
> + /* Remember original content, type and key (hashes)
> + */
> + SVN_ERR(store_content_part(tag, data, size, pool));
> + memcpy(&entry->tag, tag, sizeof(*tag));
> +
> +#endif
> + }
> + }
> + }
> +
> + /* done here -> unlock the cache
> + */
> + return unlock_cache(cache, err);
> +}
> +
Re: svn commit: r1093983 - in /subversion/trunk/subversion:
include/private/svn_cache.h libsvn_fs_fs/caching.c
libsvn_subr/cache-inprocess.c libsvn_subr/cache-membuffer.c
libsvn_subr/cache-memcache.c libsvn_subr/cache.c libsvn_subr/cache.h
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stefan2@apache.org wrote on Sat, Apr 16, 2011 at 13:06:07 -0000:
> Author: stefan2
> Date: Sat Apr 16 13:06:07 2011
> New Revision: 1093983
>
> URL: http://svn.apache.org/viewvc?rev=1093983&view=rev
> Log:
> In symmetry to svn_cache__get_partial, introduce a partial
> setter / modifier API function that enabled the caller to
> modify changed items without the full deserialization -
> modification - serialization chain. Therefore, adding entries
> to e.g. directories becomes O(1) instead of O(n).
>
...
> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Sat Apr 16 13:06:07 2011
> @@ -1174,6 +1174,12 @@ membuffer_cache_get(svn_membuffer_t *cac
> return deserializer(item, buffer, entry->size, pool);
> }
>
> +/* Look for the cache entry identified by KEY and KEY_LEN. FOUND indicates
> + * whether that entry exists. If not found, *ITEM will be NULL. Otherwise,
> + * the DESERIALIZER is called with that entry and the BATON provided
> + * and will extract the desired information. The result is set in *ITEM.
> + * Allocations will be done in POOL.
> + */
> static svn_error_t *
> membuffer_cache_get_partial(svn_membuffer_t *cache,
> const void *key,
> @@ -1239,6 +1245,105 @@ membuffer_cache_get_partial(svn_membuffe
> return unlock_cache(cache, err);
> }
>
> +/* Look for the cache entry identified by KEY and KEY_LEN. If no entry
> + * has been found, the function returns without modifying the cache.
> + * Otherwise, FUNC is called with that entry and the BATON provided
> + * and may modify the cache entry. Allocations will be done in POOL.
> + */
> +static svn_error_t *
> +membuffer_cache_set_partial(svn_membuffer_t *cache,
> + const void *key,
> + apr_size_t key_len,
> + svn_cache__partial_setter_func_t func,
> + void *baton,
> + DEBUG_CACHE_MEMBUFFER_TAG_ARG
> + apr_pool_t *pool)
> +{
> + apr_uint32_t group_index;
> + unsigned char to_find[KEY_SIZE];
> + entry_t *entry;
> + svn_error_t *err = SVN_NO_ERROR;
> +
> + /* cache item lookup
> + */
> + group_index = get_group_index(&cache, key, key_len, to_find, pool);
> +
> + SVN_ERR(lock_cache(cache));
> +
> + entry = find_entry(cache, group_index, to_find, FALSE);
> + cache->total_reads++;
> +
> + /* this function is a no-op if the item is not in cache
> + */
> + if (entry != NULL)
> + {
> + /* access the serialized cache item */
> + char *data = (char*)cache->data + entry->offset;
> + char *orig_data = data;
> + apr_size_t size = entry->size;
> +
> + entry->hit_count++;
> + cache->hit_count++;
> + cache->total_writes++;
> +
> +#ifdef DEBUG_CACHE_MEMBUFFER
> +
> + /* Check for overlapping entries.
> + */
> + SVN_ERR_ASSERT(entry->next == NO_INDEX ||
> + entry->offset + size
> + <= get_entry(cache, entry->next)->offset);
> +
> + /* Compare original content, type and key (hashes)
> + */
> + SVN_ERR(store_content_part(tag, data, size, pool));
> + SVN_ERR(assert_equal_tags(&entry->tag, tag));
> +
> +#endif
> +
> + /* modify it, preferrably in-situ.
> + */
> + err = func(&data, &size, baton, pool);
> +
> + /* if modification caused a re-allocation, we need to remove the old
> + * entry and to copy the new data back into cache.
> + */
> + if (data != orig_data)
Shouldn't you be checking ERR before storing the change made by FUNC?
> + {
> + /* Remove the old entry and try to make space for the new one.
> + */
> + drop_entry(cache, entry);
> + if ( (cache->data_size / 4 > size)
> + && ensure_data_insertable(cache, size))
> + {
> + /* Write the new entry.
> + */
> + entry->size = size;
> + entry->offset = cache->current_data;
> + if (size)
> + memcpy(cache->data + entry->offset, data, size);
> +
> + /* Link the entry properly.
> + */
> + insert_entry(cache, entry);
> +
> +#ifdef DEBUG_CACHE_MEMBUFFER
> +
> + /* Remember original content, type and key (hashes)
> + */
> + SVN_ERR(store_content_part(tag, data, size, pool));
> + memcpy(&entry->tag, tag, sizeof(*tag));
> +
> +#endif
> + }
> + }
> + }
> +
> + /* done here -> unlock the cache
> + */
> + return unlock_cache(cache, err);
> +}
> +