faster getpoint

[rafaqz]

getpoint is pretty slow currently, to the point rasterizing a shapefile from ArchGDAL.jl is 20x slower than one from Shapefile.jl. Mostly because getting points allocates every time.

This PR greatly (but not completely) reduces the gap by:

1. Not returning GDAL points. They are pretty inefficient when we have thousands of points. A Tuple is better.
2. Preallocating Ref for getpoint! in the iterator for Geointerfce.getpoint(linestring), and reusing them for each point.

This is implemented for line string but that means polygon will also work. It's probably not worth the effort of passing the Ref through from polygons, but I haven't checked. There may be more objects that need this treatment.

Edit: I assumed this would have a test already, but seems not.

[evetion]

I was slightly hesitant about changing the return type here, but I think this still fits as it qualifies as point.

We could better document that we essentially are changing GeoInterface.coordinates to be a nested Vector of a Tuple instead of a Vector. That has impact on some construction methods as well.

[rafaqz]

Yeah, I tried to optimise the gdal point first but the overhead of allocating them all individually is just really bad.

Once all the methods here accept any point it won't make much difference either.

And yes I've noticed there's still a lot of nested vector around. It would be a good performance gain to swap them all to tuples.

[rafaqz]

Another thought I had writing this is it would be faster to use getx, gety etc because they don't need allocations at all.

But it assumes trad point order, so I went with getpoint!.

[yeesian]

I'd err on the side of assuming this is a breaking/major change.

There may be more objects that need this treatment. [...] Once all the methods here accept any point it won't make much difference either. [...] And yes I've noticed there's still a lot of nested vector around. It would be a good performance gain to swap them all to tuples.

Can we do it all within a single PR? I don't think it'd be a good idea to have inconsistencies in return types across geometry types.

[rafaqz]

I guess its a breaking change? But calling GeoInerface methods returns a lot of types of points already, and it doesn't promise what kind of point comes from what parent object.

I also doubt it will actually break much.

We should actually put this on getgeom and I guess it's just multipoint missing? I'm not sure that will be faster because it's accessed differently.

[yeesian]

LGTM because of #369 (comment) --

I was slightly hesitant about changing the return type here, but I think this still fits as it qualifies as point.

Will get a second opinion from @evetion nevertheless just in case there's anything other context from GeoInterface I might be missing

[evetion]

Do we have a roundtrip test? I.e. ArchGDAL.createpoint(GeoInterface.getpoint))? Similarly for GeoInterface.coordinates, if this PR changes that.

[rafaqz]

It doesn't touch coordinates. But yes round trip on points would be good. I think that should just work already.

Edit: a tuple is just a normal input for createpoint, it's already tested..

Edit2: I added round trip tests for LineaRing and LineString

[visr]

Nice speedup, looks good to me!