On 26.03.2011 06:31, Daniel Shahaf wrote:
> stefan2_at_apache.org wrote on Sun, Mar 13, 2011 at 12:40:50 -0000:
>> Author: stefan2
>> Date: Sun Mar 13 12:40:49 2011
>> New Revision: 1081097
>>
>> URL: http://svn.apache.org/viewvc?rev=1081097&view=rev
>> Log:
>> Prepare for the introduction of a generic cache statistics API:
>> make cache-inprocess caches identifiable.
>>
>> * subversion/include/private/svn_cache.h
>> (svn_cache__create_inprocess): add id parameter
>> * subversion/libsvn_subr/cache-inprocess.c
>> (inprocess_cache_t): add id member
>> (svn_cache__create_inprocess): store id parameter
>> * subversion/libsvn_fs_fs/caching.c
>> (init_callbacks): new function; factored out from the init function
>> (svn_fs_fs__initialize_caches): provide IDs / prefixes to all cache types,
>> call new init sub-function
>> * subversion/libsvn_fs_fs/tree.c
>> (make_txn_root): adapt to internal API change
>> * subversion/tests/libsvn_subr/cache-test.c
>> (test_inprocess_cache_basic): dito
>>
>> Modified:
>> subversion/trunk/subversion/include/private/svn_cache.h
>> subversion/trunk/subversion/libsvn_fs_fs/caching.c
>> subversion/trunk/subversion/libsvn_fs_fs/tree.c
>> subversion/trunk/subversion/libsvn_subr/cache-inprocess.c
>> subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
>>
>> Modified: subversion/trunk/subversion/include/private/svn_cache.h
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_cache.h?rev=1081097&r1=1081096&r2=1081097&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/include/private/svn_cache.h (original)
>> +++ subversion/trunk/subversion/include/private/svn_cache.h Sun Mar 13 12:40:49 2011
>> @@ -136,6 +136,7 @@ svn_cache__create_inprocess(svn_cache__t
>> apr_int64_t pages,
>> apr_int64_t items_per_page,
>> svn_boolean_t thread_safe,
>> + const char *id,
> Need to document the ID parameter please...
Thanks for finding that!
Fixed in r1088351.
>
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1081097&r1=1081096&r2=1081097&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
>> +++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Sun Mar 13 12:40:49 2011
>> @@ -3916,7 +3916,9 @@ make_txn_root(svn_fs_root_t **root_p,
>> svn_fs_fs__dag_serialize,
>> svn_fs_fs__dag_deserialize,
>> APR_HASH_KEY_STRING,
>> - 32, 20, FALSE, root->pool));
>> + 32, 20, FALSE,
>> + apr_pstrcat(pool, txn, ":TXN", (char *)NULL),
> This doesn't use namespacing...
>
Name spaces are only required for membuffer caches.
This is a good ol' inprocess cache that will not share
cached information with other cache instances.
> So, beyond the theoretical problem, doesn't it mean that a single
> application that creates two repositories and begins a txn against each
> repository will mix the two txn's caches? (since txn names are
> predictable)
>
> IOW: shouldn't this use the PREFIX argument that caching.c uses?
Unless somebody reads the statistics info for this
cache instance, the id won't be used at all. If someone
should call svn_cache__get_info on that info for
profiling etc., the transaction name plus the ":TXN"
marker should be sufficient to identify the data
in a log, for instance.
>> + root->pool));
>>
>> root->fsap_data = frd;
>>
>>
>> Modified: subversion/trunk/subversion/tests/libsvn_subr/cache-test.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/cache-test.c?rev=1081097&r1=1081096&r2=1081097&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/libsvn_subr/cache-test.c (original)
>> +++ subversion/trunk/subversion/tests/libsvn_subr/cache-test.c Sun Mar 13 12:40:49 2011
>> @@ -136,6 +136,7 @@ test_inprocess_cache_basic(apr_pool_t *p
>> 1,
>> 1,
>> TRUE,
>> + "",
>> pool));
>>
> Same problem... (I imagine we might want to use "" in the cache
> infrastructure internally some day, for example)
This is also a cache_inprocess instance. So, we don't need
a proper ID / prefix here for proper function.
-- Stefan^2.
Received on 2011-04-03 20:00:59 CEST