[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