[RFC] [CTDB] Optimized memory handling.

Amitay Isaacs amitay at gmail.com
Fri Dec 15 07:57:55 UTC 2017


On Fri, Dec 15, 2017 at 6:04 AM, Swen Schillig <swen at vnet.ibm.com> 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.

Even though it's a wip, it's much easier to look at the patches which
can be applied with "git am".

Please send patches generated with "git format-patch ...".

Amitay.



More information about the samba-technical mailing list