[RFC] Performance improvements

Martin Schwenke martin at meltin.net
Wed Jul 25 08:42:04 UTC 2018


Hi Swen,

On Thu, 19 Jul 2018 10:06:40 +0200, Swen Schillig <swen at vnet.ibm.com>
wrote:

> We only read up-to (configured) buffer_size from the socket and not
> what could be read. This is the original behaviour but the code got
> modified to keep as much as possible of the improvements 
> (e.g. direct mem size calculation and not jumping back and forth until
> we have the right size)

It took me a couple of hours to understand this patch.

It does several things:

1. Drops the "extend" buffer element and does the required calculation
   inline to avoid multiple allocations for a packet

2. Introduces a variable "qb" to improve readability

3. Drops the "navail" variable and uses a MIN() calculation instead

4. Changes the conditions for the default buffer allocation

5. Avoids one use of "goto failed".

When I finally understood this, I wondered if I could accomplish the
goal (i.e. #1), more easily.

Does the attached patch do this?  It seems to work but I haven't spent
a lot of time trying to make sure it is correct.

The point I'm trying to make is that this patch was *really* hard to
review because it made a bunch of changes that aren't necessary for the
optimisation you're interested in.  Some of those other changes might
be valid clean-ups (e.g. qb) but putting them in the same patch as the
optimisation just makes it hard to understand what you're really trying
to do.  Simple decisions like using a new variable name m_length instead
of using the same pkt_size variable as in the previous function
require multiple looks because they don't follow the established
pattern.

peace & happiness,
martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ctdb-common-Extend-buffer-only-once-to-accommodate-l.patch
Type: text/x-patch
Size: 2047 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180725/c5f5403c/0001-ctdb-common-Extend-buffer-only-once-to-accommodate-l.bin>


More information about the samba-technical mailing list