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

[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.