You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Dongsheng Song <do...@apache.org> on 2011/02/23 06:47:50 UTC

[PATCH] Remove dead code - was - Re: svn commit: r1067669

On Sun, Feb 6, 2011 at 22:13, <st...@apache.org> wrote:

> Author: stefan2
> Date: Sun Feb  6 14:13:57 2011
> New Revision: 1067669
>
> URL: http://svn.apache.org/viewvc?rev=1067669&view=rev
> Log:
> Merge all changes (r998649, r998843, r998852) from the
> integrate-cache-membuffer branch.
>
> These patches introduce in-process full-text caching.
>
> Added:
>    subversion/trunk/subversion/libsvn_fs_util/caching.c
>      - copied unchanged from r998852,
> subversion/branches/integrate-cache-membuffer/subversion/libsvn_fs_util/caching.c
>    subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>      - copied unchanged from r998852,
> subversion/branches/integrate-cache-membuffer/subversion/libsvn_subr/cache-membuffer.c
> Modified:
>    subversion/trunk/   (props changed)
>    subversion/trunk/subversion/include/private/svn_cache.h
>    subversion/trunk/subversion/include/private/svn_fs_private.h
>    subversion/trunk/subversion/include/svn_fs.h
>    subversion/trunk/subversion/libsvn_fs_fs/caching.c
>    subversion/trunk/subversion/svnserve/main.c
>    subversion/trunk/subversion/svnserve/server.h
>
>
Hi Stefan,

Your commit have 2 static functions which defined but not used:

subversion/libsvn_subr/cache-membuffer.c:1221: warning:
‘svn_membuffer_cache_get_partial’ defined but not used
subversion/libsvn_subr/cache-membuffer.c:1254: warning:
‘svn_membuffer_cache_is_cachable’ defined but not used

[
Remove dead code.

* subversion/libsvn_subr/cache-membuffer.c
  (svn_membuffer_cache_get_partial, svn_membuffer_cache_is_cachable):
Remove, not longer called.
]


--- a/subversion/libsvn_subr/cache-membuffer.c
+++ b/subversion/libsvn_subr/cache-membuffer.c
@@ -1217,51 +1217,6 @@ svn_membuffer_cache_iter(svn_boolean_t *completed,
                           _("Can't iterate a membuffer-based cache"));
 }

-static svn_error_t *
-svn_membuffer_cache_get_partial(void **value_p,
-                                svn_boolean_t *found,
-                                void *cache_void,
-                                const void *key,
-                                svn_cache__partial_getter_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;
-
-  combine_key(cache->prefix,
-              sizeof(cache->prefix),
-              key,
-              cache->key_len,
-              &full_key,
-              &full_key_len,
-              pool);
-
-  SVN_ERR(membuffer_cache_get_partial(cache->membuffer,
-                                      full_key,
-                                      full_key_len,
-                                      value_p,
-                                      func,
-                                      baton,
-                                      pool));
-  *found = *value_p != NULL;
-  return SVN_NO_ERROR;
-}
-
-static svn_boolean_t
-svn_membuffer_cache_is_cachable(void *cache_void, apr_size_t size)
-{
-  /* Don't allow extremely large element sizes. Otherwise, the cache
-   * might by thrashed by a few extremely large entries. And the size
-   * must be small enough to be stored in a 32 bit value.
-   */
-  svn_membuffer_cache_t *cache = cache_void;
-  return (size < cache->membuffer->data_size / 4)
-      && (size < APR_UINT32_MAX - ITEM_ALIGNMENT);
-}
-
 /* the v-table for membuffer-based caches
  */
 static svn_cache__vtable_t membuffer_cache_vtable = {

--
Dongsheng

Re: [PATCH] Remove dead code - was - Re: svn commit: r1067669

Posted by Stefan Fuhrmann <eq...@web.de>.
On 23.02.2011 12:03, Stefan Sperling wrote:
> On Wed, Feb 23, 2011 at 09:05:16AM +0100, Stefan Fuhrmann wrote:
>> On 23.02.2011 06:47, Dongsheng Song wrote:
>>
>> Hi Donsheng,
>>
>> Thanks for your review. You are right that these two
>> functions are not being used right now, but the
>> integration-partial-getter and integration-is-cachable
>> branches will add them to the svn_cache_t interface.
>>
>> I didn't temporarily remove the functions from the
>> membuffer cache code to reduce the merge hazards
>> that I already get for merging changes from the
>> performance branch in a completely different order
>> and granularity then that in which they got developed.
> You could #if 0 them to get rid of the warnings,
> and explain the situation in a comment.
Done so in r1074072.

-- Stefan^2.

Re: [PATCH] Remove dead code - was - Re: svn commit: r1067669

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Feb 23, 2011 at 09:05:16AM +0100, Stefan Fuhrmann wrote:
> On 23.02.2011 06:47, Dongsheng Song wrote:
> 
> Hi Donsheng,
> 
> Thanks for your review. You are right that these two
> functions are not being used right now, but the
> integration-partial-getter and integration-is-cachable
> branches will add them to the svn_cache_t interface.
> 
> I didn't temporarily remove the functions from the
> membuffer cache code to reduce the merge hazards
> that I already get for merging changes from the
> performance branch in a completely different order
> and granularity then that in which they got developed.

You could #if 0 them to get rid of the warnings,
and explain the situation in a comment.

Re: [PATCH] Remove dead code - was - Re: svn commit: r1067669

Posted by Stefan Fuhrmann <eq...@web.de>.
On 23.02.2011 06:47, Dongsheng Song wrote:

Hi Donsheng,

Thanks for your review. You are right that these two
functions are not being used right now, but the
integration-partial-getter and integration-is-cachable
branches will add them to the svn_cache_t interface.

I didn't temporarily remove the functions from the
membuffer cache code to reduce the merge hazards
that I already get for merging changes from the
performance branch in a completely different order
and granularity then that in which they got developed.

Sorry to have caused you extra work!

-- Stefan^2.

> On Sun, Feb 6, 2011 at 22:13,<st...@apache.org>  wrote:
>
>> Author: stefan2
>> Date: Sun Feb  6 14:13:57 2011
>> New Revision: 1067669
>>
>> URL: http://svn.apache.org/viewvc?rev=1067669&view=rev
>> Log:
>> Merge all changes (r998649, r998843, r998852) from the
>> integrate-cache-membuffer branch.
>>
>> These patches introduce in-process full-text caching.
>>
>> Added:
>>     subversion/trunk/subversion/libsvn_fs_util/caching.c
>>       - copied unchanged from r998852,
>> subversion/branches/integrate-cache-membuffer/subversion/libsvn_fs_util/caching.c
>>     subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>>       - copied unchanged from r998852,
>> subversion/branches/integrate-cache-membuffer/subversion/libsvn_subr/cache-membuffer.c
>> Modified:
>>     subversion/trunk/   (props changed)
>>     subversion/trunk/subversion/include/private/svn_cache.h
>>     subversion/trunk/subversion/include/private/svn_fs_private.h
>>     subversion/trunk/subversion/include/svn_fs.h
>>     subversion/trunk/subversion/libsvn_fs_fs/caching.c
>>     subversion/trunk/subversion/svnserve/main.c
>>     subversion/trunk/subversion/svnserve/server.h
>>
>>
> Hi Stefan,
>
> Your commit have 2 static functions which defined but not used:
>
> subversion/libsvn_subr/cache-membuffer.c:1221: warning:
> ‘svn_membuffer_cache_get_partial’ defined but not used
> subversion/libsvn_subr/cache-membuffer.c:1254: warning:
> ‘svn_membuffer_cache_is_cachable’ defined but not used
>
> [
> Remove dead code.
>
> * subversion/libsvn_subr/cache-membuffer.c
>    (svn_membuffer_cache_get_partial, svn_membuffer_cache_is_cachable):
> Remove, not longer called.
> ]
>
>
> --- a/subversion/libsvn_subr/cache-membuffer.c
> +++ b/subversion/libsvn_subr/cache-membuffer.c
> @@ -1217,51 +1217,6 @@ svn_membuffer_cache_iter(svn_boolean_t *completed,
>                             _("Can't iterate a membuffer-based cache"));
>   }
>
> -static svn_error_t *
> -svn_membuffer_cache_get_partial(void **value_p,
> -                                svn_boolean_t *found,
> -                                void *cache_void,
> -                                const void *key,
> -                                svn_cache__partial_getter_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;
> -
> -  combine_key(cache->prefix,
> -              sizeof(cache->prefix),
> -              key,
> -              cache->key_len,
> -&full_key,
> -&full_key_len,
> -              pool);
> -
> -  SVN_ERR(membuffer_cache_get_partial(cache->membuffer,
> -                                      full_key,
> -                                      full_key_len,
> -                                      value_p,
> -                                      func,
> -                                      baton,
> -                                      pool));
> -  *found = *value_p != NULL;
> -  return SVN_NO_ERROR;
> -}
> -
> -static svn_boolean_t
> -svn_membuffer_cache_is_cachable(void *cache_void, apr_size_t size)
> -{
> -  /* Don't allow extremely large element sizes. Otherwise, the cache
> -   * might by thrashed by a few extremely large entries. And the size
> -   * must be small enough to be stored in a 32 bit value.
> -   */
> -  svn_membuffer_cache_t *cache = cache_void;
> -  return (size<  cache->membuffer->data_size / 4)
> -&&  (size<  APR_UINT32_MAX - ITEM_ALIGNMENT);
> -}
> -
>   /* the v-table for membuffer-based caches
>    */
>   static svn_cache__vtable_t membuffer_cache_vtable = {
>
> --
> Dongsheng
>