#1244 closed defect (fixed)
[Performance] PointObj cause performance lost
Reported by: | jlacroix | Owned by: | mapserverbugs |
---|---|---|---|
Priority: | high | Milestone: | 4.6 release |
Component: | MapServer C Library | Version: | unspecified |
Severity: | normal | Keywords: | |
Cc: | jmckenna@…, sgillies@…, fsimon@… |
Description
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.
Change History (21)
comment:1 by , 19 years ago
blocked: | → 1224 |
---|
comment:3 by , 19 years ago
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;
comment:4 by , 19 years ago
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.
comment:5 by , 19 years ago
Milestone: | → 4.6 release |
---|---|
Status: | new → assigned |
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
comment:6 by , 19 years ago
Sounds ok I guess. (I was gone on vacation last week which is why you didn't hear from me...)
comment:7 by , 19 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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
comment:8 by , 19 years ago
Thanks Julien, that's a significant enough gain to warrant the change then... Steve
comment:9 by , 19 years ago
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.
comment:11 by , 19 years ago
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?
comment:12 by , 19 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
I'll change shape-z-m to point-z-m Reopening
comment:13 by , 19 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I commited my changes to use USE_POINT_Z_M and --enable-point-z-m. Marking as FIXED
comment:14 by , 19 years ago
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.
comment:15 by , 19 years ago
Cc: | added |
---|
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.
comment:16 by , 19 years ago
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.
comment:17 by , 19 years ago
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.
comment:18 by , 19 years ago
Cc: | added |
---|
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).
comment:19 by , 19 years ago
Jeff, Fernando has found and fixed the issue. There is probably no need to duplicate the testing effort.
Note:
See TracTickets
for help on using tickets.