Perf improvements

[lukesteensen]

These changes together seem to get us a ~2x performance increase on the TCP -> blackhole use case we've been testing. Most are pretty straightforward.

The one that might be controversial is moving host back to being an optional field on Record. Reasons I think it's a good idea:

1. Even without the to_string, there was significant cost to the HashMap::insert because it was always allocating to resize from 0 -> 1.
2. We could initialize records with a map of non-zero capacity, but that would just move the allocation to a place where we have to pay the price all the time. Ideally there'd be no overhead if we don't use the map.
3. This works well with Determine whether data has gone through explicit structuring #256 by moving "metadata" like host out of the map. That way we can check for keys in the map to determine if a record went through a structuring transform.

That said, I'm definitely open to any feedback on any of these changes.

Tagging #268

[binarylogic]

This might be completely wrong since I'm just a lowly Elixir engineer 😄, but could you gain the same performance benefit by only cloning on the second and later sinks? This has the benefit of not cloning on the first sink when there are multiple.

[lukesteensen]

Pretty much! I just pushed a change that doesn't clone on the last sink, which is basically the same thing.

[binarylogic]

This works well with #256 by moving "metadata" like host out of the map. That way we can check for keys in the map to determine if a record went through a structuring transform.

I'm ok with this as long as the end result is the same. I can see this biting us down the road again, but I'm hoping we'll integrate benchmark results into releases before then.

[LucioFranco]

I wonder if we can do better here and use something like https://doc.rust-lang.org/std/net/struct.Ipv4Addr.html#method.octets and https://doc.rust-lang.org/std/net/struct.Ipv6Addr.html#method.segments this should reduce the overhead in checking the utf8?

[lukesteensen]

I'm not worried about this because it only happens when a new connection is established, not for every record.

[michaelfairley]

For the host issue, I wonder if we couldn't use a persistent hash map, or something Cowish, such that host isn't special-cased on Record but could be shared by default.

[LucioFranco]

Luke and I talked about this as an improvement maybe now is the right time to do this?

[lukesteensen]

@LucioFranco I'm pretty sure what we discussed was using shared values within the hashmap, which we're already doing with Bytes. This doesn't get rid of the per-record allocation of the hashmap itself.

@michaelfairley Using something like im to share the hashmap itself could be interesting. For now I think I'm happy with this, since it also handles the metadata vs data problem, but that's definitely something we could try out in the future. I also wonder if there's a way to do a similar optimization for transforms (since you can't just initialize the record with an existing shared map) with some kind of smart persistent merge?

[michaelfairley]

I forgot to point out that this break's splunk's usage of the host field: https://github.com/timberio/vector/blob/43f207c90564b8363b8784096e2e8a9c7555bd95/src/sinks/splunk.rs#L83

[binarylogic]

I actually think that's a good reason host should be a declared field. If we were using the Host field directly vector wouldn't compile if we removed it, correct?

[lukesteensen]

@michaelfairley Good catch. Opened #276 to address that.

@binarylogic Yeah, now if we remove the host field on the Record struct we'll get a compilation failure where the splunk sink uses it.