[RFC] [CTDB] Optimized memory handling.

[jim]

Shouldn't the 32 in '32 * DEFAULT_OBJ_SIZE' in talloc_pool call be '8 * 
DEFAULT_OBJ_NUMBER' or a separate define?

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