RFC-24 Mapscript memory management: call for comments

[Howard Butler]

On Jan 13, 2007, at 1:26 AM, Frank Warmerdam wrote:

> Tamas Szekeres wrote:
>> Umberto,
>> My personal attitude to this RFC is slightly ambivalent. On one  
>> hand I
>> appreciate your efforts to discover the similarities and the
>> differences between the various languages
>> regarding to a particular problem and it's also appealing that one of
>> our ancient problem had been tempting the mapscript users for a long
>> ago was included in the proposal.
>> But on the other hand one of the problems enumerated is not  
>> considered
>> to be a problem (by me), however a large amount of code should be
>> altered so as to "fight against it", and even I doubt that this
>> solution is working correctly as it stands.
>
> Umberto,
>
> I'm concerned about the substantial disruption to backwards  
> compatibility
> that seems to be implicit in this proposal.  I would appreciate it  
> if the
> RFC could have a section discussing the compatibility problems that  
> will
> be caused by the proposed changes.
>
> On the one hand, I appreciate the effort you have put into this  
> RFC, and
> your willingness to take on a thorny problem.   On the other hand, my
> understanding of mapscript use and the different language bindings is
> sufficiently weak that I don't really see the problem(s) being solved.
>
> /me re-reads, and really tries to grok the RFC...
>
> 2.1 Object equality/identity
>
> OK, I agree with Tamas on this. I think it is unfortunate that the
> insertLayer() is really just copying the original layer rather than
> literally inserting that instance, but I think this is just something
> we can document and it is easily worked around.  Generally speaking
> I think we should be discouraging creating "free standing" layerObj's.
>
> 2.2 Early garbage collection
>
> Tamas writes:
> > That's really an issue, and a reference to the real memory owner
> > object should be kept by the child to avoid the crash.
>
> My opinion is different.  The map is the main object, and I think it
> is up to applications to keep a handle to the map as long as they want
> to do stuff, and once the map is released (dereferenced) the
> application  should not use any part of the map.
>
> 2.3 Dynamically populated layers, classes, etc
>
> I'm afraid I'm not clear on what the issue is here.  I tried digging
> through the bug reports, but this did not turn out to be an easy way
> to understand the problem.  I'd encourage you to explain the problem,
> and perhaps refer to the bug reports as supporting information.
>
> Tamas's suggestion seems to make sense to me, but only in a rough
> sort of way since I was unable know the details.  Certainly I
> can understand that we need a way in a constructor to distinguish
> between returning a reference to a pre-existing object as opposed
> to a brand new one "owned" by the reference.
>
> I'm afraid I got quite lost in the proposed implementation.  It seems
> the changes are quite dramatic and I too worry about whether it would
> ever work properly and whether the changes will be so dramatic that
> they break all existing mapscript applications.
>
> I wish I understood swig better.  The mapscript bindings, and the
> "next generation" GDAL/OGR bindings seem so complicated that I am
> quite lost.  This no doubt means I'm a bit dumb, but I'm sure I'm
> not the only one getting overwhelmed by it.  If things get so
> complicated that only very few developers can grok them then
> getting maintenance done becomes increasingly problematic.
>
> I know that Howard is at least in a general sense enthusiastic
> about RFC 24, but I certainly feel uncertain about it so far.
> Perhaps some additional work can be done to clarify the RFC (ie.
> a compatibility section), and to try and iron things out with Tamas
> who clearly has a much better understanding than I do.
>
>

Umberto,

I also echo the kudos for writing such a detailed RFC.  Memory  
management in MapScript (as both a user and a developer) has always  
been rather treacherous, and your attempt to clear things up is  
admirable.  Frank is right as far as my enthusiasm for the *intent*  
of RFC 24, with respect to the specific implementation, I think  
putting all of this logic in the SWIG bindings and reimplementing it  
in n languages worth of customization is putting it in the wrong place.

MapServer CGI is in my opinion, one very big, powerful, and  
specialized MapScript application.  It has driven much of the feature  
development of MapServer, and how it works has driven much of  
MapServer's design.  MapServer CGI has some luxuries that MapScript  
doesn't have, and not having to worry so much about object  
referencing and dynamic memory management is probably its greatest.

There has long been a desire for a MapServer 'C' API to congeal the  
commonalities of both MapScript and MapServer CGI.  Right now, our  
'C' API is essentially the stuff in the SWIG interface files, and if  
you're looking to program with MapServer in 'C', this is the best  
place to look for examples and ideas.  Two very big components of  
MapServer don't utilize this pseudo-C API, however -- MapServer CGI  
and PHP/MapScript.  They currently exist in their own little worlds,  
and all three components (SWIG MapScript, PHP/MapScript, and  
MapServer CGI) reimplement pieces of functionality that might live in  
a common location.

I agree and am generally enthusiastic about the *intent* of RFC 24,  
but with respect to the specific implementation, I think implementing  
it as customizations for each MapScript language (and depending on  
SWIG's uneven support for customizing each MapScript language) is the  
wrong place to put it.  This approach will create a lot of work for  
each MapScript maintainer and, in my opinion, lead to even more  
uneven MapScript implementations.  It won't do anything for PHP/ 
MapScript (which also has the same problem).

I think that object lifetimes should be MapServer's duty to manage --  
not the SWIG layer's and most definitely not the user's code  
(contrary to Frank's opinion).  The whole point of garbage collection  
is to abstract away object memory management, and implementations  
that do it half-way, like GDAL's language bindings and SWIG  
MapScript, are in some ways worse than having no object management at  
all.  Most users of our MapScript bindings don't come to the party  
with backgrounds in worrying about who "owns" anything, and if they  
are looking for that level of granularity, performance, and control,  
they shouldn't be looking at writing their stuff in MapScript in the  
first place.  Ease should be the focus, and the definition of ease is  
not having to worry about reassigning a mapObj to a new variable and  
things blowing up when you do so.

As it stands now, I am +1 on the intent of RFC 24 but -1 on where it  
is being implemented.  I think this is MapServer 'C' API stuff, and I  
am very concerned that implementing it all in the SWIG bindings will  
lead to even more uneven MapScript implementations.  Not every  
language supports the %feature("*code") feature, and even if it did,  
we would still be repeating ourselves across all of the languages.   
If we've given MapServer enough information to know about its  
objects, MapServer should take care of doing the right thing with  
them.  Another strike against implementing it where RFC 24 currently  
proposes is that it does nothing for PHP/MapScript or MapServer CGI  
(hey, that's becoming more dynamic every day ;).

Howard