[Spread-users] Another way to leak (valgrind report)

[Theo Schlossnagle]

On Aug 30, 2004, at 11:16 PM, Ryan Caudy wrote:
> Jonathan did apply Theo's sl_destruct patch to CVS last spring, and my
> groups rewrite (not yet available from CVS) also uses sl_destruct to
> prevent these sorts of leaks.

My apologies.  I checked my 3.17.2 source tree and didn't see it.  I 
forgot that it was that old :-)  Oops.

> Dave, what version are you using, 3.17.2?  or CVS?
>
> Theo, for those of us who aren't in the know, what is the arena
> system?  I don't recall the patch, but it may have been before my
> time.  I know that the skiplists are an important part of Spread's
> scalability, and anything that improves their performance would
> certainly impact the groups layer's CPU performance.

the way memory.c allocates memory is by using freelists with 
watermarks.  Effectively, memory.c allows Spread to manage its own 
arena for data types that it knows about.  As the vast majority of 
memory allocations in Spread of fixed sizes (that Spread knows about).  
As spread isn't multithreaded, it can simply place freed objects of a 
certain size on the the freelist for that object type and later "alloc" 
(on in Spread "new") an object at the cost of a two point dereferences, 
a pointer set and a few counter updates (faster than malloc).

I know I wrote the code to have the skiplists use Spread's "new" and 
"dispose", but that was back when I wrote skiplist.c and reworked 
groups.c -- in other words, a long long long time ago.

We use skiplists in Ecelerity and our unit tester shows a 1.17 speedup 
on memory allocations by using memory allocator vaguely similar to the 
one in Spread.  We opted for a pagetable approach over a freelist due 
to the  number of tiny allocations we have.  But you should see a 
similar speedup to that in Spread.

If you're in the market for speed-ups, I'd snag a copy of valgrind and 
run it with the cachegrind skin.  kcachegrind will show you some things 
to seriously consider improving.  One thing on the skiplist topic -- 
which makes the code a little ugly, but adds a serious performance 
boost is:

Take all the skiplist comparator functions in groups.c:
   G_compare
   G_member_recordcompare
   G_member_keycompare
   G_work_groups_comp
   G_work_groups_keycomp

And make them inline functions in groups.h adding also:
#define SL_INTERNAL_G_COMPARE 0x1
#define SL_INTERNAL_G_MEMBER_RECORDCOMPARE 0x2
.. etc. etc.

Change the sl_find_compare and and sl_insert_compare to switch on the 
comparison functions and call the groups.h inline comparison functions 
is they match the #defines.  Then update groups.c to pass the magic 
SL_INTERNAL_* keys into sl_set_compare instead of the function 
pointers.

While I'll be the first to admit this is an ugly hack.  If you have a 
lot of membership changes and you find the sl_* to be got code paths, 
the benefit from having the compiler inline the comparison functions is 
so substantial it is visible to the naked eye.

// Theo Schlossnagle
// Principal Engineer -- http://www.omniti.com/~jesus/
// OmniTI Computer Consulting, Inc. -- http://www.omniti.com/
// Ecelerity: fastest MTA on Earth