[Performance] PointObj cause performance lost

[tbonfort]

Reporter: jlacroix
Date: 2005/02/14 - 22:01

In bug 1224, I found that that a performance lost is observed since we add the Z
parameter in the point object. Just because the pointObj is bigger, it takes
longer to access the parameters of the pointObj.
A line like:
      x1 = shape->line[i].point[j].x;
      y1 = shape->line[i].point[j].y;
takes between 15% and 50% more time.

Daniel and I talked about it and he proposed to put all access to the m and z
parameters inside a 
#ifdef USE_SHAPE_Z_M
...
#endif

By default we could make the m and z options disabled since most of the users
don't use it.

I made some test and it seems that if we put the m and z inside the ifdef, we
gain with this change:
(50 calls to the gmap mapfile)
With the M and Z parameters in point object
4.4: 7.56
4.0: 6.18

With all M and Z parameters inside not enabled ifdef
4.4: 7.10
4.0: 6.23

I propose to commit that in 4.5, but this would require changes to the core and
to mapscript (all flavor). So I want to inform you and maybe have some comment
before.

[tbonfort]

Author: sdlime
Date: 2005/02/15 - 00:32

Hmmm... I thought z and m values were stored at the shapeObj level rather than 
at the pointObj level although I can understand why you might want to do both. 
I hesitate to add yet another compile time option.

Steve

[tbonfort]

Author: woodbri@swoodbridge.com
Date: 2005/02/16 - 00:36

I would think the the compiler optimization should deal with the
"shape->line[i].point[j]" as a semi-invariant within the loop, but maybe it is
not. You should try some manual optimization in case the expression is too
complicated for the compilier to optimize correctly. Like:

 pobj = shape->line[i].point[j];
 x1 = pobj.x;
 y1 = pobj.y;

[tbonfort]

Author: jlacroix
Date: 2005/04/07 - 22:32

Steve, I know we already have a lot of compile option, but How can we fix that
if not by a compilation option?

We may move the m and z parameters alone in a new object (PointDimensionObj) and
add the new object in the shape object. This new object could contain a pointer
to the x and y values. It should take care of the performance lost without
adding the compilation option. However, I'm not sure how easy and how clean it
is to do that.

Any other idea?

Stephen Woodbridge: I'm pretty sure the compiler takes care of this issue.
Others may confirm or contradict that.

[tbonfort]

Author: jlacroix
Date: 2005/04/12 - 17:53

Steve, since I did not receive any response, I will integrate the compile
option. However, if we want, we may change that later.

Aiming 4.6

[tbonfort]

Author: sdlime
Date: 2005/04/12 - 20:46

Sounds ok I guess. (I was gone on vacation last week which is why you didn't 
hear from me...)

[tbonfort]

Author: jlacroix
Date: 2005/04/14 - 17:27

I commited my changes to remove the Z and M parameters in th pointObj by
default. Most of the work was pretty straight forward, but I did not test with
swig mapscript nor windows. The new configure flag is --enable-shape-z-m and the
define is USE_SHAPE_Z_M.

This gives around 7 to 10% performance gain in the overall.

Marking as FIXED

[tbonfort]

Author: sdlime
Date: 2005/04/14 - 18:02

Thanks Julien, that's a significant enough gain to warrant the change then...

Steve

[tbonfort]

Author: sgillies@frii.com
Date: 2005/04/14 - 21:02

I've made a few changes to mapscript and the Python tests pass without the 
--enable-shape-z-m option.

I haven't run the tests with Z and M enabled.  Making this a compile option 
effectively doubles the amount of testing I have to do now, did you realize?
It was enough to stay on top of one MapServer, but now we have two different
programs and I have to stay on top of two configurations and builds.  Sort of
a pain.

My last two cents is that the option and macro should be changed to
--enable-point-z-m and USE_POINT_Z_M because these attributes are possessed 
by pointObj, not shapeObj.

[tbonfort]

Author: sgillies@frii.com
Date: 2005/04/14 - 21:09

Tests pass with --enable-shape-z-m

[tbonfort]

Author: dmorissette
Date: 2005/04/14 - 22:32

I agree it would make sense to change to --enable-point-z-m and USE_POINT_Z_M
Could you please make that change Julien?

[tbonfort]

Author: jlacroix
Date: 2005/04/14 - 22:43

I'll change shape-z-m to point-z-m
Reopening

[tbonfort]

Author: jlacroix
Date: 2005/04/21 - 17:16

I commited my changes to use USE_POINT_Z_M and --enable-point-z-m.

Marking as FIXED

[tbonfort]

Author: fsimon@univali.br
Date: 2005/04/21 - 18:42

Hi folks,
I will check the code for maporaclespatial.c.
A user report that using the last version of cvs the points from Oracle 
Spatial don't appear in Map.
Thanks.

[tbonfort]

Author: fsimon@univali.br
Date: 2005/04/28 - 16:16

Hi folks,
    I did the tests with Oracle Spatial connection, but the point only appear
with --enable flag. When I don't use this flag or user the --disable the points
(points, polygons, and lines) don't appear.
    After this I checked the maporaclespatial.c and I didn't fine problem with
the last changes. In my tests is only wotk with --enable-point-z-m
    Is it working with another connections (shape, PostGis)? Any tests?
    Thanks.

[tbonfort]

Author: jlacroix
Date: 2005/04/28 - 17:32

Shape, Tab, OVF work. I guess PostGIS works too since there's multiple user of
it.  I have few oracle data here. I will make some test.

[tbonfort]

Author: fsimon@univali.br
Date: 2005/04/28 - 21:21

Hi,
    Problem solved.
    The SDOPointObj "need" the z value. It's a internal point Object for Oracle
(only used by Oracle). So, I remove the ifdef for this typedef and worked
without --enable flag. 
    I will do more tests to check it, but I belive that it's finished. 
    I saw in the code that only Oracle Spatail read and set the real z values
for points, it's right? PostGis, OGR, and Proj set the value to 0, right?
    Thanks.

[tbonfort]

Author: jmckenna@dmsolutions.ca
Date: 2005/04/28 - 22:14

i'm setting up to test this issue with Oracle for Julien (although i don't have
the skill to go as deep as you guys).

[tbonfort]

Author: dmorissette
Date: 2005/04/28 - 22:28

Jeff, Fernando has found and fixed the issue. There is probably no need to
duplicate the testing effort.

[tbonfort]

Author: jmckenna@dmsolutions.ca
Date: 2005/04/28 - 22:36

sorry, i should have added myself to this bug earlier

[tbonfort]

Author: fsimon@univali.br
Date: 2005/04/29 - 00:29

Updated the new version of maporaclespatial.c