[RFC] [CTDB] Optimized memory handling.

[Swen Schillig]

On Thu, 2017-12-14 at 14:50 -0500, jim via samba-technical wrote:
> Shouldn't the 32 in '32 * DEFAULT_OBJ_SIZE' in talloc_pool call be '8
> * 
> DEFAULT_OBJ_NUMBER' or a separate define?
You're right, but this is not a committable patch yet and the numbers
are subject to change once I'm ready to run some real life scenarios.

Cheers Swen
> 
> On 12/14/2017 2:04 PM, Swen Schillig via samba-technical wrote:
> > On Fri, 2017-12-15 at 04:28 +1100, Amitay Isaacs via samba-
> > technical
> > wrote:
> > > A few comments on the code.
> > > > 1) why do we have to free the memory for queue->buf everytime ?
> > > >     I don't think this is necessary (talloc_realloc if needed)
> > > > and
> > > >     would save time again. Once done with the queue it'll be
> > > >     free'd anyway.
> > > 
> > > To avoid the buffer sizes growing.  Instead of adding some
> > > heuristic
> > > to shrink the buffer size, this was simpler.
> > 
> > The buffer would only grow if needed and the lifetime of the queues
> > (including their attached buffers) is limited as well.
> > Therefore I was thinking we could leave out the shrinking AND the
> > explicit free'ing of the buffer.
> > What do you think ?
> > > > 2) I still believe we don't need a destructor.
> > > >     There's nothing done which is either required (queue-> =
> > > > -1) or
> > > >     not done automatically (TALLOC_FREE(queue->fde)).
> > > 
> > > Sure.
> > 
> > Done.
> > > > 3) @Amitay: I really like the removal (in comparison to
> > > > ctdb_io) of
> > > >     the really expensive "data" memory / memcpy thing, that on
> > > > it's
> > > > own
> > > >     should save time already. I just have to make sure to
> > > > change all
> > > >     the callbacks for ctdb_io to NOT free that memory anymore.
> > > 
> > > That may not be possible.  The model used here is that the
> > > callback
> > > is
> > > supposed to free the memory once the callback has processed the
> > > packet.  Usually the packets are processed quickly and freed, so
> > > we
> > > can still use talloc pools.
> > 
> > Ok, as mentioned in an earlier mail, I re-introduced the idea of
> > the
> > data_pool again.
> > > > Attached you can find two patches, the memory-opt patch is
> > > > pretty
> > > > small
> > > > and straight forward. I could prepare a committable patch by
> > > > tomorrow,
> > > > if it's wanted. Please let me know if you'd be ok with the 2
> > > > addtl.
> > > > changes I suggested above( no destructor / no mem-free).
> > > > 
> > > > Regarding the other patch (ctdb_sock_io replacement) this is
> > > > WIP
> > > > and
> > > > won't come out of this state until next year.
> > > > It does compile but has most likely a good few bugs in it.
> > > 
> > > Can you send the patches created with "git format-patch"
> > > command?  I
> > > cannot "git am" the attached patches.
> > 
> > Yep, see attachment.
> > Wasn't sure if the changes are even wanted, therefore I chose to
> > send
> > just a diff.
> > 
> > Thanks again for reviewing.
> > 
> > Cheers Swen
> 
>