Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#870 closed enhancement (fixed)

[raster] Optimize ST_DumpAsWKTPolygons

Reported by: dzwarg Owned by: jorgearevalo
Priority: medium Milestone: PostGIS 2.0.0
Component: postgis Version: master
Keywords: Cc:

Description

I looked at the code for ST_DumpAsWKTPolygons(), and identified a good place for optimization after talking it over with Frank W at the code sprint this afternoon.

In rt_raster_dump_as_wktpolygons (see trunk/raster/rt_core/rt_api.c@6923#L1728, after calling GDALPolygonize, there are notes regarding what to do with polygons that contain NODATA cell values. Instead of iterating over both loops (which both create and destroy all features in the memory layer — copying and destroying them, effectively), it would probably be more performant to do the following steps after trunk/raster/rt_core/rt_api.c@6923#L2002:

  1. Add a sql-like filter to the layer to select the features that are ± FLT_EPSILON.
  2. Iterate over all those features, and delete them from the layer.
  3. Remove the filter, and get all the remaining features in the memory layer.

This approach means that all features in the layer are read once, even though there are 2 loops. The first loop would cycle over the features to remove, and the second loop would cycle over the features to create and return.

Change History (12)

comment:1 by jorgearevalo, 13 years ago

Owner: changed from dzwarg to jorgearevalo
Status: newassigned

Actually, there's no need of double loop. The first loop's intention was to count the valid features (polygons with a pixel value different from NODATA). But applying a spatial filter forces OGR_L_GetFeatureCount to take that filter into account, and OGR_L_GetNextFeature to return only valid features. We only need one loop.

Working on it…

comment:2 by jorgearevalo, 13 years ago

I meant "applying an attributes filter", with OGR_L_SetAttributeFilter.

comment:3 by jorgearevalo, 13 years ago

Resolution: fixed
Status: assignedclosed

Implemented in r6975. I close the ticket.

Just for the record: I trust OGR SQL executor to compare floating point numbers using "!=". Check it and reopen the ticket if the comparison:

doubleValue != anotherDoubleValue

is not safe.

comment:4 by pracine, 13 years ago

Did you get any performance improvement?

comment:5 by pracine, 13 years ago

This commit generated three new warnings…

comment:6 by jorgearevalo, 13 years ago

It works faster, because I changed two loops into only one. Working on those warnings…

comment:7 by pracine, 13 years ago

I get the regress test rt_spatial_relationship.shp to fail after your change. Apparently the set of polygon returned is different than before. It should not.

comment:8 by jorgearevalo, 13 years ago

Fixed some bugs. I'm going to have a look at spatial_relationship test right now.

in reply to:  8 comment:9 by jorgearevalo, 13 years ago

Replying to jorgearevalo:

Fixed some bugs. I'm going to have a look at spatial_relationship test right now.

Forgot to say the release: r6980

comment:10 by jorgearevalo, 13 years ago

rt_spatial_relationship is working for me at r6984. Do you still have problems with it?

comment:11 by pracine, 13 years ago

rt_spatial_relationship works but create_rt_gist_test fail:

*** create_rt_gist_test_expected	Tue Mar 29 11:32:59 2011
--- /tmp/rtpgis_reg_5884/test_25_out	Tue Mar 29 11:35:18 2011
***************
*** 1,2 ****
! SELECT 100
! SELECT 9
--- 1,2 ----
! SELECT
! SELECT

comment:12 by jorgearevalo, 13 years ago

Fixed in r6986. The problem is I was testing with PostgreSQL 9.0.3. With that version, the expected output is:

SELECT 100 SELECT 9

instead of

SELECT SELECT

This will probably require a new ticket.

Note: See TracTickets for help on using tickets.