Mapserver compilation flags should be added to the SWIG command line

[Daniel Morissette]

That was bug 1244:
http://mapserver.gis.umn.edu/bugs/show_bug.cgi?id=1244

Unfortunately this bug report doesn't contain detailed profiling 
results, just total times to render a map, but we had initially 
identified the issue using gprof if I remember correctly.

Not sure what patch you have in mind, but my opinion is that the Z and M 
members of pointObj need to remain disabled by default since they double 
the amount of memory required to store a pointObj (and a shape). Also my 
unverified guess is that in the case of bug 1244, the difference in 
processing time may have been caused by the fact that the compiler could 
not optimize access to the larger pointObj struct as efficiently as it 
could for the more compact struct.

Daniel


Umberto Nicoletti wrote:
> I'm preparing to commit the GEOS patch this evening. I'll have a patch
> for Z_M ready for review tomorrow.
> 
> Umberto
> 
> On 9/1/06, Steve Lime <Steve.Lime at dnr.state.mn.us> wrote:
> 
>> Initially pointObj's did have z and m members always but DM Solutions
>> testing
>> found a significant performance penalty for doing so. Perhaps that
>> performance
>> penalty can be addressed. However, when it comes to performance we
>> should
>> strive to keep that as high as possible. I think that trumps design
>> consistency in
>> a few instances.
>>
>> Steve
>>
>> >>> Tamas Szekeres <szekerest at GMAIL.COM> 8/31/2006 5:14:19 PM >>>
>> Steve,
>>
>> Currently some members of some structs are also affected by the
>> #defines (eg. shapeObj, pointObj) so if we want to accept this way for
>> handling the issue every interface item should be generated regardless
>> of the switches. For example shapeObj must always have a "void*
>> geometry" and pointObj must always have z and m members.
>>
>> The second option would be to pass the preprocessor defines to the
>> SWIG command line. (I could not get any reason why not to do it so
>> far)
>>
>> So I think we should choose the right way according to the decision of
>> the majority of the developers. The next IRC meeting would be a
>> sufficient place to make the decision.
>>
>> Unfortunately I cannot take part on it at the proposed time. 2 hours
>> later would be more suitable for me.
>>
>> I would have +0 on the first solution and +1 on the second one.
>>
>> Tamas
>>
>>
>> 2006/8/31, Steve Lime <Steve.Lime at dnr.state.mn.us>:
>> > I'm ok with the always having the geometry there and then hiding in
>> any
>> > case.
>> >
>> > Regarding the defines with SWIG. I guess it boils down to whether or
>> > not you
>> > want:
>> >
>> >   1) a consistent interface regardless of build environment, that
>> is,
>> > something like $shape->interects(...) always exists, but will throw
>> an
>> > error if the right supporting library isn't available
>> >
>> >   2) an inconsistent interface that has different syntax depending
>> on
>> > build environment
>> >
>> > Option 1 forces folks to practice good programming and actually
>> check
>> > returned values, while option 2 is far less subtle. You'll know
>> right
>> > away if something isn't available. Not sure which I prefer.
>> >
>> > Steve
>> >
>> > >>> Tamas Szekeres <szekerest at GMAIL.COM> 8/31/2006 6:49:48 AM >>>
>> > Umberto,
>> >
>> > At this point I cannot see considerable reason not to apply those
>> > changes. But I am still sure that adding the defines to SWIG and the
>> > mapscript C compilation ensures that the SWIG interface will be
>> > generated correctly and no runtime problems will happen (dedicated
>> to
>> > this issue).
>> >
>> > But again, I am +1 adding void* geometry to shapeObj regardless of
>> the
>> > USE_GEOS setting and hiding this member from the SWIG interface.
>> >
>> > Tamas
>> >
>> >
>> >
>> > 2006/8/31, Umberto Nicoletti <umberto.nicoletti at gmail.com>:
>> > > Tamas,
>> > > I have reviewed all the .h files included in mapscript.i and I
>> have
>> > > found that there are only a few unsafe usages of defines (and they
>> > > refer to the usual suspects, that is z/m members for points and
>> > geos).
>> > >
>> > > In mapprimitive.h
>> > >
>> > > line 74 (Definitely UNSAFE!!)
>> > > #ifdef USE_POINT_Z_M
>> > >   double z;
>> > >   double m;
>> > > #endif
>> > >
>> > > line 100 (UNSAFE, but fixable):
>> > > #ifdef USE_GEOS
>> > >   void *geometry;
>> > > #endif
>> > >
>> > > Other suspicious usages could be in mapows.h, but at this time I
>> am
>> > > not sure whether they are actually posing a problem.
>> > >
>> > > I don't see an easy solution to the z/m defines, but I attach a
>> > patch
>> > > that removes the need for #ifdef USE_GEOS in mapprimitive.h. I
>> have
>> > > made a couple of tests and it seems ok to me, but I want your
>> > valuable
>> > > feedback and Steve's approval before I commit it.
>> > >
>> > > Umberto
>> > >