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

[UV]

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
>
>