[Qgis-developer] Potentially serious performance regression in new geometry - should 2.10 be delayed?

[Werner Macho]

Hi!
I would be in favor of releasing it as is.

1. This is not the _LTS_ release (I thought this is exactly why we
introduced LTS) - so people should know that there are new features
which sometimes can cause "something"
2. releasing it with the changes means more feedback if this is really
a problem and more time with lots of testing to fix it until the next
LTS

Though there should probably a release note to make people aware and
ask them to test this very well.

kind regards
Werner

On Tue, Jun 23, 2015 at 8:15 AM, Marco Hugentobler
<marco.hugentobler at sourcepole.ch> wrote:
> Hi Nyall
>
>>This potentially has huge impacts on the performance of common tasks such
>> as selecting all features which intersect a geometry.
>
> Are you really sure the impact is huge? If you select features on the map,
> you get a new one from provider each time. So also with the old geometry
> class, it was necessary to convert to geos for each intersect.
>
> But yes, if several geos operations are done in sequence on really the same
> geometry instance, there is an overhead now. However, I would assume the
> geos conversion takes far less time than the geos operation itself. Did you
> observe cases where the geos conversion really takes very long time?
>
> Lets assume the geos conversion indeed takes a long time. Couldn't we just
> store the pointer to QgsGeos (or better QgsGeometryEngine class) inside
> QgsGeometry? This would be straightforward to do. Furthermore, it is
> possible to prepare QgsGeos. A prepared geos geometry can do _much_ faster
> intersects/touches/ etc. operations if called repeatedly. We can cache a
> prepared QgsGeos now with the new engine, but we couldn't do that with the
> old QgsGeometry class. So for repeated predicate calculations, the new
> geometry is much better, also performance wise.
>
>
> Regards,
> Marco
>
>
> On 23.06.2015 04:09, Nyall Dawson wrote:
>
> Hi all,
>
> Unfortunately, we've become aware of a serious performance regression caused
> by the new geometry engine. Basically, the situation is that for all
> geometry operations which rely on geos (think buffers, splits, spatial
> relation operations such as intersects and within,... ) the geometry now
> needs to be converted into a geos representation with *every* operation. In
> the old geometry engine this conversion was done once, and the result stored
> so that follow up operations would not need to recalculate it. This
> potentially has huge impacts on the performance of common tasks such as
> selecting all features which intersect a geometry.
>
> I've had a look, and unfortunately it's not trivial to fix this. I think the
> correct solution to this is to:
>
> - make QgsGeometryEngine accept and return QgsGeometry containers, not
> QgsAbstractGeometryV2
> - store the generated geos representation of geometries within
> QgsGeometryPrivate inside the QgsGeometry container. This way it will be
> reusable between different geometry operations, and shared when QgsGeometry
> objects are copied. This will also have the benefit that if a geometry is
> prepared using geos then subsequent geos operations performed on that
> QgsGeometry and its shared copies will be much faster.
> - make QgsGeometry a friend class of QgsGeo, so that it can access
> QgsGeometryPrivate to retrieve or set the geos representation of the
> geometry as required
>
> An alternative (short term) solution would be to just cache the geos
> representation when geometry operations are called through the older
> QgsGeometry modification/relationship operations. This would be easier, but
> means that the API of QgsGeometryEngine will be stuck with the current
> design, and we won't be able to properly fix this until breaking the api for
> 3.0.
>
> Either way, I doubt this can be addressed within the remaining 3 days we
> have until release. Should we delay to address this? Release with the
> regression? Or am I missing something and there's an easier solution we
> could implement? Or even possibly this additional cost of recalculating the
> geos representation is trivial and can be ignored (maybe someone could test
> this with a little repeated intersection script)?
>
> Thoughts?
>
> Nyall
>
>
>
> _______________________________________________
> Qgis-developer mailing list
> Qgis-developer at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/qgis-developer
>
>
>
> --
> Dr. Marco Hugentobler
> Sourcepole -  Linux & Open Source Solutions
> Weberstrasse 5, CH-8004 Zürich, Switzerland
> marco.hugentobler at sourcepole.ch http://www.sourcepole.ch
> Technical Advisor QGIS Project Steering Committee
>
>
> _______________________________________________
> Qgis-developer mailing list
> Qgis-developer at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/qgis-developer