Re: [PATCH] Issue 2016 (was Re: Re: 1.1rc1 performance regression in 'svn status' (and also 1.1rc2))

Your docstring for svn_utf_initialize says:

+/** @since New in 1.1.
+ * Initialize the utf8 encoding/decoding routines.
+ * NOTE: It is optional to call this function, but if it is used, no other
+ * svn_utf function may be in use during the call of this function.
+ * Initializing the UTF-8 routines will improve performance.
+ */
+void svn_utf_initialize (apr_pool_t *pool);

There was a reason I said "no Subversion library function may be in use"
when I described the constraint; it's clearer to describe it that way.
Almost any Subversion library function is liable to use svn_utf
functions.

If you're going to take a pool parameter, document that the pool must
live for as long as Subversion library functions are in use.
(Another option is to create a pool from scratch like we do in
svn_error_create(). But on consideration, I think that's a bad idea,
since that pool could never be cleaned up.)

+ if (utf_initialized++ == 0)

If you're treating utf_initialized as a flag, then it's stylistically
confusing to operate on it like it's a counter. APR treats its
initialization static as a counter because it wants to count
apr_terminate() calls to know when it should clean up.

I'll review more thoroughly when you've written a log message. It's
good to do that before asking for review.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Aug 26 19:59:24 2004