Hi Robert,
I accept your point about keeping code readable/maintainable so I've looked at 
this again with a view to keeping the code readable, while making it faster on 
all platforms.
I've changed two loops:
- I experimented with a wide range of bit-counting techniques to replace the 
first loop (including some from here: 
http://graphics.stanford.edu/~seander/bithacks.html). Typically only the first 
4 bits are set, so I've used the method that tested fastest for that particular 
case (the Kernighan & Ritchie way).
- The second loop is largely unchanged, I've just replaced the iterator with an 
index. This is faster for MSVC, and shouldn't make a significant difference for 
other compilers. Arguably it also makes the code more readable.

I hope you find these changes acceptable
Thanks
-Michael

-----Original Message-----
From: [email protected] 
[mailto:[email protected]] On Behalf Of Robert 
Osfield
Sent: 07 May 2009 14:03
To: OpenSceneGraph Submissions
Subject: Re: [osg-submissions] Checked iterator workarounds

HI Michael,

I've done a review of your changes.  I've merged the changes to CMakeLists.txt 
and checked them in to svn/trunk.

The changes to Group::traverse is simply not a step forward for the code, it's 
more code, and less readable.  I'm perplexed by the changes to Polytope, as 
you've changed one loop to using indicies, but not a second loop.

One of main priorities is keeping the OSG code base robust and maintainable, 
I'm afraid neither of changes to Group nor Polytope advance this, but take a 
step backwards.  It's simply not appropriate to start introducing peculiar 
workarounds to perfectly valid and optimal OSG code just for one particular 
compiler's bugs.

Robert.

On Mon, May 4, 2009 at 6:50 PM, Michael Platings <[email protected]> 
wrote:
> This time with a subject line :P
>
> Hi Robert,
> further to the discussion on Visual C++ checked iterators, I've done 
> some profiling which suggests that Group::traverse and 
> Polytope::setAndTransformProvidingInverse are the two functions that 
> would benefit most from some trivial optimisation. I've attached the 
> changes I've made, which should be fairly uncontroversial.
> For the benefit of those who can disable checked iterators, I've added 
> an option for CMake.
> Thanks
> ______________________________________________________________________
> This email and any files transmitted with it are confidential and 
> intended solely for the use of the individual or entity to whom they 
> are addressed. If you have received this email in error please notify 
> the system manager.
>
> This email has been scanned by the MessageLabs Email Security System.
> For more information please visit http://www.messagelabs.com/email 
> ______________________________________________________________________
>
> _______________________________________________
> osg-submissions mailing list
> [email protected]
> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscene
> graph.org
>
>
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org

______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email 
______________________________________________________________________

______________________________________________________________________
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email 
______________________________________________________________________

Attachment: Polytope
Description: Polytope

_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org

Reply via email to