[WIP] ctdb-common: optimze sock's memory managament (was [RFC] [CTDB] Optimized memory handling)

[Martin Schwenke]

Hi Swen,

On Thu, 11 Jan 2018 13:06:00 +0100, Swen Schillig <swen at vnet.ibm.com>
wrote:

> As suggested by Amitay, I mapped the code changes from ctdb_io
> to ctdb's sock "API".
> 
> The attached patch series does contain a set of changes which might not
> all be summarized under the above subject, however, the main driver of
> those changes was the optimization of the memory handling/processing.
> 
> Even though this series is flagged as WIP, I believe they're not far
> from being committable.

As per our off-list discussion, I think commits 1-5 now look good.

The only minor comment I have is that you probably want to flow the
commit messages out to roughly 80 columns.  Some of the flow is
weird.  I was leaving this superficial comment until last.  :-)

Before pushing them I would like to see some testing including some
performance numbers.  I would also like Amitay to have a look.

In "[PATCH 6/7] ctdb-common: Adding branch prediction" there is this:

-	if (! sock_queue_set_fd(queue, fd)) {
+	if (unlikely(sock_queue_set_fd(queue, fd) == false)) {

I don't think you need to change it to a compare-with-false, which is
rarely a good idea.  :-)

While touching that line it might be good to switch it to Samba's more
verbose style, by assigning the function call result to a variable ("ok"
or "status") and testing the variable in the if condition.  This can
make it easier to step through code in a debugger.  The compiler can
always optimise.

You almost certainly don't want "[PATCH 7/7] ctdb-common: Minor
cleanup.".  Although the forward declarations look a bit weird, and
possibly unnecessary, they are intentional.  The idea is that the
flow of the code can be followed by reading downwards through the
file.  You see this a lot in tevent_req-based code.  The more you see
it the more natural it becomes...  :-)

peace & happiness,
martin