On Tue, Mar 8, 2011 at 17:40, <hwright_at_apache.org> wrote:
> Author: hwright
> Date: Tue Mar 8 22:40:09 2011
> New Revision: 1079588
>
> URL: http://svn.apache.org/viewvc?rev=1079588&view=rev
> Log:
> Update the wc-ng stat caching code to use the well-established stringbuf,
> rather than reinventing said wheel.
Cool.
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc_db_pdh.c Tue Mar 8 22:40:09 2011
> @@ -97,28 +97,34 @@ get_path_kind(svn_wc__db_t *db,
> apr_pool_t *scratch_pool)
> {
> svn_boolean_t special;
> - size_t abspath_size;
>
> /* This implements a *really* simple LRU cache, where "simple" is defined
> as "only one element". In other words, we remember the most recently
> queried path, and nothing else. This gives >80% cache hits. */
>
> if (db->parse_cache.abspath
> - && strcmp(db->parse_cache.abspath, local_abspath) == 0)
> + && strcmp(db->parse_cache.abspath->data, local_abspath) == 0)
> {
> /* Cache hit! */
> *kind = db->parse_cache.kind;
> return SVN_NO_ERROR;
> }
>
> - abspath_size = strlen(local_abspath);
> - if (abspath_size >= db->parse_cache.size)
> + if (!db->parse_cache.abspath)
> {
> - db->parse_cache.size = abspath_size * 2 + 1;
> - db->parse_cache.abspath = apr_palloc(db->state_pool,
> - db->parse_cache.size);
> + db->parse_cache.abspath = svn_stringbuf_create(local_abspath,
> + db->state_pool);
> + }
> + else
> + {
> + size_t abspath_size = strlen(local_abspath);
> +
> + if (abspath_size < db->parse_cache.abspath->blocksize)
> + svn_stringbuf_set(db->parse_cache.abspath, local_abspath);
> + else
> + db->parse_cache.abspath = svn_stringbuf_ncreate(local_abspath,
> + abspath_size * 2 + 1, db->state_pool);
Hunh? Why are you creating a NEW stringbuf? The whole point of
stringbuf is that it reallocs in place. It manages the size and
everything.
I don't have the testing setup right now, or I'd just go make the
changes, but that's never as good as shared knowledge.
So. First thing: just embed the svn_stringbuf_t in the db structure.
No need for a pointer-to-stringbuf. Just make sure it is zeroed out
during initialization (ie. apr_pcalloc()).
Then, all you need to do is one line:
svn_stringbuf_set(&db->parse_cache.abspath, local_abspath);
That will replace your entire if/else block above. No strlen. No
recreation of a stringbuf. No separate set(). No initial creation...
One line.
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc_db_private.h Tue Mar 8 22:40:09 2011
> @@ -56,8 +56,7 @@ struct svn_wc__db_t {
> get_path_kind() for use. */
> struct
> {
> - char *abspath;
> - size_t size;
> + svn_stringbuf_t *abspath;
Per above, remove the ptr here, and ensure that init will set the
stringbuf to all zeroes.
Cheers,
-g
Received on 2011-03-09 02:46:20 CET