I wanted to thank everyone for the feedback on the suggestion. This
is my understanding of the chain so far:
1) The getenv() call reduces performance by 1% in the hyper-often call
scenario (the first suggestion)
2) A static with some kind of synchronization, using Joseph's
suggestion would reduce this performance penalty
As to (2), we could use pthread (used by apr) and something like:
pthread_mutex_t svn_admin_dir_mutex = PTHREAD_MUTEX_INITIALIZER ;
char * getadmdir()
{
static char * szAdmDir = 0 ;
pthread_mutex_lock( &svn_admin_dir_mutex ) ;
if( szAdmDir == 0 )
{
szAdmDir = getenv( "SVN_ADM_DIR" ) ;
if( szAdmDir == 0 )
szAdmDir = ".svn" ;
}
pthread_mutex_unlock( &svn_admin_dir_mutex ) ;
return szAdmDir ;
}
Thanks,
Jonathan
On 6/29/05, Joseph Galbraith <galb@vandyke.com> wrote:
> Duane Griffin wrote:
> > On Wed, 2005-06-29 at 18:26, Joseph Galbraith wrote:
> >
> >>char* getadmdir()
> >>{
> >> static char* szAdmDir = 0;
> >> if ( szAdmDir == 0 )
> >> {
> >> szAdmDir = ::getenv("SVN_ADM_DIR");
> >> if ( szAdmDir == 0 )
> >> szAdmDir = ".svn";
> >> }
> >>
> >> return szAdmDir;
> >>}
> >
> >
> > Aside from the possibility of non-atomic pointer assignment, there is
> > another failure case where the function returns NULL:
> >
> > Consider the situation with two threads A and B where SVN_ADM_DIR is not
> > set. A and B both enter the function and the first if block. Thread A
> > continues into the second if block and does the assignment. Now thread B
> > executes the getenv line. Thread A executes the return statement,
> > returning the current global value of szAdmDir, which has been set by
> > thread B to NULL. Bang.
> >
> > It really isn't possible to do this sort of thing in a thread-safe
> > manner without synchronization.
>
> Well, given that the non-atomic pointer assignment does seem to kill
> it in the portable sense, it probably isn't worthwhile continuing this
> discussion too much further. (And the objection to the use of
> environment variables for config in general-- though there is precednet
> in the SVN_SSH environment variable.)
>
> But, wouldn't the introduction of a temporary resolve the
> problem you point out above?
>
> char* szEnv = ::getenv("SVN_ADM_DIR");
> if ( szEnv == 0 )
> szAdmDir = ".svn"
> else
> szAdmDir = szEnv;
>
> Now szAdmDir is written exactly one time (in each thread), always
> with a valid value.
>
> Thanks for the careful analysis, by the way. I'll keep an eye
> out for that particular mistaken pattern in my own code.
>
> Thanks,
>
> Joseph
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jun 30 01:36:03 2005