[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Keywords as hash: API design: null or empty hash

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2005-09-22 14:06:36 CEST

John Peacock wrote:
> Julian Foad wrote:
>
>> Check whether the implementations of the functions that accept a
>> keywords hash are reasonably efficient both when given NULL and when
>> given an empty hash.
>>
>> svn_subst_keywords_differ2(a,b) - yes
>> svn_subst_translate_stream3(keywords) - no
>> svn_subst_copy_and_translate3(keywords) - no
>> svn_subst_translate_cstring2(keywords) - no
>
> Is this something we need to get into 1.3?

No, it's not. Sorry, I was completely misleading there, saying its something
we "ought to do now". You're spot on correct that it's just a matter of
efficiency, and since I've no reason to believe that the currrent inefficiency
is noticeable, it can be done whenever.

That said, I tentatively suggest the attached patch. What do you think of it?

>> Clean up svn_subst_translate_stream3()'s doc string: it refers to
>> keywords in the hash having null values, and to the syntax
>> "keywords->revision".
>
> That's easier to get done; if someone else doesn't beat me to it, I'll
> submit a patch tomorrow.

Thanks. Again, it's not critical because the real meaning can be inferred -
the text is not significantly misleading. If you'd rather leave it to me,
that's fine, I'm happy to do it.

- Julian

[[[
Make the hash-based keywords-processing functions handle an empty hash
efficiently.

* subversion/libsvn_subr/subst.c
  (svn_subst_translate_stream3,
   svn_subst_translate_cstring2,
   svn_subst_copy_and_translate3):
    Treat an empty hash in the same way as a null pointer, thus
    avoiding the overhead of doing a no-op translation.
]]]

Index: subversion/libsvn_subr/subst.c
===================================================================
--- subversion/libsvn_subr/subst.c (revision 16194)
+++ subversion/libsvn_subr/subst.c (working copy)
@@ -797,6 +797,10 @@ svn_subst_translate_stream3 (svn_stream_
 
   buf = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE + 1);
 
+ /* For efficiency, convert an empty set of keywords to NULL. */
+ if (keywords && (apr_hash_count (keywords) == 0))
+ keywords = NULL;
+
   /* The docstring requires that *some* translation be requested. */
   assert (eol_str || keywords);
   interesting = (eol_str && keywords) ? "$\r\n" : eol_str ? "\r\n" : "$";
@@ -952,7 +956,7 @@ svn_subst_translate_cstring2 (const char
   src_stringbuf = svn_stringbuf_create (src, pool);
   
   /* The easy way out: no translation needed, just copy. */
- if (! (eol_str || keywords))
+ if (! (eol_str || (keywords && apr_hash_count (keywords))))
     {
       dst_stringbuf = svn_stringbuf_dup (src_stringbuf, pool);
       goto all_good;
@@ -1194,7 +1198,7 @@ svn_subst_copy_and_translate3 (const cha
     }
 
   /* The easy way out: no translation needed, just copy. */
- if (! (eol_str || keywords))
+ if (! (eol_str || (keywords && apr_hash_count (keywords))))
     return svn_io_copy_file (src, dst, FALSE, pool);
 
   subpool = svn_pool_create (pool);

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Sep 22 14:07:30 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.