Thanks David, while this is marginal improvement I have merged with svn/trunk the change as on code review it looks pretty safe. I won't merged the change with the OSG-2.8 branch as there really is no need to, the changes don't solve a bug or make the code particularly more readable or efficient so I won't risk making changes as the OSG-2.8 branch will get far less testing that the upcoming OSG-2.10 release.
On Mon, Jul 13, 2009 at 2:34 PM, David Fries<[email protected]> wrote: > Comparing the win32 barrier to the pthread barrier, win32 puts the > while in an else clause. When if is true the phase changes and the > while condition will always by false, so might as well put the while > in the else to skip the check. There's also a benefit to having the > code logic similar between platforms. > > SprocBarrier.c++ is the same as pthreads without the else, I just > don't like to submit changes without being able to at least compile > it. > > win32: > my_phase = pd->phase; > if(pd->cnt == pd->maxcnt) { > pd->phase = 1 - my_phase; > ...broadcast... > } else { > while(pd->phase == my_phase) { > ...wait... > } > } > > pthreads: > my_phase = pd->phase; > if(pd->cnt == pd->maxcnt) { > pd->phase = 1 - my_phase; > ...broadcast... > } > while(pd->phase == my_phase) { > ...wait... > } > > > Index: src/OpenThreads/pthreads/PThreadBarrier.c++ > =================================================================== > --- src/OpenThreads/pthreads/PThreadBarrier.c++ (revision 10273) > +++ src/OpenThreads/pthreads/PThreadBarrier.c++ (working copy) > @@ -173,15 +173,15 @@ > pd->cnt = 0; // reset for next use > pd->phase = 1 - my_phase; // toggle phase > pthread_cond_broadcast(&(pd->cond)); > - } > + }else{ > + while (pd->phase == my_phase) { > > - while (pd->phase == my_phase) { > + pthread_cleanup_push(barrier_cleanup_handler, &(pd->lock)); > > - pthread_cleanup_push(barrier_cleanup_handler, &(pd->lock)); > - > - pthread_cond_wait(&(pd->cond), &(pd->lock)); > + pthread_cond_wait(&(pd->cond), &(pd->lock)); > > - pthread_cleanup_pop(0); > + pthread_cleanup_pop(0); > + } > } > } > > -- > David Fries <[email protected]> > http://fries.net/~david/ (PGP encryption key available) > > _______________________________________________ > osg-submissions mailing list > [email protected] > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org > > _______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
