[mapguide-internals] Exception Handling for Memory limitations+ Patch for #480 & #520

[Haris Kurtagic]

I have worked a lot with MapGuide and FDO code after it was released as
Open Source.

I think that MapGuide and FDO are great pieces of software.
I am very glad that creators of it decided to share it with us.

Haris

-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org
[mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of UV
Sent: Friday, April 03, 2009 6:49 AM
To: MapGuide Internals Mail List
Subject: Re: [mapguide-internals] Exception Handling for Memory
limitations+ Patch for #480 & #520

Sorry,

my apology that this did not come accross correctly...
I was meaning in this case that I was exhausing my possibilities to 
solve deeper errors after following the trace into the SDF Provider.
That was refering to this particular debugging session ending up in 
sparsely documented code between API borders not having a clue whats 
going on.

Do you really claim that all parts of this code base have been written 
with the next developer trying to understand it in mind?
I am not saying this is a problem all over. I just ended up in a few 
places where I think this is a valid claim.

And I am not trying to bash anyone. We all write code with the knowledge

at the time we wrote it. And this changes over time.
So no code is written in stone. Keyword maintainability.
Setting new documentation standards is just a change in paradigms as the

code base is being opened and it needs to be openly discussed and some
clear standards being set. I clearly understand the situation where this

happens but I also believe this has to change.

Code maintenance should become a different priority now that its open 
source.
And code reviews are something thats hard to promote in developer land I

know by experience. however its the best way to make the best code.
I worked in agile projects with pair development and constant reviewing 
sessions.
It helps everyone and we all get used that if someone has a remark to 
make to our code it might be useful.

With the documentation standards this is particular difficult as the 
know-how sits almost exclusively at autodesk.
So any external developer runs into the default scenario of lacking 
information and the need of support. This can get very tedious.

I do understand that this is not done from one day to the other and that

its not a general problem in all areas of the code base
but we should still find a way to agree on a reasonable standard also to

be able to tell it to new members of the community.
Its all about initiating the right processes.

_In the end putting the right information into the code base on the side

might be less work than answering emails on the internals list all the 
time... think about this...
_
Sorry again if you understand this as bashing.
Thats the general problem in defining new coding styles and standards as

it means that some older code does not adhere to it.

But I dont understand this as bashing - this is just a new goal for a 
code base which carries all this great know-how and we dont want to 
loose it by obfuscation.

Cheers,
UV

==  ByteSink::ToFile ==
Can we guarantee that any failure during the method leaves us with an 
invalid file?
I was not sure so I added the check as I had some non empty files which 
I could not determine to be invalid.
I have a vague idea of whats happening in there but no idea which parts 
of the code ar using it except the tile writing.


Walt Welton-Lair wrote:
> Hi UV,
>
> I worked on ByteSink::ToFile earlier today to fix ticket 480 and made
most of the same changes as you.  Was planning on submitting tomorrow.
I didn't do the halving of the memory allocation, but what you did looks
reasonable.  Also, in my change I delete the file if any exception is
encountered - in your case you delete it only if it's empty.  Both seem
like reasonable choices.  When an exception occurs it's possible the
file got written out completely and is valid, but it's also possible it
was only partially written and therefore not valid.  Any more thoughts
on that?
>
> One other thing - I realize you're frustrated at times working with
the overall code.  Some parts are not documented as well as they could
be, some parts are not coded as well as they could be.  But comments
like "I am just slowly getting fed up reading a lot of code that has
clearly 
> not been written to be understood by other people" are not going to
help.  Despite the code not being optimal, a lot of people put a lot of
work into this code and so in a way you're bashing them.  Instead,
channel your frustration to actually help improve the code.  Like
submitting these patches - these are very much appreciated.
>
> Walt
>
>
> -----Original Message-----
> From: mapguide-internals-bounces at lists.osgeo.org
[mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of UV
> Sent: Thursday, April 02, 2009 11:17 PM
> To: MapGuide Internals Mail List
> Subject: [mapguide-internals] Exception Handling for Memory
limitations + Patch for #480 & #520
>
> Hi there,
>
> I am trying to make the mgserver behave in the case of limited
memory..
> We are using a massive map for our client and the server keeps on 
> falling over. (did not reproduce in 2.0.2)
>
> I followed the trace from StylizeLayerinto the SimpleSDFReader which 
> fails with a (ret != SQLiteDB_OK)
>
> This interface to the underlying SQLiteTable::get call does not do 
> exception handling so we have lost the underlying reason.....
> I assume its a failed malloc somewhere as I am investigating out of 
> memory behaviour.
>
> Furthermore the MappingUtil::StylizeLayers method simply eats up the 
> exception.
>
> I am not sure if this is really the desired behaviour. Maybe this
should 
> be examinated in more detail.
> I am just slowly getting fed up reading a lot of code that has clearly

> not been written to be understood by other people.
> So other approaches are needed.
>
> == Smart Handling of Out of Memory Exceptions ==
>
> I suggest to wrap all memory allocation deeper in the code into retry 
> loops with a time delay doubling at each repetition (like CSMA/CD).
> Another option used in my ByteSink patch simply decreases the
allocated 
> buffer as it can in this particular case.
>
> This should increase server stability massively as memory is used and 
> returned a lot in this multi-threaded server.
> This should provide a way for the halted threads to get the memory
they 
> need at a later point in time.
> I think waiting is a realistic engeering approach to solve the OoM 
> problem.  (I learned this style while coding on a realtime audio
engine :)
>
> == Patch for #480 #520 ==
>
> The patch cleans up the file handle and removes empty files.
Furthermore 
> an intelligent exception handler uses a buffer of an available size if

> 1MB is not available anymore.
> (1MB looks like a quite a hefty estimation of the needed size in the 
> first place.....)
>
> _______________________________________________
> mapguide-internals mailing list
> mapguide-internals at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>
>   

_______________________________________________
mapguide-internals mailing list
mapguide-internals at lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/mapguide-internals