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.hugentob...@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@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.hugentob...@sourcepole.ch http://www.sourcepole.ch > Technical Advisor QGIS Project Steering Committee > > > _______________________________________________ > Qgis-developer mailing list > Qgis-developer@lists.osgeo.org > http://lists.osgeo.org/mailman/listinfo/qgis-developer _______________________________________________ Qgis-developer mailing list Qgis-developer@lists.osgeo.org http://lists.osgeo.org/mailman/listinfo/qgis-developer