Reduce UDP service per-packet allocation size

[sparrc]

Required for all non-trivial PRs

• Rebased/mergable
• Tests pass
• CHANGELOG.md updated

[sparrc]

This change makes the UDP service only create a buffer that is the exact size of the incoming packet.

This will reduce memory pressure and number of GC cycles, my results sending 100,000 UDP points were:

• udp-payload-size=0: 242 GC cycles
• udp-payload-size=1500: 142 GC cycles
• with change (no more config option): 114 GC cycles

[sparrc]

cc @joelegasse because you've been working on the influxdb relay, might be relevant to the UDP service there

[joelegasse]

I actually did something similar in the relay that entirely eliminates the need for the config option, and instead just has a per-listener 64K buffer. :-)

https://github.com/influxdata/influxdb-relay/blob/master/relay/udp.go#L113

[sparrc]

this essentially does that too, I just don't really understand the influxdb config enough to know how to deprecate a config option?

[sparrc]

Erp, nevermind, I've figured it out

[sparrc]

cc @sebito91, I believe you made this change to have a configurable udp-payload-size in the first place. This change goes a bit further and only allocates a buffer on each loop iteration that is the exact size of the incoming packet.

[sebito91]

I'm away from the machine at the moment but the reason we put in the option
was to prevent alloc of the entire 64k buffer each time you called read on
the socket...if you do a very cheesy memalloc in a loop and time it you
will see about 10x speed up for a smaller buffer (e.g. 1600 vs 64k).

I will post the findings when I'm back at the terminal.

On Thursday, April 7, 2016, Cameron Sparr notifications@github.com wrote:

cc @sebito91 https://github.com/sebito91, I believe you made this
change to have a configurable udp-payload-size in the first place. This
change goes a bit further and only allocates a buffer on each loop
iteration that is the exact size of the incoming packet.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#6263 (comment)

Sebastian Borza

PGP: EDC2 BF61 4B91 14F2 AAB4 06C9 3744 7F3F E411 0D3E

[sebito91]

And re-reading the thread I think this change would actually be awesome,
that was my intent before but my Golang skill set at the time was wonky :/

👍 from me!

On Thursday, April 7, 2016, Sebastian Borza sborza@alumni.princeton.edu
wrote:

I'm away from the machine at the moment but the reason we put in the
option was to prevent alloc of the entire 64k buffer each time you called
read on the socket...if you do a very cheesy memalloc in a loop and time it
you will see about 10x speed up for a smaller buffer (e.g. 1600 vs 64k).

I will post the findings when I'm back at the terminal.

On Thursday, April 7, 2016, Cameron Sparr <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

cc @sebito91 https://github.com/sebito91, I believe you made this
change to have a configurable udp-payload-size in the first place. This
change goes a bit further and only allocates a buffer on each loop
iteration that is the exact size of the incoming packet.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#6263 (comment)

Sebastian Borza

PGP: EDC2 BF61 4B91 14F2 AAB4 06C9 3744 7F3F E411 0D3E

Sebastian Borza

PGP: EDC2 BF61 4B91 14F2 AAB4 06C9 3744 7F3F E411 0D3E

[sebito91]

IIRC we have to zero out buf for each subsequent ReadFromUDP otherwise we fill the buf?

[sparrc]

each call to ReadFromUDP will write to buf from the beginning, so it won't ever fill

[joelegasse]

This is correct the way it's written. ReadFromUDP can't modify our slice, so buf will always refer to the full 64K slice, and will get data copied into it starting at index zero, and return n, the number of bytes that were copied for a given read.

[sebito91]

I'm not sure why I thought (or remembered) differently, but as long as this checks out and passes tests then I'm fine with it.

[joelegasse]

LGTM 👍