Malcolm Rowe wrote:
> On Wed, Oct 11, 2006 at 02:51:28PM +0530, Kamesh Jayachandran wrote:
>
>> I think you have misunderstood it.
>> I have some merges on '/a/b/c' which records the actual mergeinfo which
>> is applicable to its child '/a/b/c/d' and grand child '/a/b/c/d/e'...etc.
>> When Someone ask for 'mergeinfo' on '/a/b/c/d/e' current
>> 'get_merge_info_for_path' gives nothing which my patch fixes it.
>>
>>
>
> Is that because we already have a negative cache result for a query
> directly on /a/b/c/d? If so, it sounds like we shouldn't be assuming
> that a negative cache on /a/b/c/d implies that there's no result on the
> parent, or alternatively that we should be caching the parent's value
> against /a/b/c/d (though that seems like it would bloat the cache).
>
> I presume that someone has measured that the additional space taken up
> by the negative cache entries (and consequently slower lookups) is a net
> win over just re-performing the query directly?
>
> Regards,
> Malcolm
>
Bugs,
1)'negative cache result' in cache is set at the wrong place
2)if has_no_mergeinfo == TRUE we should not elide further
3)Irrespective of 'result' we should translate the 'cache' entries so
that we could consume it meaningfully at the top of the recursion unwinding.
4)We do apr_hash_set(result, path, APR_HASH_KEY_STRING, NULL); which
should have been apr_hash_set(*cache*, path, APR_HASH_KEY_STRING, NULL);
I have a working patch taking care of all of the above.
<snip>
SVN_ERR(get_merge_info_for_path(db, parentpath->data, rev,
NULL, cache, include_parents,
pool));
//during eliding recursive calls result is NULL so hash won't be properly translated
//consider the case if user calls with path /a/b/c/d/e and we don't have direct mergeinfo for /a/b/c/d/e we
//make recursive call with path /a/b/c/d and result NULL and if /a/b/c/d also don't have direct info we
//make one more recursive call with path /a/b/c and result NULL, if we get the mergeinfo now
//during first unwinding(path=/a/b/c/d) we simply return as result == NULL.
//during next unwinding(path=/a/b/c/d/e) we simply return as cacheresult == -1 as we have cache('/a/b/c/d' => '-1' in the earlier call).
//so we have broken eliding.
if (result)
{
/* Now translate the result for our parent to our path */
cacheresult = apr_hash_get(cache, parentpath->data,
APR_HASH_KEY_STRING);
if (cacheresult == NEGATIVE_CACHE_RESULT)
apr_hash_set(result, path, APR_HASH_KEY_STRING, NULL);
else if (cacheresult)
{
apr_hash_t *translatedhash;
const char *toappend = &path[parentpath->len + 1];
append_component_to_paths(&translatedhash, cacheresult,
toappend, pool);
apr_hash_set(result, path, APR_HASH_KEY_STRING,
translatedhash);
}
}
</snip>
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Oct 11 13:32:14 2006