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

[Nyall Dawson]

On 23 June 2015 at 16:25, Werner Macho <werner.macho at gmail.com> wrote:
> 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
>

mmmm... I'm not in favour of treating non LTS releases as "beta" type
releases though. There's been potentially 10s of thousands of dollars
of investment in 2.10, and people who have sponsored features are
still expecting a stable release when 2.10 comes out.

Nyall


> 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
> _______________________________________________
> Qgis-developer mailing list
> Qgis-developer at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/qgis-developer