[geos-devel] Re: GEOS Improvements from Safe

[strk at refractions.net]

Graeme, thanks for your contribution.
Here are a few notes:

1) CGAlgorithms was no different from current CVS.

2) NonRobustCGAlgorithms is currently unused, like RobustCGAlgorithms
   we have a single CGAlgorithms being the robust implementation.
   I've removed the other two from the list of object compiled into
   the library.

3) The only applied patch was agains mergeZ() function, removing
   Coordinates copy. What remains are the 'controversial' ones.

I'd discuss the 'controversial' ones on geos-devel.
I think the Geomgraph change could be ported to JTS as well
while for the CoordinateSequence we should probably port
JTS changes, which allow higher dimensioned Coordinates.

If - as will probably be - we're going to work on GEOS3 we might
make the CoordinateSequence a template class allowing for functions
inlining still keeping the flexibility of using other Coordinate
implementations. This would require more analisys though. Applying
your patches now would work for an eventual 2.1.2 version of GEOS
altought I'm really not sure about ABI compatibility with 2.1.1...
should check it.

Finally.. do you have any performance test comparing runs
with OPTIMIZE_COORD_SEQUENCE_FOR_SPEED on and off ?

--strk;

On Wed, Apr 20, 2005 at 08:53:22AM -0700, Graeme Hiebert wrote:
> On 14 Apr, 2005, at 12:04, Paul Ramsey wrote: 
> 
> 
> > Thanks Dale, get it touch w/ Sandro when you're ready to go, 
> > Paul 
> > 
> > Dale Lutz wrote: 
> > 
> > > Hi Paul, 
> > > This is something Graeme (Hiebert) and I have to connect one, we 
> > > definitely 
> > > want to get this in right away.  Graeme is out this week at a 
> > > course until 
> > > Thursday, but it should be top of our list to get our 
> > > improvements your way 
> > > then. 
> > > Dale 
> > > 
> > 
>  
> Hi guys. 
> 
> I'm attaching a tarfile of the changes we made.  These are not 
> extensive by any means, but they make a tremendous difference in 
> performance for us.  (The reason it took so long to get them to you was 
> more of a resource contention issue than anything else - Dale and I 
> have just been too busy.) 
> 
> The set of .cpp changes is really completely orthogonal to the set of 
> .h changes.  The .cpp changes simply use some "const Coordinate &p" 
> variables in place of "Coordinate p" in a few places, to reduce 
> coordinates being copied. 
> 
> There are essentially two changes made to the .h files.  Each of these 
> is enabled by defining a macro in platform.h.  I've attached our own 
> platform.h as a sample, but I'll leave it up to you to fit this into 
> the existing GEOS autoconfigure framework. 
> 
> The first change, enabled with the OPTIMIZE_COORD_SEQUENCE_FOR_SPEED 
> macro, changes the way CoordinateSequence is defined.  Inline functions 
> are used instead of virtual functions in a few places.  This breaks the 
> ability to have subclasses with different kinds of coordinates, but 
> makes an amazing difference in performance. 
> 
> This change looks a little weird, because the DefaultCoordinateSequence 
> still exists, but its "vect" private member has been moved as a 
> protected member to CoordinateSequence.  It also still has non-inline 
> versions of the inline functions, but these will not normally be 
> called, because most (all?) calls to them are done through the 
> CoordinateSequence interface, which will instead get the inlines. 
> 
> The point_3d and PointCoordinateSequence classes are completely 
> disabled when this macro is defined, so that they are not accidentally 
> used.  Any code using a PointCoordinateSequence would implode if the 
> OPTIMIZE_COORD_SEQUENCE_FOR_SPEED macro was defined. 
> 
> The second change is enabled with the macro 
> ALLOW_GEOMETRYGRAPH_FOR_RELATE_SUBCLASS.  It is not nearly as useful to 
> most people, but it makes a big difference for the way some of our code 
> operates at Safe.  It just makes a couple of GeometryGraph's methods 
> virtual to allow us to make a subclass of GeometryGraph which is 
> suitable for performing Relate operations (though probably not for much 
> else).  Because we perform a series of relates against the same object, 
> this lets us precompute a GeometryGraph object and reuse it for 
> multiple calls to RelateComputer::computeMatrix() without crashing. 
> 
> I think that's everything.  Let me know if you have any questions. 
> 
>    -g 
>