RFC 17: Dynamic Array Sizing

[Daniel Morissette]

Frank Warmerdam wrote:
> Benedikt Rothe wrote:
> 
>>
>> Hi
>>
>> Sorry for posting in this list since I'm no mapserver-developer, but I 
>> wonder wether RFC17 may break
>> mapscript-applications, which hold pointers to mapserver-Objects.
>>
>> Example Java:
>> - A Java-layerObj is created with "aMapObject.getlayer(1)". This 
>> layerObj-instance holds a pointer
>>   into the Layer-Array of the aMapObject.
>> - After this, a new layer is added to the webobject.  The layers-Array 
>> in the webObject could be reallocated
>>   by this action.
>> - Therfore the existing pointer in the Java-layerObj could become 
>> invalid.
>> This could cause crashes.
>>
>> Did I oversee something?
>> Benedikt
> 
> 
> Benedikt,
> 
> Yikes, you are quite right.  In fact whenever we actually end up "growing"
> the list of layers, classes or styles any old pointers could be 
> invalidated.
> I don't forsee this occuring too often in the mapserver core itself because
> it isn't common to keep pointers around.  Instead it is layer/class/style
> indexes that are normally kept over time.
> 

This would be an issue in PHP MapScript too. There may be ways to update 
the pointers in the PHP object wrappers after a reallocation has 
happened, but that would require some work.


> 
> I *suppose* we could move to individual allocations in mapserver as well.
> But I think this would cause a lot of changes in the code that would make
> the effort much bigger, as well as resulting in more memory fragmentation.
> 

True.

> 
> I think I would be willing to try and implement individual allocation of
> the layer/class/style objects if the developer would like to take this
> approach.  The downside is that there is likely to be changes required in
> parts of MapServer I don't normally build (like specific data providers
> and specific output drivers).  There is also likely to be more work 
> required
> in the swig bindings.  Should I rewrite the RFC with this intent?
> 
> To be clear, most of the RFC would remain the same, but something like:
> 
>   layerObj *layers;
>   int numlayers; /* number of layers in mapfile */
>   int maxlayers;
> 
> would become:
> 
>   layerObj **layers;
>   int numlayers; /* number of layers in mapfile */
>   int maxlayers;
> 
> So "layers" is a pointer to an array of layerObj pointers.  When the map is
> initialized each of the initial MS_MAXLAYERS layerObj would be individually
> malloced, and initialized. 

Is it really a good idea to pre-alloc/initialize all objects when the 
map is initialized? Should we not instead find a way to alloc/init on 
demand?

The current default limits are:
   #define MS_MAXCLASSES 250
   #define MS_MAXSTYLES 5
   #define MS_MAXLAYERS 200

If we stick to those values then when the map is initialized we would 
alloc and init 200 layerObjs, 50,000 classObjs and 250,000 styleObjs ... 
that's an huge number of memory allocations and I fear that will have a 
very big impact on performance, especially when we consider that in most 
cases we need less than a hundred objects total, or a few hundreds in 
large maps.

Perhaps if we really want to stick to the pre-init/alloc approach then 
we should reduce the default MS_MAX* values to something more reasonable 
like:

   #define MS_MAXCLASSES 4
   #define MS_MAXSTYLES 2
   #define MS_MAXLAYERS 8

Resulting in the pre-allocation/initialization of 8 layers, 32 classes 
and 64 styles.

Then if the grow functions double the number of objects when they 
realloc, there should not be a significant impact on maps with large 
numbers of layers, or layers with large numbers of classes, etc.

Daniel
-- 
Daniel Morissette
http://www.mapgears.com/